Add spherical arc extents and manifold-aware Extents.extent#434
Open
asinghvi17 wants to merge 6 commits into
Open
Add spherical arc extents and manifold-aware Extents.extent#434asinghvi17 wants to merge 6 commits into
Extents.extent#434asinghvi17 wants to merge 6 commits into
Conversation
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>
spherical_arc_extent and spherical manifold edge decompositionExtents.extent
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>
46c76e2 to
5ee72be
Compare
Co-Authored-By: Claude Fable 5 <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
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.
Makes spherical extents first-class, in three layers. Follow-up to #432.
spherical_arc_extent— new public API inUnitSphericalspherical_arc_extent(a, b)returns the 3D CartesianExtent{(:X, :Y, :Z)}of the shorter great-circle arc between two points. An arc bulges out of its endpoints' bounding box (two points atz = 0.9joined over the pole reachz = 1), so unioning endpoint extents is not a valid bound.The computation is trig-free: with
t̂ₐ = normalize(n × a)the unit tangent along the arc (normal fromrobust_cross_product, so nearly-degenerate/antipodal arcs stay stable), each coordinate along the arc is a sinusoid with amplitudehypot(aᵢ, t̂ₐᵢ); an interior extremum on axisiexists iff the coordinate rises ataand falls atb. Bounds get a 4-ulp pad guaranteeing containment, in the spirit of S2'sS2LatLngRectBounder. (Unlike S2, no error-budget branch is needed on the extremum sign test: the amplitude is computed from the endpoint itself, so a sign misjudged within fp noise costs only a quadratically small sliver, already inside the pad — S2'smax_latis a whole-great-circle bound, which is why it needs the "latitude budget".)Manifold-aware edge decomposition
eachedge,to_edgelist, andlazy_edgelistgain manifold-first methods:Spherical()yields edges asUnitSphericalPointpairs (converting geographic input) whoseGI.Lines carry arc extents. Rings already made ofUnitSphericalPoints take this path with no manifold argument — previously they threw aMethodErrorfrom the 2D-hardcoded_lineedge.Manifold-aware
Extents.extent— the generic ring bboxExtents.extent(m::Manifold, geom, [::Type{T} = Float64]). GO does not exportextent(Extents.jl already exports that name); since the manifold is our type, the methods extendExtents.extentdirectly and are reachable asGO.extent:Planar()delegates toGI.extent.Spherical(): points → degenerate boxes;LineStrings → union of edge arc extents (exact — a curve has no interior); rings and polygons → regions. An extremum of a coordinate over a region is attained on the boundary or at one of the six on-sphere critical points(±1,0,0), (0,±1,0), (0,0,±1)— a cell enclosing a pole extends past every edge extent (the caveat from the rtreeify sweep, and why odd-h_elemcubed-sphere panels break with edge-only extents: the six points are exactly the six panel centers).s2loop.h): "All loops are defined to have a CCW orientation, i.e. the interior of the loop is on the left side of the edges. This implies that a clockwise loop enclosing a small area is interpreted to be a CCW loop enclosing a very large area." A CW cap ring therefore yields the complement's extent, exactly per S2.S2Loop::InitBound/Containsdecide pole containment (s2loop.cc): pick an anchor edge whose great circle misses the query pointq; which side of itqlies on is the side the arc from the anchor's midpoint toqdeparts into (left = interior; the arc can re-meet that great circle only at the midpoint's antipode, out of reach for an arc under a half turn), and each transversal boundary crossing along the arc flips it. Boundary points count as contained.spherical_orient's tolerance band and retries with the next anchor edge; if every anchor is degenerate the axis extends to±1— possibly loose, never under-covering. Two systematic configurations resolve instead of retrying, since no anchor can fix them: a query on an edge's great circle but off the edge contributes zero crossings (lonlat meridian edges pass through±eₓ/±e_yexactly), and a ring vertex exactly antipodal to the query contributes zero (polar lonlat cells, whose pole vertex sits on every test circle through the far pole).Compared to the winding-number + signed-area heuristic this replaces, parity is a per-point answer with no preconditions: the two previously documented approximations (single edges sweeping more than a half turn about an axis; regions containing both poles at small area) are gone by construction — the new dumbbell test (both polar caps joined by a thin corridor) is exactly the case winding could not see.
Also fixed
to_unit_spherical_pointsreinterpreted the first two Cartesian coordinates ofUnitSphericalPointinput as geographic lon/lat (its docstring claimed pass-through). It now maps points through theUnitSphericalPointconstructor, which actually passes them through.Tests (~1,000 new assertions)
spherical_arc_extent: equatorial exactness, over-the-pole bulge, geographic parity, degenerate arcs, 100 random arcs × dense slerp containment incl. nearly-degenerateSpherical()and USP-ring paths agree, lazy variant agrees,NaturalIndex/RTreeindex in 3D, bulge-only R-tree query regressionextent: CCW cap covers the pole; CW cap = complement (X/Y extend to ±1, Z min to −1, Z max stays on boundary); geographic polar polygon; larger-than-hemisphere cap; pole-vertex cells; dumbbell region enclosing both poles through a thin corridor; all 12 polar lonlat cells (exact pole vertex + duplicate point, no far-pole leak); axis point on the boundary clamps that bound to exactly±1; multis/collections; LineStrings stay curves; 50 random cells × dense boundary/interior samples + axis-point coverage via the star-shaped inradius criterion + reversed-ring duality (complement box covers outside axis points; the pair of boxes covers all six)🤖 Generated with Claude Code