Unify the spherical relate kernel with the shared extent substrate#438
Closed
asinghvi17 wants to merge 21 commits into
Closed
Unify the spherical relate kernel with the shared extent substrate#438asinghvi17 wants to merge 21 commits into
asinghvi17 wants to merge 21 commits into
Conversation
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Anshul Singhvi <anshulsinghvi@gmail.com> Co-authored-by: Alex Gardner <alex.s.gardner@jpl.nasa.gov>
Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: rafaqz <rafaelschouten@gmail.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* Allow `SphericalCap`s to intersect 3D `Extents.Extent`s Compare the squared Euclidean distance from the cap center to the box against the squared chord radius — the same comparison as S2's `S2Cap::Contains(S2Point)`, extended to a box by clamping the center into it. The chord radius is `2sin(min(radius, π)/2)` following S2's `S1ChordAngle`, which keeps precision at tiny radii where `2 - 2cos(radius)` cancels to zero. Conservative for spatial tree pruning: never a false negative for geometry on the sphere. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * improve docstring Co-authored-by: Anshul Singhvi <anshulsinghvi@gmail.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
A great-circle arc bulges out of its endpoints' bounding box, so edge extents built from endpoints alone silently miss queries that touch only the bulge. `spherical_arc_extent(a, b)` computes the true 3D Cartesian extent of the shorter arc: each coordinate along the arc is a sinusoid, so an interior extremum exists iff the coordinate rises at one endpoint and falls at the other, where it attains the sinusoid's amplitude — no trigonometric calls, with tangents from `robust_cross_product` so nearly-degenerate arcs stay stable. Bounds are padded by a few ulps to guarantee containment, as S2's `S2LatLngRectBounder` does. `eachedge`, `to_edgelist`, and `lazy_edgelist` gain manifold-first methods: `Spherical()` yields edges as `UnitSphericalPoint` pairs (converting geographic input) whose `GI.Line`s carry arc extents, and rings that are already `UnitSphericalPoint`s take that path with no manifold argument. Previously `_lineedge` threw a `MethodError` for such rings. Spatial trees built over these edges (`NaturalIndex`, `RTree`) therefore index spherical edges in 3D, correctly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`Extents.extent(m::Manifold, geom, [T])` computes extents on a manifold: `Planar()` delegates to `GI.extent`; `Spherical()` returns 3D Cartesian extents on the unit sphere. Curves are covered by the union of their edges' arc extents. Rings and polygons are regions under S2's loop convention (CCW, interior on the left): an extremum over a region lies on the boundary or at one of the six axis points `±eᵢ`, so the boundary extent is widened per axis by an enclosure check from the ring's winding number and signed area. GO does not export `extent`; the methods extend `Extents.extent`, reachable as `GO.extent`. Also fix `to_unit_spherical_points` to pass `UnitSphericalPoint`s through unchanged instead of reinterpreting their first two Cartesian coordinates as geographic longitude/latitude. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Axis-point enclosure in `_spherical_region_extent` is now decided the way `S2Loop::InitBound` decides pole containment: the side of an anchor edge gives the departure side of the arc from the query point to the anchor's midpoint, and the parity of transversal boundary crossings flips it. Degenerate configurations retry with the next anchor and fall back to a conservative extension, never an under-covering box. This removes both documented approximations of the winding approach: a region containing both poles at small area (dumbbell) and edges sweeping near an axis are now handled by construction. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Tighten the literate headers, docstrings, and comments of `spherical_arc_extent` and `Extents.extent(m::Manifold, geom)`; fix an overstated dumbbell test comment (zero net winding holds about the polar axis, not every axis). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`rk_interaction_bounds(::Spherical)` now composes `spherical_arc_extent` and `_spherical_region_extent` over kernel-converted points instead of carrying a private extent stack; `arc_extent`, `_point_box`, `_arcs_extent`, `_sph_bounds`, and `_widen_area_axes` are deleted. Area widening thereby gains the parity test's super-hemisphere and degeneracy handling. Rings keep dim-1 curve bounds (a CW hole must not become a complement region), and boxes keep a few ulps of padding. The antipodal-edge throw moves from the extent computation to explicit ingest validation (`_validate_relate_edges`, run in the extent-cache pass), since `spherical_arc_extent` deliberately picks a stable plane instead of throwing. Stored-extent reuse on `Spherical` now requires a 3D `(X, Y, Z)` extent (a user's lon/lat box must not be compared against unit-sphere boxes), and `_union_stored_extents` computes missing child extents through `rk_interaction_bounds` instead of `GI.extent`. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…t_in_ring` `spherical_ring_contains` moves from the extent internals to UnitSpherical as documented public API (the extent code and the relate kernel are its two consumers), with its three geometric predicates injectable: `orient`, `on_arc`, and `proper_crossing` default to the tolerance-based substrate predicates, so the extent path is unchanged. `rk_point_in_ring(::Spherical)` keeps its exact boundary classification and delegates the interior decision to the shared anchor-retry parity, injecting `rk_orient` and `_arcs_cross_properly` so the answer is as exact as the kernel's predicates. This replaces the fixed north-pole reference walk, whose documented degeneracies (query at/antipodal to the pole, vertex on the query meridian) and super-hemisphere winding limitation no longer apply; an all-anchors-degenerate configuration now throws instead of answering wrong. The scale-dependent guards in the shared function are normalized so the conformance suite's exact integer (non-unit) rings work unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`rk_bounds_disjoint` was `!Extents.intersects` under a kernel name — its call sites now say that directly. `rk_bounds_covers` had no callers (the covers short-circuit uses `ext_covers` = `Extents.covers`, which is dimension-generic over shared keys). The kernel contract now documents `rk_interaction_bounds` as the shared manifold extent plus relate glue, and the `_validate_relate_edges` ingest hook. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Share `_exactly_antipodal` between the ingest validation and `AntipodalEdgeSplit` (one spelling of the condition), route the spherical point interaction box through `GI.extent`, drop narrative from the new kernel comments, and update the extent-cache comment for `_reusable_stored_extent`. 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
3 participants
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 #422 (base branch
relateng-spherical), which stacks on #416. Note for review: the branch mergesspherical-arc-extent(#434) to get the shared substrate, so the three-dot diff includes #434's content until it lands onmain; the changes to review are the four commits after the merge (84e02ce…8d5ea7e, +250/−280 net).What this does
Makes the spherical relate kernel consume the shared spherical substrate (#434) instead of carrying a private fork of it. The kernel keeps exactly what is relate-specific — point conversion, ingest validation, exact predicates, padding — and delegates the geometry.
Extent delegation.
rk_interaction_bounds(::Spherical)now composesspherical_arc_extentper edge and_spherical_region_extentfor area interiors, over kernel-converted points (so boxes and ingested vertices agree bit-for-bit); the kernel's ownarc_extent,_point_box,_arcs_extent,_sph_bounds, and_widen_area_axesare deleted. Area widening thereby upgrades from the winding-sum test (documented super-hemisphere limitation) to the shared crossing parity with anchor retry and conservative ±1 fallback. Two deliberate seams:_validate_relate_edges, run once per curve atRelateGeometryconstruction), sincespherical_arc_extentdeliberately picks a stable plane instead of throwing.Ring containment. The anchor-retry crossing parity from #434 is promoted to
UnitSpherical.spherical_ring_contains(documented, exported) with its three geometric predicates injectable —orient,on_arc,proper_crossing— defaulting to the tolerance-based substrate predicates, so the extent path is unchanged.rk_point_in_ring(::Spherical)keeps its exact boundary classification and injectsrk_orient+_arcs_cross_properlyfor an interior decision as exact as the kernel's predicates. This retires the fixed north-pole reference walk and its documented degeneracies (query at/antipodal to the pole, vertex on the query meridian); an all-anchors-degenerate configuration now throws informatively instead of answering wrong.Pruning.
rk_bounds_disjoint(=!Extents.intersectsrenamed) andrk_bounds_covers(no callers) are deleted; extent tests use Extents.jl vocabulary directly. Stored-extent reuse onSphericalnow requires a 3D(X, Y, Z)extent, so a user's lon/lat box is never compared against unit-sphere boxes, and_union_stored_extentscomputes missing child extents in the manifold's coordinate space.Tests
Full suite green: JTS XML conformance 6,537; RelateNG engine 1,868; LibGEOS differential fuzz 500; spherical kernel conformance 51,614 (the arc-extent containment testset moved to
test/utils/unitspherical.jlwith the substrate); spherical end-to-end incl. the relocated antipodal guard; allocations + type stability.🤖 Generated with Claude Code