Benchmark the surface_distance paths that were never timed (#3709) - #3717
Open
brendancol wants to merge 1 commit into
Open
Benchmark the surface_distance paths that were never timed (#3709)#3717brendancol wants to merge 1 commit into
brendancol wants to merge 1 commit into
Conversation
The benchmark file covered one of six compute paths, and the path it did cover ran on a source raster where 99.95% of pixels were sources, so the Dijkstra relaxation body never executed. - parameterize cupy and dask+cupy alongside numpy and dask, matching pathfinding.py - replace the near-uniform integer source raster with scattered point sources so the frontier crosses the grid - add time_surface_distance_bounded with a finite max_distance whose pixel radius stays inside one chunk, which routes the dask backends through map_overlap instead of the iterative tile fallback - call .compute() on dask results, following the convention in flood.py, twi.py and interpolate.py - add a numpy-only geodesic class for _dijkstra_geodesic and _precompute_dd_grid Benchmark file only, no source changes. All 50 parameter combinations were run on this host, 13s for a single pass.
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.
Closes #3709.
benchmarks/benchmarks/surface_distance.pywas 26 lines against a 1561-line module. It timed one of six compute paths, and it timed that one on input where the algorithm barely runs.What changed
Benchmark file only. No source changes.
cupy and dask+cupy are now parameterized.
_surface_distance_cupy(with the@cuda.jit_sd_relax_kernel) and_surface_distance_dask_cupyhad no coverage at all.paramsnow reads["numpy", "cupy", "dask", "dask+cupy"], matchingpathfinding.py::AStarSearch. Thedask+cupytype came intocommon.get_xr_dataarraywith #3706.The bounded dask branch is now reached.
_surface_distance_daskpicks betweenmap_overlapand the iterative tile Dijkstra based onmax_distance; every existing benchmark call used the defaultnp.infand landed on the iterative fallback, emittingusing iterative tile Dijkstraeach time.time_surface_distance_boundedpicks amax_distancewhose pixel radius stays inside one chunk, following the arithmetic and the comment already incost_distance.py. Verified by asserting the warning fires for the unbounded method and not for the bounded one:Dask results are computed. The old file never called
.compute(), so the dask numbers were real only because_sd_dask_iterativehappens to evaluate eagerly while building its graph. A bounded benchmark added the same way would have measured 5 ms of graph construction.flood.py,twi.py,interpolate.pyandflow_length.pyall use theif hasattr(result.data, 'compute')guard; this file now does too.Source pixels are sparse.
get_xr_dataarray(is_int=True)drawsrng.integers(-nx, nx), and_seed_sourcestreats any non-zero finite pixel as a target, so at nx=1000 499769 of 500000 pixels seeded the search at distance zero.new_cost < dist[vr, vc]was never true and the relaxation body never executed. Maximum distance found was 3.62 map units on a 360x180 degree grid. With scattered point sources it is 160.44, and the call is 6.4x slower.Geodesic has its own class.
_dijkstra_geodesicand_precompute_dd_gridwere untouched; they cost about 2.2x the planar path. numpy only, since the module raisesNotImplementedErrorfor geodesic on cupy, dask, and dask+cupy.Verification
All 50 parameter combinations executed on this host (which has CUDA and cupy), 13.01s for a single pass:
xrspatial/tests/test_surface_distance.pyis 37 passed. flake8 clean.Verification was by direct invocation of the benchmark classes rather than
asv run, becauseasv discovercurrently fails suite-wide in this conda environment on anasv_runnermetadata bug under Python 3.14 (dist.metadata["Name"]is None for some installed distribution). It fails identically on unmodified files such asCostDistance, so it is an environment problem, not something this branch introduces.One thing to expect
Changing the source pattern will show as a one-time step change in the asv history for
time_surface_distance,time_surface_allocationandtime_surface_direction. The old numbers were timing a heap drain.Not fixed here
nx=100gives a 50x100 grid where setup dominates. The ladder up to nx=1000 still shows scaling, so it is recorded in the sweep state rather than changed.Found by
/sweep-benchmarks.