search

libgeos / geos

Geometry Engine, Open Source
https://libgeos.org
GNU Lesser General Public License v2.1
1.1k stars 339 forks source link

Add test for GH issue 1064, for main branch #1093

Open strk opened 3 weeks ago

strk commented 3 weeks ago

See GH-1064

And see GH-1092 for the 3.8 runs

dr-jts commented 3 weeks ago

AFAIK there is no known fix for this. It's caused by rounding in the calculation of the intersection point (note, not the intersects test, which is accurate). So this test can't be added to CI at this time, since it will just keep failing.

Note that 3.8 is reporting that there is no intersection between the lines at all. The Orientation predicate however is reporting that there is a proper intersection between them (see image below). This uses the best known algorithm we have, so the only thing we can do is to accept it as the correct analysis of the situation. So in this case 3.8 is clearly wrong. The result of this test should be changed to be the result produced in later versions of GEOS (F01FF0102).

image
strk commented 3 weeks ago

Rather than adding a broken expectance I'd expect the right answer, and I'd expect the test to fail.

If the picture you attached is correct the right answer should be: 0F1FF01F1, right ?

Maybe we can implement an "expected failure" that turns red in case the answer gets correct

dr-jts commented 3 weeks ago

Rather than adding a broken expectance I'd expect the right answer, and I'd expect the test to fail. If the picture you attached is correct the right answer should be: 0F1FF01F1, right ?

What is the "right" answer is very subtle. The best we can say is based on whatever numerical method is used to evaluate the Orientation predicate. (It can also depend on the fundamental imprecise representation of decimal numbers - e.g. 0.9, as in #968.).

For current versions of GEOS the answer is as given above. Older versions use different arithmetic and get a different answer. I agree there should be a "correct" answer - but I don't know how to calculate it (do you?). And in any case, even if the computed answer is wrong, it doesn't help us to have CI failing all the time.

Maybe we can implement an "expected failure" that turns red in case the answer gets correct

Yes, I've done this in JTS in a few places, mostly in order to capture cases which are known failures. But someone needs to figure out what the correct answer is in this case. At the moment all we have are two different answers, with no way to choose between them.

dr-jts commented 3 weeks ago

I'm not sure there's much point in adding tests like this, where there is not a "known good" answer. If we change (or improve) the orientation predicate algorithm, that may cause this test to start failing, but it won't provide a solid indication that something is wrong. So we'd end up just changing or deleting it.

I'm actually dealing with exactly this situation, with a change I'm considering for the orientation predicate. A bunch of tests fail with the new code, but they don't indicate it's wrong, just has different results. It's to handle #968 - and it does make that case work.