Prune implicit machinery flagged by the substrate audit#439
Closed
asinghvi17 wants to merge 4 commits into
Closed
Conversation
…ethods
`_select_edge_set_accelerator` and the `AutoAccelerator` arm of
`_build_prepared_edge_index` had identical bodies on both manifolds;
dispatch on `Union{Planar, Spherical}` instead. Also update the
`AutoAccelerator` docstring, which still claimed non-`Planar` manifolds
never take the tree path.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The tree path builds a `NaturalIndex` over each side's segment extents and traverses the two simultaneously — exactly what the existing `DoubleNaturalTree` accelerator names. `AutoAccelerator` selected (and the docs advertised) `DoubleSTRtree`, which builds no STRtree here. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Unprepared `locate_on_polygonal` counted queries per polygonal element and silently switched from the direct ring loop to a lazily built `IndexedPointInAreaLocator` on the 9th — an implicit adaptive index that deviates from Java, which keys indexing purely on `isPrepared`. Restore that rule: unprepared mode scans the rings on every query, and repeated point location is what `prepare` (the explicit escape hatch) is for. This makes `sort_leaves = false` dead — the lazy path was its only user — so `SortedPackedIntervalRTree` always midpoint-sorts, as JTS does; the measured sorted-vs-ring-order tradeoff stays recorded in its docstring. `poly_query_count` and `_LAZY_INDEX_QUERY_THRESHOLD` are deleted with the heuristic. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
As ported it could never return `false` — matrix entries start at DIM_FALSE and only increase, so DIM_UNKNOWN never appears. A note keeps the JTS-parity gap explained. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jul 13, 2026
Member
Author
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.
Stacked on #438 (base branch
relateng-shared-substrate). The four small cleanups from the substrate audit; each is its own commit.Merge the byte-identical
Planar/Sphericaltwin methods._select_edge_set_acceleratorand theAutoAcceleratorarm of_build_prepared_edge_indexnow dispatch onUnion{Planar, Spherical}(the manifolds with a segment-extent kernel). Also fixes theAutoAcceleratordocstring, which still claimed non-Planarmanifolds never take the tree path.Name the relateng tree accelerator
DoubleNaturalTree. The tree path builds aNaturalIndexover each side's segment extents and traverses the two simultaneously — exactly what clipping's existingDoubleNaturalTreenames.AutoAcceleratorselected (and the docs advertised)DoubleSTRtree, which builds no STRtree here.Remove the lazy point-in-area auto-index. Unprepared
locate_on_polygonalcounted queries per polygonal element and silently switched to a lazily builtIndexedPointInAreaLocatoron the 9th — an implicit adaptive index deviating from Java, which keys indexing purely onisPrepared. Restored the Java rule: unprepared mode scans the rings on every query; repeated point location against one geometry is whatprepare(the explicit escape hatch) is for. This orphanssort_leaves = false, soSortedPackedIntervalRTreealways midpoint-sorts as JTS does (the measured sorted-vs-ring-order tradeoff stays recorded in its docstring), andpoly_query_count/_LAZY_INDEX_QUERY_THRESHOLDare deleted.For the record, the removed heuristic's rationale: an unsorted index build cost ~10–13 ring scans (Natural Earth coastlines), so switching at 8 queries had worst-case regret of about one build. If unprepared multi-element relates ever show up hot,
preparecovers it explicitly.Delete the unused
is_known_entry. As ported it could never returnfalse(matrix entries start at DIM_FALSE and only increase, so DIM_UNKNOWN never appears); a comment keeps the JTS-parity gap explained.Tests
Full RelateNG suite green (JTS XML conformance, engine, LibGEOS differential fuzz, spherical conformance + end-to-end, allocations + type stability). The lazy-index testset now asserts the unprepared locator never builds an index; the prepared-vs-unprepared differential is unchanged.
🤖 Generated with Claude Code