Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run-time exception with difference op after removal of old overlay engine #797

Closed
mwtoews opened this issue Jan 12, 2023 · 5 comments
Closed
Assignees
Labels

Comments

@mwtoews
Copy link
Contributor

mwtoews commented Jan 12, 2023

Since #788 a new runtime exception is encountered:

$ ./bin/geosop -f wkt -a "GEOMETRYCOLLECTION(POINT(51 -1), LINESTRING(52 -1, 49 2))" -b "POINT(2 3)" difference
Run-time exception: IllegalArgumentException: Overlay input is mixed-dimension

and same also with differenceSR.

Previous behaviour was:

$ ./bin/geosop -f wkt -a "GEOMETRYCOLLECTION(POINT(51 -1), LINESTRING(52 -1, 49 2))" -b "POINT(2 3)" difference
GEOMETRYCOLLECTION (POINT (51 -1), LINESTRING (52 -1, 49 2))
@mwtoews mwtoews added the Bug label Jan 12, 2023
@mwtoews mwtoews changed the title Difference run-time exception after removal of old overlay engine Run-time exception with difference op after removal of old overlay engine Jan 12, 2023
@dbaston
Copy link
Member

dbaston commented Jan 13, 2023

I think the GEOS support for mixed-dimension overlay may be accidental -- the docs say that it should raise an exception. And it apparently has no tests, or removing the support would have caused a failure.

Some more background is in https://trac.osgeo.org/geos/ticket/755 and https://trac.osgeo.org/geos/ticket/1092

Should we:

  • Do nothing, and continue to produce the error,
  • Extend OverlayNG to handle these cases, or
  • Maintain a second overlay engine with no tests or documented semantics to handle these cases ?

@dr-jts
Copy link
Contributor

dr-jts commented Jan 13, 2023

It would be nice to know how the old code handled mixed-type collections. And how far the support goes (e.g. does it handle subtracting a line from a line and point which are overlapped?). (I wonder if the HeuristicOverlay just kept reducing precision until it produced a totally garbage answer, which just happens to be right in this case?)

Extending OverlayNG to handle this is going to be a significant piece of work, so not a short-term solution.

My vote is to keep producing the error, as originally intended and as implemented in the current code.

@dr-jts
Copy link
Contributor

dr-jts commented Jan 13, 2023

For the record, at the moment I think the way to implement OverlayNG support for mixed collections is to run a series of overlay ops on sub-collections of elements of same dimension, and then combine the results. This needs to be preceded by a union of each input to make them non-overlapping.

For example, for intersection the result is the intersection of each combination of areas, lines, points from A against the areas, lines and points of B (once A and B have been made non-overlapping).

This is a lot of overlap ops, but in many cases some of them can be omitted due to empty or disjoint sub-collections.

pramsey added a commit to pramsey/geos that referenced this issue Jun 12, 2023
@strk
Copy link
Member

strk commented Jun 14, 2023

Here's a real world example of the heuristic overlay reducing precision up to garbage answer: https://trac.osgeo.org/postgis/ticket/5401

@pramsey
Copy link
Member

pramsey commented Jun 14, 2023

I think that #924 is a different problem than this (the precision reduction). Let's keep this just about handling mixed-dimension collections (which I'm about to merge a fix for).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants