Skip to content

pathfinding: LOW-severity cleanup backlog from the 2026-07-08 sweep round #3705

Description

@brendancol

Is your feature request related to a problem?

The 2026-07-08 sweep round over xrspatial/pathfinding.py (issues #3644, #3645, #3649, #3651, #3652, #3655, #3657, #3660) fixed everything MEDIUM and above but left a trail of LOW findings in the sweep notes. None of them justifies a PR on its own, so this bundles them into one cleanup pass. Every item below was re-verified against current main on 2026-08-16. Items already fixed since the sweeps (TSP solver unit tests, anisotropic-cellsize golden tests) are not listed.

Performance nits (documented in the #3660 sweep, not fixed)

  • _nearest_neighbor_2opt calls _tour_cost twice per 2-opt candidate, recomputing the full tour sum each time instead of the O(1) delta for a segment reversal. Only matters for hundreds of waypoints (_MAX_WAYPOINTS is 1000).
  • _a_star_dask re-fetches the popped node's friction value (f_u_val, invariant across the loop) inside the 8-neighbor loop (pathfinding.py:752). Hoist above the loop.
  • parent_ys/parent_xs are built with np.ones((h, w), dtype=np.int64) * NONE (pathfinding.py:286-287); np.full avoids the multiply pass.
  • The dask start/goal crossability check runs two separate scalar .compute() calls (pathfinding.py:1069,1072); one dask.compute(start_val, goal_val) halves the scheduler round-trips.
  • f_min for dask friction is recomputed with a full nanmin scan per a_star_search call (pathfinding.py:1130), so an N-waypoint multi_stop_search pays N-1 redundant scans. Flagged as a follow-up in multi_stop_search materializes the full grid on dask backends, defeating the sparse A* design #3660.

Error message (documented in the #3649 sweep, not fixed)

Test gaps (the "documented but not fixed" list from #3657)

  • _ChunkCache eviction: no test exercises the maxsize overflow path, so the LRU popitem branch has zero coverage.
  • _held_karp n==2 shortcut (pathfinding.py:1300-1301): the TSP unit tests added since use 3-4 cities only.
  • The 2-opt improvement branch: test_nearest_neighbor_2opt_vs_held_karp's distance matrix never produces an improving swap (nearest-neighbor already returns the 2-opt-stable tour), so improved = True is never hit.
  • _get_pixel_id default xdim/ydim arguments (pathfinding.py:103-106): every caller in the test suite passes explicit dims.

Benchmark gaps (documented in the #3645 sweep, not fixed)

  • The asv pathfinding benchmarks route an open grid with no barriers and no friction, which is A*'s best case; regressions in barrier/friction handling or frontier growth won't show. Add a maze or barrier-field variant.
  • dask+cupy is not a parameterizable backend anywhere in the suite (benchmarks/benchmarks/common.py get_xr_dataarray has no such type). This is suite-wide rather than pathfinding-specific; fixing it here would benefit every benchmark, but it can also be split out if it grows.

Describe the solution you'd like

One cleanup PR (or one PR plus a split-out for the suite-wide dask+cupy benchmark type):

  • the five small performance fixes, none of which change results
  • a zero-size raster validation error
  • four targeted tests for the uncovered branches
  • a barrier/friction benchmark variant

Excluded on purpose: the snap_start/snap_goal vs snap naming split (intentional per #3644, per-endpoint vs per-waypoint semantics) and the library-wide surface/raster/agg first-parameter naming (cross-cutting, tracked in #3644's notes).

Found by /sweeps-report rollup of the 2026-07-08 sweep round; re-verified 2026-08-16.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions