From 83476ddf89032760305b9a7ea2aef80403bc8328 Mon Sep 17 00:00:00 2001 From: Brendan Collins Date: Wed, 8 Jul 2026 17:22:33 -0400 Subject: [PATCH 1/2] sweep-accuracy: pathfinding re-audit 2026-07-08 (#3646 HIGH, #3647 MEDIUM) Claude-Session: https://claude.ai/code/session_0155N4QGamQVxgpAAPbpQNq4 --- .claude/sweep-accuracy-state.csv | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.claude/sweep-accuracy-state.csv b/.claude/sweep-accuracy-state.csv index 2cee22dfc..5f6f509f2 100644 --- a/.claude/sweep-accuracy-state.csv +++ b/.claude/sweep-accuracy-state.csv @@ -24,7 +24,7 @@ mcda,2026-06-10,3146,MEDIUM,5,"Cat5 backend failures, all raise loudly (no wrong morphology,2026-04-30,"1397,1399",HIGH,2;5,HIGH fixed in #1397/PR #1398: morph_erode/dilate seeded centre cell into running min/max even when kernel[centre]==0 (all 4 backends). HIGH fixed in #1399/PR #1400: dask backends raised on 1xN/Nx1 kernels because empty-slice writeback (0:-0). multispectral,2026-03-30T14:00:00Z,1094,,, normalize,2026-05-01,,,,rescale and standardize across all 4 backends. NaN/inf filtered via isfinite mask before min/max/mean/std. Constant input handled (range=0 -> new_min; std=0 -> 0.0). Output dtype float64 consistently. Backend parity covered by test_matches_numpy. No accuracy issues found. -pathfinding,2026-07-03,3629;3630;3631,HIGH,1;3;5,"Cat1/3 HIGH+MEDIUM #3629: _get_pixel_id used int(abs(point-coord0)/cellsize) so out-of-bounds points on the coords[0] side folded to mirrored interior pixels (silent wrong path instead of ValueError) and truncation gave a half-cell floor bias plus fp flip at exact centers (0.3 on 0.1-res grid -> pixel 2); fixed with signed step + round-to-nearest-center. Cat5 HIGH+MEDIUM #3630: multi_stop_search crashed on dask+cupy (np.asarray(seg.values) hits cupy implicit-conversion TypeError) and returned numpy-backed output for cupy input while a_star_search preserves array type; note test_multi_stop_cupy_matches_numpy had a tautological conversion expression masking it (test-coverage sweep: no dask+cupy multi_stop test existed). Cat2/5 MEDIUM #3631: _hpa_star_search returned a partial finite cost trail when refinement failed (89 finite px on a wall-split 200x200 grid with unreachable goal) vs the all-NaN no-path contract elsewhere. Cat6 clean: a_star cost == scipy csgraph dijkstra on 8-connected 20x25 grid with anisotropic cells + random NaN barriers, friction and no-friction, delta 0.0; A->B==B->A symmetric; dask matches. Cat4: planar coordinate-unit distances (no haversine) is the library-wide convention, noted not flagged. CUDA available: cupy + dask+cupy paths executed (cupy is CPU-fallback by design, dask sparse A* verified vs numpy). HPA* suboptimality is inherent/documented, not flagged." +pathfinding,2026-07-08,3646;3647,HIGH,2,"Re-audit after 2026-07-03 sweep fixes (#3635/#3637/#3638) merged. Cat2 HIGH #3646: multi_stop_search(optimize_order=True) silently drops waypoints; _held_karp returns [start,end] when all tours are inf (best_last=-1), and _optimize_waypoint_order reads segment cost at the UNSNAPPED goal pixel so snap=True makes reachable waypoints inf-distance; repro: wall grid raises without optimize_order but returns finite 2-waypoint route with it; snap case drops a reachable waypoint. Cat2 MEDIUM #3647: _hpa_star_search routes the coarse grid with the caller's barriers but coarse cells are block MEANS, so a mean equal to a barrier value (e.g. -1/+1 data, barriers=[0]) falsely blocks passable regions -> all-NaN no-path; reproduced helper-level and end-to-end via auto-radius HPA* (600x600, mocked RAM); fix routes coarse grid with empty barriers (NaN coarse cells already encode impassable blocks). Cat6 clean: a_star cost == scipy csgraph dijkstra (8-conn 20x25, anisotropic cells, NaN barriers, friction and no-friction, delta 0.0). Cat1/3/4 clean; Cat5: all 57 pathfinding tests pass incl. cupy and dask+cupy (CUDA available, executed)." perlin,2026-04-10T12:00:00Z,,,,Improved Perlin noise implementation correct. Fade/gradient functions verified. Backend-consistent. Continuous at cell boundaries. polygon_clip,2026-06-10,3186,HIGH,5,"Cat5 backend inconsistency: dask+cupy clip_polygon rasterizes the mask with a uniform chunk size from the raster's first chunk, then feeds raster+mask to da.map_blocks (positional block pairing). Non-uniform raster chunks gave the mask a different block layout -> IndexError/ValueError (or silent mis-stamp). Repro (8,6) rechunk ((3,5),(6,)) on dask+cupy raised ValueError Shapes do not align; dask+numpy was fine via xarray.where rechunk. Fix #3186/PR: rechunk cond to raster.data.chunks[-2:] before map_blocks; added non-uniform regression tests for dask+numpy and dask+cupy. use_cuda->gpu migration in that branch was already landed by #3089/#3122. CUDA available; cupy+dask+cupy verified, 25 tests pass. Cats 1-4 clean: numpy path uses raster.where, cupy path operates on raw arrays, NaN inputs preserved, no neighborhood ops/curvature. Prior fix #1197/#1200 (crop+all_touched) merged and unrelated." polygonize,2026-05-29,2606,HIGH,5,"Cat 5 HIGH: dask connectivity=8 cross-chunk merge filled diagonal notch where same-value regions meet only at a corner across a chunk boundary; total area exceeded raster. Hole ring was dropped because containment tested hole[0] (on exterior at pinch). Fixed via _ring_interior_point in PR for #2606. numpy, dask+numpy, dask+cupy area parity now holds; 4-conn was already correct. cupy + dask+cupy paths validated on GPU host. Other cats clean: NaN masked on numpy/cupy float paths (tested), _is_close handles +/-inf via exact-equality short-circuit, atol/rtol/simplify_tolerance reject NaN/inf, integer GPU CCL matches numpy." From a0b871f31d7a51048cbc75dff4279d0c64e7a46a Mon Sep 17 00:00:00 2001 From: Brendan Collins Date: Wed, 8 Jul 2026 17:24:03 -0400 Subject: [PATCH 2/2] Raise on infeasible tours and honor snap in multi_stop_search optimize_order (#3646) _held_karp returned [start, end] when every full tour was infinite (best_last stayed -1), so multi_stop_search(optimize_order=True) silently dropped interior waypoints instead of raising like the optimize_order=False path. Raise ValueError when the exact solver's total cost is infinite. The pairwise distance matrix also read each segment's cost at the unsnapped goal pixel, so with snap=True any waypoint sitting on an invalid cell got an infinite distance and fell into the same hole. Use the max-finite-cost pixel as the true goal, matching the segment loop. Claude-Session: https://claude.ai/code/session_0155N4QGamQVxgpAAPbpQNq4 --- xrspatial/pathfinding.py | 19 +++++++++- xrspatial/tests/test_pathfinding.py | 56 +++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 1 deletion(-) diff --git a/xrspatial/pathfinding.py b/xrspatial/pathfinding.py index 3c1a4199c..05d52ad69 100644 --- a/xrspatial/pathfinding.py +++ b/xrspatial/pathfinding.py @@ -1344,13 +1344,30 @@ def _optimize_waypoint_order(surface, waypoints, barriers, x, y, ) seg_vals = _segment_to_numpy(seg.data) goal_py, goal_px = _get_pixel_id(waypoints[j], surface, x, y) + # If snap is on, the actual goal pixel may differ from the + # requested one; read the cost at the true (snapped) goal, + # which is the max-finite-cost pixel, like the segment loop. + if snap and not np.isfinite(seg_vals[goal_py, goal_px]): + finite = np.isfinite(seg_vals) + if finite.any(): + max_idx = np.nanargmax(seg_vals) + goal_py, goal_px = np.unravel_index( + max_idx, seg_vals.shape) goal_cost = seg_vals[goal_py, goal_px] if np.isfinite(goal_cost): dist[i][j] = goal_cost # Fixed endpoints: first=0, last=n-1 if n <= 12: - order, _ = _held_karp(dist, 0, n - 1) + order, total = _held_karp(dist, 0, n - 1) + # An infinite total means no ordering visits every waypoint + # (some waypoint is unreachable). Held-Karp's reconstruction + # would return only [start, end], silently dropping the + # interior waypoints, so raise instead. + if not np.isfinite(total): + raise ValueError( + "optimize_order: no feasible route visits all waypoints " + "(some waypoints are unreachable from the others)") else: order, _ = _nearest_neighbor_2opt(dist, 0, n - 1) diff --git a/xrspatial/tests/test_pathfinding.py b/xrspatial/tests/test_pathfinding.py index 0db74e791..e43dcf7da 100644 --- a/xrspatial/tests/test_pathfinding.py +++ b/xrspatial/tests/test_pathfinding.py @@ -1044,6 +1044,62 @@ def test_optimize_order_finds_better_route(): assert optimized.attrs['total_cost'] <= naive.attrs['total_cost'] + 1e-10 +def test_optimize_order_unreachable_waypoint_raises(): + """Unreachable interior waypoint raises instead of being dropped (#3646). + + Without optimize_order the segment loop raises "no path between + waypoints"; with optimize_order the infeasible tour used to make + _held_karp return only [start, end], silently dropping the interior + waypoint and returning a finite route. + """ + data = np.ones((8, 8)) + data[4, :] = np.nan # wall: bottom rows unreachable from top rows + + agg = _make_raster(data) + + wp0 = (7.0, 0.0) # pixel (0, 0), above the wall + wp_mid = (0.0, 0.0) # pixel (7, 0), below the wall (unreachable) + wp_end = (7.0, 7.0) # pixel (0, 7), above the wall + + with pytest.raises(ValueError, match="unreachable"): + multi_stop_search(agg, [wp0, wp_mid, wp_end], optimize_order=True) + + +@pytest.mark.filterwarnings("ignore:End at a non crossable location:Warning") +@pytest.mark.filterwarnings("ignore:Start at a non crossable location:Warning") +def test_optimize_order_with_snap_keeps_waypoints(): + """optimize_order must not drop waypoints that need snapping (#3646). + + The pairwise distance matrix used to read the segment cost at the + unsnapped goal pixel (NaN when the waypoint sits on an invalid cell), + so every snapped waypoint got an infinite distance and was dropped + through the infeasible-tour hole. + """ + data = np.ones((8, 8)) + data[3, 3] = np.nan # single invalid cell; snap moves off it + + agg = _make_raster(data) + + wp0 = (7.0, 0.0) # pixel (0, 0) + wp_mid = (4.0, 3.0) # pixel (3, 3) -> NaN cell, needs snapping + wp_end = (0.0, 7.0) # pixel (7, 7) + + result = multi_stop_search( + agg, [wp0, wp_mid, wp_end], snap=True, optimize_order=True) + + order = result.attrs['waypoint_order'] + assert len(order) == 3 + assert len(result.attrs['segment_costs']) == 2 + assert tuple(order[0]) == wp0 + assert tuple(order[-1]) == wp_end + + # Same route as the unoptimized call (the input order is already + # optimal here), so costs must match too. + plain = multi_stop_search(agg, [wp0, wp_mid, wp_end], snap=True) + np.testing.assert_allclose( + result.attrs['total_cost'], plain.attrs['total_cost'], atol=1e-10) + + def test_optimize_order_preserves_endpoints(): """First and last waypoints should remain fixed after optimization.""" data = np.ones((10, 10))