Skip to content

Prune implicit machinery flagged by the substrate audit#439

Closed
asinghvi17 wants to merge 4 commits into
relateng-shared-substratefrom
relateng-cleanups
Closed

Prune implicit machinery flagged by the substrate audit#439
asinghvi17 wants to merge 4 commits into
relateng-shared-substratefrom
relateng-cleanups

Conversation

@asinghvi17

Copy link
Copy Markdown
Member

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/Spherical twin methods. _select_edge_set_accelerator and the AutoAccelerator arm of _build_prepared_edge_index now dispatch on Union{Planar, Spherical} (the manifolds with a segment-extent kernel). Also fixes the AutoAccelerator docstring, which still claimed non-Planar manifolds never take the tree path.

  • Name the relateng tree accelerator DoubleNaturalTree. The tree path builds a NaturalIndex over each side's segment extents and traverses the two simultaneously — exactly what clipping's existing DoubleNaturalTree names. AutoAccelerator selected (and the docs advertised) DoubleSTRtree, which builds no STRtree here.

  • Remove the lazy point-in-area auto-index. Unprepared locate_on_polygonal counted queries per polygonal element and silently switched to a lazily built IndexedPointInAreaLocator on the 9th — an implicit adaptive index deviating from Java, which keys indexing purely on isPrepared. Restored the Java rule: unprepared mode scans the rings on every query; repeated point location against one geometry is what prepare (the explicit escape hatch) is for. This orphans sort_leaves = false, so SortedPackedIntervalRTree always midpoint-sorts as JTS does (the measured sorted-vs-ring-order tradeoff stays recorded in its docstring), and poly_query_count / _LAZY_INDEX_QUERY_THRESHOLD are 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, prepare covers it explicitly.

  • Delete the unused is_known_entry. As ported it could never return false (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

asinghvi17 and others added 4 commits July 13, 2026 12:58
…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>
@asinghvi17

Copy link
Copy Markdown
Member Author

Collapsed into #416 for whole-piece review — the relateng branch was fast-forwarded to this stack's tip, so #416 now contains these commits. This PR stays as a focused, reviewable diff of its layer (see the review map in #416).

@asinghvi17 asinghvi17 closed this Jul 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant