Spherical RelateNG kernel#422
Open
asinghvi17 wants to merge 18 commits into
Open
Conversation
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Upgrade rk_point_on_segment to a scale-invariant cross-product span test (exact for unit and non-unit inputs alike), which the classify symmetry tests over integer-grid points require. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Generalize the PolygonNodeTopology port (_compare_angle, _is_angle_greater, _is_between, _compare_between, rk_crossing_dirs_ccw, rk_is_crossing, rk_is_interior_segment) to m::Manifold; they depend only on rk_quadrant and rk_orient. Add a tangent-plane rk_quadrant and a crossing-apex comparator for Spherical. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
An area element's boundary-arc box does not cover its interior (a ring encircling a pole has a thin boundary band but its interior reaches the pole). Widen each axis whose ±eᵢ is interior to the exterior ring out to ±1 — conservative, since over-covering can only under-prune. Decide ±eᵢ membership by the ring's winding about the axis rather than a per-triangle signed-solid-angle sum: each projected edge contributes a turn < π, so there is no atan2 branch ambiguity. The solid-angle sum reported spurious interiors for axes far from the loop's winding axis (e.g. equatorial axes against a polar-cap ring). `_ring_contains_pole` now shares this winding test. Documented limitation: super-hemisphere rings under-report a non-encircled interior axis. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Thread the manifold's kernel point type `P` through the engine so a
non-planar manifold ingests its own coordinate representation. Planar is
byte-identical: `_kernel_point_type(::Planar)` is the signed-zero
`Tuple{Float64, Float64}` and `_to_kernel_point(::Planar, p)` is
`_node_point(p)`, while `_kernel_point_type(::Spherical)` is
`UnitSphericalPoint{Float64}` converted via `_spherical_kernel_point`.
- `kernel.jl`: add `_kernel_point_type`, `_to_kernel_point`,
`_to_kernel_points` (manifold-aware `_node_points`), and
`_kernel_point_box` (dimension-matched point extent).
- `RelateGeometry` and `RelatePointLocator` gain a `P` type parameter;
`unique_points`, the locator `points`/`line_boundary`/`adj_edge_locator`
fields, the segment-string eltype and `TopologyComputer`'s `NodeKey`
dict are all typed from it.
- Route every coordinate ingest (`_node_points`/`_node_point`/`_tuple_point`
on raw vertices) through `_to_kernel_point(m, …)`; the two remaining
`_tuple_point`/`_node_point` reads are pure equality checks where
conversion preserves the result.
- Spherical `rk_point_in_ring` reads a ring via `_ring_kernel_pts`: a 3D
ring is already kernel coordinates (read verbatim to preserve the exact
boundary orient), a 2D lon/lat ring is converted.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Smoke tests for `relate(RelateNG(; manifold = Spherical()), …)` on the unindexed NestedLoop path: a mid-latitude overlapping-box patch whose DE-9IM matrix matches the planar result exactly, and a pole-containing cap ring that `contains` a near-pole point — a case only the spherical kernel classifies correctly. Both pass without further engine changes, confirming the Phase 3 plumbing (ring orientation, node topology, point-in-ring) is correct end-to-end. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Make the tree-accelerated edge-intersection paths (unprepared and prepared) valid on the sphere by building the per-segment spatial index over bulge-aware 3D great-circle `arc_extent`s instead of a 2D endpoint box. A 2D box misses a long arc's bulge — e.g. a near-equatorial arc from lon 0 to 170 bulges to y ≈ 1 at lon 90 while its endpoint box has y ∈ [0, 0.17] — and prunes away real crossings, producing a wrong DE-9IM. - `_segment_extent` / `_segment_extent_table` / `_push_segment_extents!` dispatch on the manifold; Spherical uses `arc_extent`. - `_select_edge_set_accelerator(::Spherical)` enables the tree above the segment-count threshold; `_segment_envs_disjoint(::Spherical)` prunes the nested loop with 3D arc extents. - `_build_prepared_edge_index(::Spherical, ::AutoAccelerator)` indexes A in 3D, so prepared spherical relate matches the unprepared result (the `NaturalIndex` is dimension-generic — no separate STR ordering needed). Covered by `spherical_end_to_end.jl`: the arc-bulge equivalence case, AutoAccelerator above threshold, and prepared/unprepared agreement. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
An edge between exactly-antipodal vertices has no unique great-circle arc (its endpoint cross product vanishes while the dot product is negative), so the bulge math in `arc_extent` is undefined. Detect that case at the ingest chokepoint — every edge's extent is computed through `arc_extent` during `RelateGeometry` construction — and throw an `ArgumentError` that names the `AntipodalEdgeSplit` remedy. A vanishing normal with a positive dot product is a zero-length/repeated vertex and is left alone. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The opt-in remedy for the antipodal-edge error: a `GeometryCorrection` that inserts the lon/lat midpoint of every edge whose endpoints map to exactly-antipodal unit vectors, replacing one ambiguous edge with two well-defined arcs. Antipodal detection reuses the spherical kernel's `_spherical_kernel_point`, so it splits exactly the edges `arc_extent` rejects. Applicable through `fix` or directly on a geometry. Also record in the design doc that the lon-interval indexed spherical point locator (Task 16) is skipped — a pure optimization over the already-correct `rk_point_in_ring` fall-through. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add `test/methods/relateng/s2_crossings_conformance.jl`, ported from s2geometry `s2edge_crosser_test.cc` (`TEST(S2, Crossings)` and `CollinearEdgesThatDontTouch`). Maps S2 `CrossingSign` +1/-1/0 onto `rk_classify_intersection` `SS_PROPER`/`SS_DISJOINT`/`SS_TOUCH`, asserting the full mapping on the authoritative exact path and the conservative never-invent/never-deny contract on the float accelerator. Covers S2's numerically extreme cases (floating-point underflow, >2000-bit determinants). `CoincidentZeroLengthEdgesThatDontTouch` and `GetIntersection` are not ported (documented in the file): the former asserts S2's symbolic-perturbation convention, which this kernel's exact-on-actual-coordinates model legitimately diverges from; the latter has no analog since proper-crossing nodes are symbolic. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
1 participant
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.
Summary
Implements the
Spherical()manifold for the RelateNG engine, sorelate(RelateNG(; manifold = Spherical()), a, b)produces a DE-9IM on theunit sphere. Stacked on top of the
relatengport branch; this PR is just thespherical work (17 commits) for easier review.
kernel_spherical.jl): all 13rk_*predicates overUnitSphericalPoint{Float64}, each a sign ofdet(u,v,w) = (u×v)·w, with anexact path (
ExactPredicates.orient/Rational{BigInt}composites) and afloat path. Bulge-aware great-circle
arc_extent; S2 interior-on-leftpoint-in-ring via meridian-crossing parity; ring/axis containment via a
robust winding-number test (no atan2 branch ambiguity).
_kernel_point_type/_to_kernel_pointthrough the engine. Planar stays byte-identical(signed-zero
Tuple{Float64,Float64}); Spherical ingests unit xyz.build over 3D
arc_extents, so the tree and prepared paths match the nestedloop (a 2D endpoint box misses a long arc's bulge and prunes real crossings).
vertices;
AntipodalEdgeSplitis the opt-in geometry correction that insertsthe lon/lat midpoint as the remedy.
Engine generality (manifold-generic
apply/topology) was already in place fromthe port; this adds the spherical kernel and the point-type plumbing.
Test Plan
exact ∈ {True(), False()}spherical_end_to_end.jl: planar agreement on a mid-latitudepatch, pole-containing ring, tree/NestedLoop + prepared/unprepared
agreement, antipodal throw
AntipodalEdgeSplitcorrectionPtype parameter does not regress theplanar path)
test/methods/relateng/runtests.jlgreen (exit 0)🤖 Generated with Claude Code