Validate barriers, search_radius, start/goal points, and dims in pathfinding

[brendancol]

Closes #3649

• Reject non-numeric and non-1-D barriers before any kernel runs. barriers=['0'] used to silently disable every barrier (numba compares float cells to unicode, always False) and route straight through walls; barriers=0 died with a numba TypingError. Numeric values are now coerced to float64.
• Reject search_radius values that are not None or a non-negative integer. Negative radii used to return a silent all-NaN "no path" on numpy/cupy (a misleading "no path between waypoints" through multi_stop_search), or crash with "negative dimensions are not allowed" / "slice indices must be integers" depending on where start and goal sit.
• Add the dims check to multi_stop_search (was a bare KeyError: 'y' from xarray internals) and rewrite the shared message to name the actual dims and the x=/y= parameters.
• Validate start/goal are 2-element pairs in a_star_search, matching the existing waypoint check, and make that check survive scalar waypoints in multi_stop_search.

Backend coverage: the new checks run before backend dispatch, so behavior is identical on numpy, cupy, dask+numpy, and dask+cupy. Verified by executing the bad-input battery on all four (CUDA host).

Exception types on already-raising paths are unchanged. The two silent cases (string barriers, negative radius) now raise, which only affects calls that returned wrong output.

Test plan:

• pytest xrspatial/tests/test_pathfinding.py (71 passed, includes cupy and dask+cupy on this host)
• New TestPathfindingErrorHandling class: string/scalar barriers, negative/float/zero radius, scalar start/goal/waypoint, dims mismatch in both functions, custom dims still work, numeric barriers still block
• flake8 diff vs main: no new findings

State-CSV row for the error-handling sweep is included.

https://claude.ai/code/session_0155N4QGamQVxgpAAPbpQNq4

[brendancol]

PR Review: Validate barriers, search_radius, start/goal points, and dims in pathfinding

Blockers (must fix before merge)

None found. The silent-barrier repro from #3649 (wall of zeros, barriers=['0']) now raises TypeError on all four backends, and the previously-crashing inputs get errors that name the parameter.

Suggestions (should fix, not blocking)

• multi_stop_search forwards barriers unvalidated and relies on the first a_star_search segment call to reject it (xrspatial/pathfinding.py:1553). The error still fires before any real work, but with optimize_order=True the traceback points into _optimize_waypoint_order instead of the user's call. One _validate_barriers(barriers) line next to the other multi_stop checks would keep the failure at the API boundary.

Nits (optional improvements)

• The a_star_search docstring still describes search_radius as plain int, optional and barriers as array like object (xrspatial/pathfinding.py:925, 932); neither mentions the new constraints (non-negative integer; 1-D numeric). Same for the Raises section of multi_stop_search (xrspatial/pathfinding.py:1479), which lists only ValueError while the barrier/radius type checks raise TypeError.
• _validate_barriers rejects boolean arrays (barriers=[True]) since np.bool_ is neither integer nor floating. Almost certainly nobody does this, but it did compare successfully before. Worth a conscious decision rather than an accident.
• proximity.py:1346 carries its own copy of the old malformed dims message ("raster.coords should be named as coordinates:"). Out of scope here; noting it so the wording fix can be replicated there in a follow-up.

What looks good

• The fix targets the failure at the dispatch boundary, so numpy, cupy, dask+numpy, and dask+cupy all get identical error behavior, and the PR states the battery was executed on all four.
• Exception types on already-raising paths are preserved; only the two silent-wrong-output cases start raising.
• Tests assert message content, not just exception type, and include positives (numeric barriers still block, search_radius=0 accepted, custom dims still work) so the new validation can't over-reject.
• The scalar-barriers hint ("Did you mean barriers=[0]?") is the kind of message that saves a user a trip to the docs.

Checklist

