LUCENE-8850: Calculate the area of a polygon and throw error when values are invalid #709
Conversation
| } | ||
|
|
||
| /** returns the number of vertex points */ | ||
| public int numPoints() { | ||
| return polyLats.length; | ||
| } | ||
|
|
||
| /** returns the area of the polygon */ | ||
| public double area() { |
There was a problem hiding this comment.
I can imagine some asking what the units are. And why not getArea? Maybe this should be marked @lucene.internal or package local even if we don't want to expose it.
There was a problem hiding this comment.
I rename the method to getArea() and make it protected. I have no use case outside of validation so it is ok to make it package private.
There was a problem hiding this comment.
I agree w/ @dsmiley re: units. The calculation is using the shoelace method on vertices in decimal degrees. So the units are deg^2. I remember exploring this a little over a year ago in LUCENE-8364 and originally having area because it was "easy". I ended up removing it until we had support for indexing in other projections (e.g., equal area). I think we should wait until LUCENE-8632 lands and only provide this area calculation in XYPolygon for regular cartesian geometries in non decimal degrees.
There was a problem hiding this comment.
I agree we should not provide this info to the user, +1 not to offer the calculation .
The idea behind this change was validation of the polygon. The idea of this change came from the fact that I am using this calculation to check that a Polygon tessellation is correct and I realise that:
-
Currently we compute the signed area to check the orientation of a polygon, if the area is 0 the polygon is considered CW, should we fail instead?
-
I have seen polygons with one hole where the hole and polygon are the same, should we capture that situation and fail?
There was a problem hiding this comment.
Please add a comment to the code to to link to the "shoelace method" that Nick points at. I think it's important for non-trivial algorithms to incorporate references where the readers of this code can learn more. Who knows... maybe some "velcro method" will come along and be superior and then some student looking at this code will point this out to us :-)
There was a problem hiding this comment.
- Currently we compute the signed area to check the orientation of a polygon, if the area is 0 the polygon is considered CW, should we fail instead?
+1 to fail in this situation since it is not a valid polygon.
- I have seen polygons with one hole where the hole and polygon are the same, should we capture that situation and fail?
+1 to fail in this situation as well.
|
One of the things I notice with this change is that it breaks some of the benchmarks, in particular it breaks this one as the polygon contain invalid holes: https://home.apache.org/~mikemccand/geobench.html#search-polyMedium |
|
What is the particular rule that breaks? |
|
Reading one of the polygons representing a London Borough it fails because it contains a hole where all points are coplanar. |
Compute area of a polygon and throw an error when values are invalid.