Repository navigation
FIX: Work out the area of polygons with holes and stop the compare run when a consumer fails - #394
Merged
Merged
Conversation
…n when a consumer fails
justadreamer
approved these changes
Sep 18, 2026
… to 20m in unit tests
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why:
Example_OnPremise_CompareConsolesat idle for 12 minutes on the current Enterprise data file until the test host was killed. The same night the Java examples failed on the same data withPoints of LinearRing do not form a closed linestring(51Degrees/ip-intelligence-java-examples#152), and the .NETCalculationsclass has the same code, so this ports that fix and also removes the hang that hid the error here.1. Holes
TransformGeometrytookgeometry.Coordinates, which is the points of every ring in one list, and built a single ring from them. For a polygon with holes the first point belongs to the outer boundary and the last one to the last hole, so the ring does not close and NetTopologySuite throws. The outer boundary and each hole are now transformed as separate rings and put back together as a polygon with holes. A multipolygon or collection is transformed piece by piece.2. Intersections made of several pieces
When the intersection of an area with a grid cell has more than one piece, the loop read
geo.GetGeometryN(i)where it meantintersect.GetGeometryN(i). For a polygon that returns the whole area each time, so the whole area was added once per piece and the result was too large. It now uses the pieces of the intersection.The existing
TestWktcase with three polygons expected 22803 square km, which is the inflated number this bug produced. The fixed code gives 20118, and so does the fixed Java implementation with its own geometry and projection libraries, so the expected value is updated.3. Why it hung instead of failing
In
Compare-Consoleeach consumer catches onlyOperationCanceledException, so any other exception ends the consumer's task silently. The truth collection is bounded to the processor count, so once every consumer had died on a polygon with holes the producer waited for ever inTryAdd. A consumer that fails now cancels a token linked to the caller's one; the producer stops, and the exception is thrown where the consumer's task is awaited, so the run fails with the polygon in the message instead of hanging.Tests
Two tests in
TestAreas, both written first and seen to fail:TestPolygonWithHolefailed withpoints must form a closed linestring.TestPolygonSplitByGridCellis a U shape whose arms are two separate pieces in the next grid cell. It returned 8759 where 3438 was expected.Both pass now, with the other eight. The compare example's stop-on-failure path has no unit test, because
Example.Runneeds a real on-premise engine; CI is the check for it, as it is forExample_OnPremise_CompareConsoleitself, which is skipped locally without the current data file.