• Algorithm matches reference/paper (no algorithm change; validation only)
• All implemented backends produce consistent results (checks run pre-dispatch)
• NaN handling is correct (untouched)
• Edge cases are covered by tests (scalar/string/3-element/negative/float inputs)
• Dask chunk boundaries handled correctly (untouched)
• No premature materialization or unnecessary copies (barriers copy is a handful of floats)
• Benchmark exists or is not needed (validation-only change; existing pathfinding benchmark unaffected)
• README feature matrix updated (n/a, no new function)
• Docstrings present and accurate (see nit on parameter constraint docs)

[brendancol]

PR Review follow-up (after 0c8c517)

Disposition of the first-pass findings:

• Suggestion, barriers validated only inside the first segment call: fixed. multi_stop_search now calls _validate_barriers next to its other boundary checks (xrspatial/pathfinding.py:1496), with a test asserting the TypeError fires from the multi_stop call (test_string_barriers_raise_in_multi_stop). Re-validation in a_star_search is idempotent on the coerced float64 array.
• Nit, docstring constraints: fixed. barriers and search_radius parameter docs in a_star_search state the 1-D-numeric and non-negative-integer requirements, and multi_stop_search's Raises section now lists the TypeError paths.
• Nit, boolean barriers rejected: kept as-is, now with a code comment recording the decision. [True] as a barrier list is more plausibly a bug than a request for "cells equal to 1".
• Nit, proximity.py's copy of the old dims message: not touched. This sweep is scoped to the pathfinding module; the wording fix can ride along whenever proximity gets its pass.

72 tests pass locally including the cupy and dask+cupy lanes; flake8 output is byte-identical to main for both touched files. Nothing further from me.

[brendancol]

PR Review: Validate barriers, search_radius, start/goal points, and dims in pathfinding

Re-review after resolving the merge conflict with main. The branch was 4 commits
behind and conflicted with the sibling pathfinding PRs (#3653 API consistency, #3667
optimize_order, #3652/#3656 dask token, #3655/#3663 golden tests). Merged
origin/main at 9f448802; the net PR diff keeps its original scope (94 lines in
pathfinding.py, 123 in the tests, plus one state-CSV row).

Conflict resolution

• barriers docstring (a_star_search): kept main's "Default is no barriers" note
  alongside this branch's "must be a 1-D sequence of numeric values" note, in one
  sentence.
• barriers handling (a_star_search, ~line 1043): #3653 changed the default from
  [] to None and added an if barriers is None: barriers = [] guard. Kept that
  guard, then called _validate_barriers(barriers), which is how the merge had already
  resolved the same pattern in multi_stop_search.
• test_pathfinding.py (~line 1463): both sides only add tests. Kept both this
  branch's TestPathfindingErrorHandling class and main's module-level API-consistency
  tests.
• The state CSV auto-merged to a single pathfinding row.

Integration check against #3653's new signatures

The main risk was validation still attaching to the pre-merge signatures. What I checked:

• barriers: Optional[list] = None and search_radius: Optional[int] = None are the
  new defaults. _validate_search_radius(None) returns early, and _validate_barriers
  runs after the None -> [] guard, so both defaults pass.
• #3653 added Dataset routing through the @supports_dataset decorator on both
  functions. That decorator unwraps a Dataset into per-variable DataArrays and calls the
  wrapped body on each one, so _validate_surface_dims (which does
  surface.dims != (y, x)) never sees a Dataset, where .dims is a mapping rather than
  a tuple. Dataset input is not falsely rejected.
• main's new test_barriers_default_none_matches_empty_list checks that barriers=None,
  barriers=[], and the default all agree. It passes under this branch's validation.

Tests

PYTHONPATH=<worktree> python -m pytest xrspatial/tests/test_pathfinding.py: 99 passed
in 3.53s. CUDA + cupy are present here, so the GPU and dask+cupy paths ran locally
rather than being skipped.

What looks good

• Validation sits at the API boundary, before backend dispatch, so all four backends
  get the same errors.
• Error messages name the offending parameter and value and point at the fix (the
  scalar-barriers hint, the list-wrapping suggestion).
• Rejecting bool barrier arrays on purpose is a defensible call, and it's commented.

No blockers. The resolution keeps both this branch's validation and the merged sibling
work.