Add SpatialIndex for fast spatial queries - #118
Conversation
|
Some additional background on this-- I maintain two libraries: zagg which aggregates input geospatial data to zarr grids, and mortie , which implements healpix / morton spatial indexing. The main usecase for To do the 'map CMR geometries' to worker step, there's a few options:
Option 1 was previously fast, but buggy and had omission errors; fixing these has made it less fast. It also requires a parameter to be set on the size of the grid cells for the intersection, which impacts run speed and also the commission error rate (i.e., how many false positives due to over-sized grid cells). Option 2 is fast, but doesn't work globally-- euclidean geometry breaks down at the poles. We have to reproject to a stereographic projection to build Spatial Range Tree in that metric space... this is a hassle. It's either automatic and brittle, or, differed to the user to select and subject to user error (and less usable overall). Option 3 works today, is correct (no omission or commission), and doesn't require any parameters to be set-- but it doesn't scale well. Individual polygon checks are fairly fast given that the routines are compiled with low constants, but when doing a NASA CMR catalog subset we're hitting 10's or 100's of thousands of geometries, so the actual algorithmic scaling starts to matter. Hence this PR, which implement a spatial index with things like intersect and contains predicates. The test case that I benchmarked this against is below, and builds a search across 76,575 polygons: Backend comparison — cycle-22 ATL06 (76,575 pairs)mortie 0.7.2 MOC sweep (the espg/mortie#33 fix → zero omission at every order):
As you can see above, having the spatial index drops the intersection time from about a minute, to 2 seconds (and unlike mortie doesn't require any parameter settings). |
|
Note that the three failing checks (ci/environment-dev.yml, Python 3.14, ubuntu/macos/windows) are unrelated to this PR — they fail in an upstream dependency build before any spherely code is compiled. Those legs build s2geography main from source against an unpinned s2geometry>=0.11.1. conda-forge published s2geometry 0.14.0 on 2026-06-06, and s2geography main doesn't compile against it — 0.14.0's header layout breaks the s2/base/port.h include chain (via s2coords_internal.h), failing while compiling s2geography's own accessors-geog.cc: This is independent of the change here: the dev legs passed as recently as the June 3 dependabot run (which resolved a pre-0.14 s2geometry); they broke the moment 0.14.0 landed, and this PR was simply the first CI run afterward. Any PR run now hits the same wall. The ci/environment.yml legs stay green across 3.10–3.14 because they use the conda s2geography 0.2.0 package, whose metadata keeps s2geometry on a compatible version. |
|
@benbovy is there anything I can do to move this along towards a merge? I was thinking that |
benbovy
left a comment
There was a problem hiding this comment.
Thanks @espg, this is a great addition!
Sorry for the slow review, I'm not very active on this repository and I missed your initial submission a few weeks ago.
I left a few comments, overall this looks great! @jorisvandenbossche do you want to take a look at it?
I was thinking that query_nearest / distance queries via S2ClosestEdgeQuery would make sense as a follow up to this.
Yes it is perfectly fine to keep that for a follow-up PR.
Note that the three failing checks (ci/environment-dev.yml, Python 3.14, ubuntu/macos/windows) are unrelated to this PR — they fail in an upstream dependency build before any spherely code is compiled.
Those have been fixed in #119. Could you merge the main branch here please?
I maintain two libraries: zagg which aggregates input geospatial data to zarr grids, and mortie , which implements healpix / morton spatial indexing.
Thanks for providing additional context! Actually I've been working recently on a similar project that converts other data sources to HEALPix. It is not yet publicly released but I'd be happy to discuss it once it is.
| ** The input geographies are held as a numpy object-dtype array, which both | ||
| ** keeps the underlying C++ Geography objects (whose S2Shapes are borrowed by | ||
| ** the index) alive and backs the ``geometries`` property. | ||
| */ |
There was a problem hiding this comment.
| ** The input geographies are held as a numpy object-dtype array, which both | |
| ** keeps the underlying C++ Geography objects (whose S2Shapes are borrowed by | |
| ** the index) alive and backs the ``geometries`` property. | |
| */ | |
| */ |
Not very informative.
|
|
||
| // Dispatch between scalar (single Geography -> 1-d index array) and | ||
| // array-like (-> (2, K) array of (input index, tree index) pairs) queries. | ||
| py::object query(py::object geography, std::optional<std::string> predicate) const { |
There was a problem hiding this comment.
Could use function overloading instead?
py::array_t<py::ssize_t> query(PyObjectGeography geography, std::optional<std::string> predicate) const
and
py::array_t<py::ssize_t> query(py::array_t<PyObjectGeography> geographies, std::optional<std::string> predicate) const
| throw py::type_error("geographies must be a 1-dimensional array"); | ||
| } | ||
|
|
||
| m_geographies = arr; |
There was a problem hiding this comment.
Note: a shallow copy of the input Geography objects is safe as long as we don't support in-place coordinate replacement. Currently we don't support it but if we eventually do so - rather unlikely I guess - we'll probably need deep-copy here (i.e., clone the underlying S2 objects).
There was a problem hiding this comment.
Agreed — the index borrows the S2Shapes from the input geographies, so in-place coordinate replacement would invalidate it. If that ever lands, cloning the underlying S2 objects here (and rebuilding on mutation) would be needed.
Since Geography is currently move-only/immutable from Python, the shallow copy stays, but in the same spirit the constructor now stores a shallow copy() of the input array itself: previously, replacing an element of the caller's array after building the index could drop the last reference to a Geography whose S2Shapes the index still borrows. (shapely's STRtree has the same exposure via its geometries attribute, so this only closes the input-array half, but it's a one-liner.) Added a test for it.
|
|
||
| // Return the sorted tree indices whose cells overlap the query geography, | ||
| // optionally refined by ``pred`` (predicate(query, candidate)). | ||
| std::vector<int> query_one(Geography* query_geog, const PredicateFunc* pred) const { |
There was a problem hiding this comment.
| std::vector<int> query_one(Geography* query_geog, const PredicateFunc* pred) const { | |
| std::vector<int> query_one(const Geography* query_geog, const PredicateFunc* pred) const { |
(or use const references)
There was a problem hiding this comment.
Done — it now takes const Geography&. This required making Geography::geog_index()
const, since it lazily constructs the ShapeIndexGeography on first use; the cached
pointer in geography.hpp is now mutable (pure caching, no observable state change).
| assert result.shape[0] == 2 | ||
| # (input_index, tree_index) pairs | ||
| pairs = {(int(a), int(b)) for a, b in zip(result[0], result[1])} | ||
| assert pairs == {(0, 1), (1, 3)} |
There was a problem hiding this comment.
I guess we could simply use np.testing.assert_array_equal instead of sets. IIUC query results are sorted.
There was a problem hiding this comment.
query results are indeed sorted (scalar queries) and in deterministic
input-major order (array queries), so the exact expected arrays are asserted directly now
| PredicateFunc get_predicate(const std::string& name) { | ||
| using Index = s2geog::ShapeIndexGeography; | ||
|
|
||
| if (name == "intersects") { |
There was a problem hiding this comment.
C++ can't switch on std::string, so this now goes through a small
unordered_map<std::string, PredicateId> lookup followed by a switch on the enum —
which also gets us -Wswitch coverage if a predicate is ever added to the enum but not
handled.
If you'd rather keep the simpler if/else chain, happy to revert.
There was a problem hiding this comment.
It's backed by s2geography::GeographyIndex (a MutableS2ShapeIndex), which is already available in the pinned s2geography 0.2.0 — no upstream or dependency changes needed.
s2geography 0.2.0 is indeed pinned in CI for the release, but currently there's no minimal version constraint in the CMake configuration. We should probably do it at some point.
In the meantime, for consistency could you move src/predicates_common.cpp and src/spatial_index.cpp under the if clause below alongside geoarrow and projections, please?
There was a problem hiding this comment.
Done — src/predicates_common.cpp and src/spatial_index.cpp now sit alongside
geoarrow.cpp and projections.cpp in the VERSION_GREATER_EQUAL "0.2.0" clause.
Agreed a minimum version constraint in the CMake config would be good at some point.
194c6bd to
7b26048
Compare
|
Thanks for the review @benbovy! My turn to apologize for the slow response; got sucked into the summer holidays earlier in the month.
Would enjoy that! If a call is ever easier than issue threads (and doable with the time difference), feel free to grab a slot on my calendly On the HEALPix side, I just finalized the 1.0 morton indexing spec — if you happen to take a look, I'd be really interested in your thoughts. Like I mentioned, zagg is the writer side of all this, but I'm starting to tackle the reader side now with a small library, moczarr, which is a sparse-DGGS xarray reader for zarr stores of HEALPix cells under mortie's packed z-order ("morton") encoding, with MOC-declared coverage. It plugs into xdggs through the public registry ( This All comments are addressed in the latest push, and the branch is rebased on Most suggestions were applied as-is; a few were adapted slightly for C++/pybind11 reasons
The refinement test is also reworked so the coarse candidate set is a strict superset of the predicate-refined result (a point outside a triangle but inside its cell covering), so the test now actually exercises the predicate filtering. |
|
Pushed one more change to this branch: a This came up while running on larger spatial collections and running into memory bounded slow downs... the type of case where you'd want to build and persist the index. I'll post another follow PR shortly for spatial index persistence and go over the timings in more detail there. What was added
index = spherely.SpatialIndex(geographies)
assert not index.is_built # the constructor only queues the shapes
index.build() # build now, under the default budget
assert index.is_built
big = spherely.SpatialIndex(lots_of_geographies)
big.build(tmp_memory_budget=4 * 1024**3) # ...or under a 4 GiB budgetThere is also a read-only The argument is in bytes, matching the unit of the underlying s2geometry setting ( The knob is only useful raised. A budget below the one in effect splits the build into more batches and makes it slower — 1 MB measured about ten times slower than the default on a 634k-edge collection — and the unit is easy to get wrong in the expensive direction: Why it's added hereAny user who indexes a large collection hits this today, and hits it in a confusing place: the constructor looks fast and then an unrelated-looking first query hangs. The default temp memory is bad at scale-- each batch re-absorbs the index cells built so far, so many batches therefore cost superlinearly rather than additively. (Query time is linear and has nothing to do with any of this.) Measured on 555,867 real ICESat-2 ATL03 footprint polygons (17.68M vertices), single-threaded, M1 Max —
19.5× on the build, and the fitted complexity drops from N^1.79 to N^1.08 — i.e. it goes from superlinear to essentially linear in edge count.
edit: Both arms return the same predicate-refined results — the 190,625 pairs above are from predicate="intersects", byte-identical, same SHA-256. The unrefined candidate set is not stable across budgets: s2 batches differently, and candidate lists at different budgets are mutually incomparable (candidates can appear and disappear). What is invariant is the set of true matches, so any query with a predicate is unaffected. A second The cost is transient memory: peak RSS goes from 2.6 GB to 4.3 GB at this N, since the larger budget is exactly a licence to hold more scratch at once. Peak RSS can therefore rise by roughly the budget granted, and s2 needs on the order of 226 bytes of temporary memory per edge — so |
Addresses #72
Summary
Adds
spherely.SpatialIndex, a spatial index over a collection ofGeographyobjects for fast candidate retrieval — conceptually similar to shapely'sSTRtree. Without it, spatial joins are O(N×M) brute-force over the vectorized predicates; this lets callers cheaply pre-filter candidate pairs.It's backed by
s2geography::GeographyIndex(aMutableS2ShapeIndex), which is already available in the pinned s2geography 0.2.0 — no upstream or dependency changes needed.API
predicate=Nonereturns the coarse cell-overlap candidate set (a superset of true intersections)."intersects","within","contains","covers","covered_by","touches","equals"— each a subset of the candidate set, with semantics matching the existing scalar predicates (a tree geometrytmatches whenpredicate(query_geom, t)is True)."disjoint"is rejected withValueError(it's the complement of the candidate set, so it can't be answered from the index alone); unknown predicate names also raiseValueError.Implementation notes
src/spatial_index.cpp— theSpatialIndexbinding. Builds the index withGeographyIndex::Add(geog, i); queries compute a covering viaS2RegionCovereroverGeography::Region()and walkGeographyIndex::Iterator::Query. The input geographies are held as a numpy object-dtype array, which both keeps the underlying C++Geographyobjects alive (the index only borrows theirS2Shapes) and backs thegeometriesproperty.src/predicates.hpp+src/predicates_common.cpp— a small sharedget_predicate(name)factory returning a closure over twoShapeIndexGeography, reusing the sames2geog::s2_*calls /S2BooleanOperation::Optionsaspredicates.cpp(which remains the reference for the vectorized predicates).src/spherely.cppandCMakeLists.txt; type stubs insrc/spherely.pyi; docs indocs/api.rstanddocs/api_hidden.rst.Out of scope (possible follow-ups)
query_nearest/ distance queries viaS2ClosestEdgeQuery.max_cells) and a dedicated point-only fast path (S2PointIndex).Testing
tests/test_spatial_index.py(10 tests): build /len/geometries, coarse vs. predicate-refined queries, array(2, K)layout, empty-geography handling, andValueErrorcases.mypyandpre-commit(black + clang-format) clean.