Reuse symmetric costs in optimize_order instead of running reverse A* searches

[brendancol]

Closes #3661.

_optimize_waypoint_order ran a_star_search for every ordered waypoint pair, N(N-1) searches, even though the cost matrix is symmetric: the pixel graph is undirected and edge costs are geometric_distance * mean(friction at both endpoints), so cost(i -> j) == cost(j -> i). The comment at the top of pathfinding.py already described the intended N(N-1)/2 behavior; the code just never matched it.

Changes:

• The matrix build runs only the upper triangle and mirrors the result, halving the dominant cost of multi_stop_search(..., optimize_order=True). Measured: 6 waypoints on a 400x400 grid went from 30 A* runs to 15, with identical costs on a random non-uniform friction surface.
• With snap=True both directions still run. Snapping can move the requested start and goal pixels, and the goal cost is read at the requested pixel, so the two directions are not guaranteed to agree in that case. Behavior there is unchanged.

Backend coverage: the change is in the pure-Python matrix build, upstream of backend dispatch; all four backends benefit equally. Full pathfinding suite (including cupy and dask+cupy tests, executed on this GPU host) passes.

Tests: call-count regressions for both cases (test_optimize_order_symmetric_matrix_call_count expects N(N-1)/2 with snap off, test_optimize_order_snap_keeps_both_directions expects N(N-1) with snap on), plus the existing test_optimize_order_* behavior tests.

Test plan:

• pytest xrspatial/tests/test_pathfinding.py (59 passed)
• Issue repro re-run: 15 matrix searches instead of 30, same optimized order
• flake8 finding count unchanged vs base

Note: #3666 touches the same function for the dask memory fix (#3660); whichever merges second needs a small conflict resolution in _optimize_waypoint_order.

https://claude.ai/code/session_0155N4QGamQVxgpAAPbpQNq4

[brendancol]

PR Review: Reuse symmetric costs in optimize_order instead of running reverse A* searches

Blockers (must fix before merge)

None found. The symmetry argument holds: the pixel graph is undirected, edge cost is dd[i] * (f_u + f_v) * 0.5 which is invariant under direction, and barriers apply per cell, so shortest-path cost between two pixels is the same both ways. The repro confirmed identical forward and reverse costs on a random non-uniform friction surface, and the matrix build dropped from 30 to 15 searches for 6 waypoints with the same optimized order.

Suggestions (should fix, not blocking)

• test_optimize_order_snap_keeps_both_directions runs on an all-ones grid, so snapping never actually moves a pixel; the test pins the call-count contract but not the asymmetry it exists to protect. A variant where a waypoint sits on a NaN cell (so snap relocates it) would exercise the real case. Reasonable to leave for a follow-up since the current test still catches an accidental removal of the snap branch.

Nits (optional improvements)

• _pair_cost converts the finite cost through float(...) where the old code stored np.float64. Downstream (_held_karp, _nearest_neighbor_2opt) only does arithmetic and comparisons, so this is behavior-neutral, but worth knowing it changed.

What looks good

• The snap caveat is handled by keeping both directions rather than assuming symmetry everywhere; the asymmetry mechanism (goal cost read at the requested, unsnapped pixel) is spelled out in the comment.
• Two call-count regression tests pin both sides of the contract: N(N-1)/2 with snap off, N(N-1) with snap on.
• The module-header comment describing N(N-1)/2 behavior now matches the code instead of the code quietly doing double the work.

Interaction with other open PRs

#3666 rewrites the same loop's cost extraction for the dask memory fix (#3660). Whichever merges second needs a small manual resolution in _optimize_waypoint_order; both PR bodies flag it.

Checklist

• Algorithm correctness: symmetry holds for the undirected mean-friction cost model; snap exception preserved
• All backends: change is upstream of backend dispatch; full suite (including GPU tests) passes
• NaN handling: unreachable pairs stay INF, mirrored consistently
• Edge cases: snap-on path keeps old behavior and has a call-count test
• No premature materialization introduced (cost extraction unchanged on this branch)
• Benchmark: existing AStarSearch benchmark covers the underlying searches; matrix build has no dedicated benchmark, acceptable for a call-count reduction with test-pinned counts
• README feature matrix: no API change, not needed
• Docstrings: no public signature change

[brendancol]

PR Review follow-up: Reuse symmetric costs in optimize_order

Re-review after commit 6dd3ea7:

• Snap-test suggestion: fixed. The middle waypoint now sits on a NaN cell, so snapping genuinely relocates it and the direction asymmetry the double computation protects is exercised, not just the call count. Running the variant surfaced pre-existing behavior where the unreachable waypoint is dropped from the returned order; that is issue #3646 (filed separately today) and the test notes it rather than papering over it or fixing it out of scope here.
• float vs np.float64 nit: acknowledged in the first pass as behavior-neutral; no change needed.

Suite re-run: 59 passed. flake8 finding count unchanged. Nothing further from my side.