Skip to content

Add pathfinding test coverage: dask edge handling, degenerate shapes, parameter branches#3665

Open
brendancol wants to merge 2 commits into
mainfrom
deep-sweep-test-coverage-pathfinding-2026-07-08
Open

Add pathfinding test coverage: dask edge handling, degenerate shapes, parameter branches#3665
brendancol wants to merge 2 commits into
mainfrom
deep-sweep-test-coverage-pathfinding-2026-07-08

Conversation

@brendancol

Copy link
Copy Markdown
Contributor

Closes #3657

Test-only change; no source edits. Adds 30 tests to test_pathfinding.py, taking measured branch coverage of xrspatial/pathfinding.py from 86% to 96% (57 -> 87 tests, all passing).

  • Dask edge handling: the dask backend is its own pure-Python A* (_a_star_dask), and none of its barrier, NaN-wall, no-path, non-crossable-start/goal, or friction-blocked-start branches had a test. New tests assert parity with numpy on a barrier grid plus each of the empty/warning outcomes.
  • Degenerate shapes: 1x1, 1xN, and Nx1 rasters on numpy, dask, and cupy.
  • Parameter and error branches: invalid connectivity, friction shape mismatch (both functions), friction has no positive finite values on numpy/dask/cupy, search_radius combined with friction, multi_stop_search(snap=True) goal remap, optimize_order warning, waypoint length check, snap+dask raise, numpy friction with dask surface.
  • CuPy: snap, explicit search_radius, auto-radius under mocked low memory, and non-crossable warnings.
  • HPA*: _hpa_star_search with a friction surface, plus a direct _coarsen_friction check (that kernel had zero coverage).

Backend coverage: numpy, dask+numpy, cupy, and dask+cupy paths all exercised; the cupy tests ran for real on a CUDA box.

Deliberately not tested here: the dims-mismatch error message (being reworded in #3649) and the LOW-severity branches listed in the issue (chunk-cache eviction, TSP shortcut branches, non-square cellsizes).

Test plan:

  • pytest xrspatial/tests/test_pathfinding.py — 87 passed, GPU tests included
  • NUMBA_DISABLE_JIT=1 pytest --cov=xrspatial.pathfinding --cov-branch — 96%
  • flake8 on the test file: no new findings (pre-existing ones untouched)

https://claude.ai/code/session_0155N4QGamQVxgpAAPbpQNq4

…parameter branches (#3657)

Branch coverage 86% -> 96%. New tests: dask barrier/NaN/no-path/warning
paths, 1x1 and strip rasters on all backends, friction and search_radius
parameter branches, cupy snap/auto-radius (run on GPU), HPA* with
friction. Test-only change.

Claude-Session: https://claude.ai/code/session_0155N4QGamQVxgpAAPbpQNq4

@brendancol brendancol left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review: Add pathfinding test coverage: dask edge handling, degenerate shapes, parameter branches

Test-only PR (plus the sweep state CSV). No source changes, so the review focuses on whether the new tests are correct, deterministic, and actually exercise the paths they claim to.

Blockers (must fix before merge)

None.

Suggestions (should fix, not blocking)

  • The new dask edge tests (barriers, NaN wall, non-crossable warnings, blocked start) all run on dask+numpy only. The dask+cupy path shares _a_star_dask but takes the is_cupy=True branch in _load_chunk (block.get()) and builds output blocks through _make_block_cupy. One barrier-grid parity test on dask+cupy would close that last gap cheaply (xrspatial/tests/test_pathfinding.py:1210, pattern already exists in test_dask_cupy_matches_numpy).

Nits (optional improvements)

  • test_degenerate_shapes_cupy covers 1x1 and 1x5 but not 5x1 (xrspatial/tests/test_pathfinding.py:1330). The cupy backend transfers to the same numpy kernel, so the risk is low; adding the column case would make the three shape tests symmetric across backends.
  • test_hpa_star_with_friction asserts the goal is reached but not that friction influenced the route (xrspatial/tests/test_pathfinding.py:1520). Hard to assert tightly since HPA* is heuristic, and the exact-value _coarsen_friction check compensates. Fine as is; noting it so nobody mistakes it for a routing-quality test.

What looks good

  • Every test in the diff was verified by execution: 87 passed with JIT enabled, and the coverage claim (86% -> 96% branch) reproduces with NUMBA_DISABLE_JIT=1.
  • The dask tests target the right thing: _a_star_dask is a separate pure-Python implementation, and the new barrier-parity test compares it against the numpy kernel on the same grid rather than just checking it runs.
  • Expected values are concrete where they can be: unit-step costs on strips, exact _coarsen_friction block mean of 2.0, all-NaN no-path contracts.
  • The memory-mock tests patch _available_memory_bytes with values chosen so the auto-radius arithmetic is deterministic (checked: 20x20x65 = 26000 against the 20000/32000 thresholds).
  • Deliberate exclusions are documented: the dims-mismatch message test is deferred because #3649 rewords it, which avoids a cross-PR conflict.
  • Pre-existing flake8 findings in the file were left untouched; the new code adds none.

Checklist

  • Algorithm matches reference/paper (n/a, test-only)
  • All implemented backends produce consistent results (numpy/dask/cupy/dask+cupy parity tests run locally)
  • NaN handling is correct (NaN wall, NaN friction, snap-over-NaN all covered)
  • Edge cases are covered by tests (1x1, 1xN, Nx1 added)
  • Dask chunk boundaries handled correctly (barrier grid tested at (2,2) chunks)
  • No premature materialization or unnecessary copies (n/a, test-only)
  • Benchmark exists or is not needed (benchmarks tracked separately in #3645 / PR #3648)
  • README feature matrix updated (n/a, no API change)
  • Docstrings present and accurate (each test has a one-line docstring)

@brendancol

Copy link
Copy Markdown
Contributor Author

Follow-up 155a1da addresses the review: added test_dask_cupy_barriers_match_numpy (barrier parity through the cupy chunk-conversion branch of _a_star_dask) and the 5x1 column case in test_degenerate_shapes_cupy. 88 tests pass locally, GPU tests included. The HPA* nit is left as documented.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

pathfinding: dask barrier/NaN handling, degenerate shapes, and parameter branches lack test coverage

1 participant