polygonize: optimize visvalingam_whyatt with min-heap

[Melissari1997]

Closes #2539

Summary

Replace the O(n²) linear minimum-area scan in _visvalingam_whyatt with a binary min-heap keyed on triangle area, achieving O(n log n) scaling for line/polygon simplification.

Why this solves the performance problem

The original _visvalingam_whyatt iteratively removes the vertex with the smallest triangle area
On each iteration it scans all remaining interior vertices to find the minimum — an O(n²) cost.
For polygon rings with tens of thousands of vertices (common in high-resolution rasters), this quadratic scan dominates runtime.
This PR replaces that linear scan with a binary min-heap storing (area, index) pairs:

• Heap operations are O(log n): _heap_push and _heap_pop sift up/down in logarithmic time, so each of the n removals costs O(log n) instead of O(n).
• Lazy deletion handles stale entries: After a vertex is removed, its neighbors' areas change. Rather than updating arbitrary heap elements (which would require an index map), the new area is simply _heap_pushed as a fresh entry. When the stale entry is popped later, the removed[idx] flag or a mismatched area tells us to skip it and pop the next one.
• Overall complexity drops from O(n²) to O(n log n): Building the initial heap is O(n), each removal is O(log n), and each neighbor recomputation pushes O(1) entries — totaling O(n log n).

[brendancol]

PR Review: polygonize: optimize visvalingam_whyatt with min-heap

Reviewed the heap-based rewrite of _visvalingam_whyatt. The algorithm is correct and the correctness test is solid. My only real concern is the two timing-based tests, which will likely flake on shared CI.

Blockers (must fix before merge)

None.

Suggestions (should fix, not blocking)

• xrspatial/tests/test_polygonize.py:1019 and :1180: both test_visvalingam_whyatt_no_regression_small_inputs and test_visvalingam_whyatt_scales_subquadratic assert wall-clock ratios (ratio < 3.0). Timing assertions are flaky on shared CI runners. Under load, a GC pause, numba recompilation, or a noisy neighbor can blow past 3x and fail a PR that has nothing wrong with it. The scaling test also runs n up to 32000 across 5 sizes x 10 iterations, which is slow for the unit suite. Consider moving the timing checks into the asv suite (benchmarks/benchmarks/polygonize.py) and keeping only test_visvalingam_whyatt_heap_correctness in the test file. The correctness test is what actually guards the behavior.

Nits (optional improvements)

• The PR description says "Building the initial heap is O(n)", but the heap is built with n-2 sequential _heap_push calls at polygonize.py:1879, which is O(n log n), not a bottom-up O(n) heapify. The overall O(n log n) complexity still holds, so this is just a wording fix in the description, not the code.
• The O(n^2) baseline _visvalingam_whyatt_o2 is duplicated verbatim in both test methods (test_polygonize.py:962 and :1037). Pulling it into one module-level helper would keep the two copies from drifting apart.
• No asv benchmark covers the simplify path. benchmarks/benchmarks/polygonize.py exists but doesn't exercise simplification. Since issue polygonize: _visvalingam_whyatt is O(n^2) due to linear min-area scan #2539 is about simplify runtime, a benchmark there would guard against future regressions better than the wall-clock unit tests.

What looks good

• Heap capacity n*3 (polygonize.py:1876) is sized correctly. Total pushes over a run is at most 3(n-2): one initial push per interior vertex plus at most two re-pushes per removal. Worth noting because numba nopython mode does no bounds-checking, so an undersized buffer would be silent corruption rather than an IndexError.
• Lazy deletion is handled correctly. The not removed[idx] and area == areas[idx] check at polygonize.py:1890 skips stale entries, and the float-equality test is reliable here because a fresh push stores exactly areas[idx].
• Tie-break by smallest index (polygonize.py:1770-1772, 1797-1799) matches the old linear scan's strict < plus index-order iteration, which is why the outputs stay identical.
• Endpoints never enter the heap: the p > 0 and nx_i < n - 1 guards preserve the sentinel handling from the original.
• The correctness test compares heap output against the O(n^2) baseline across 4 patterns, 5 sizes, and 3 tolerances. That is a real equivalence check, not a smoke test.

Checklist

• Algorithm matches reference (verified against the retained O(n^2) baseline)
• Backends consistent (N/A: _visvalingam_whyatt is a CPU-only numba helper, not part of the DataArray backend dispatch)
• NaN handling is correct (matches baseline: NaN areas are never selected)
• Edge cases covered (n <= 2 early return; correctness test covers small n)
• Dask chunk boundaries (N/A for this helper)
• No premature materialization or unnecessary copies
• Benchmark exists (no asv benchmark for simplify; see suggestion)
• README feature matrix (N/A: private helper, no public API change)
• Docstrings present and accurate

[brendancol]

@Melissari1997 [HUMAN] hey the review above is the .claude/commands/review-pr.md output. What I've been doing is making sure to run this command, address any blockers/suggestions/nits and then do a follow up review as a final check.

[brendancol]

[HUMAN] there were a few small changes mentioned in the review-pr comment, besides that, LGTM

[Melissari1997]

Thank you! @brendancol
By the way, the commands are really well done. It would be useful in the future to set them up as pre-commit steps, to be run locally.

[Melissari1997]

PR Review: polygonize: optimize visvalingam_whyatt with min-heap (#2817)

Blockers (must fix before merge)

None.

Suggestions (should fix, not blocking)

None — all previous findings have been addressed.

Nits (optional improvements)

None.

What looks good

• Algorithmic correctness verified against O(n²) baseline across 4 patterns × 5 sizes × 3 tolerances = 60 combinations. The heap with lazy deletion produces bit-identical results.
• Lazy deletion design is sound: stale entries are filtered via both the removed[idx] flag and the area == areas[idx] value comparison. The monotonicity guarantee (areas[p] = max(new_area, min_area)) ensures an old stale entry's area never matches the updated value.
• All edge cases handled: NaN input returns all coords unchanged (no infinite loop), identical coords simplify to endpoints, two-point input returns early.
• Performance verified: manual benchmark shows O(n log n) scaling with doubling-time ratios of 1.57x–2.97x across 2000→32000 vertices. Timing tests use 200 iterations (small) / 50 iterations (large) for statistical stability.
• Benchmark added: PolygonizeSimplify class in benchmarks/benchmarks/polygonize.py covers both DP and VW methods.
• Code is clean: the vestigial remaining variable was removed, heap allocation rationale is documented (3n - 6 max + safety margin), sentinel values are explained.

Checklist

• Algorithm matches reference/paper — heap-based Visvalingam–Whyatt is the standard fast implementation
• All implemented backends produce consistent results — N/A (CPU-only internal helper, change is transparent to backends)
• NaN handling is correct — tested, returns all coords unchanged
• Edge cases are covered by tests — NaN, identical coords, straight-line, triangle, two-point, 60-way correctness comparison
• Dask chunk boundaries handled correctly — N/A (operates on post-polygonization CPU arrays)
• No premature materialization or unnecessary copies — np.empty pre-allocation, single pass
• Benchmark exists — PolygonizeSimplify class added
• README feature matrix updated — N/A (no new public function)
• Docstrings present and accurate — all three heap helpers and _visvalingam_whyatt have docstrings

Summary: The PR is clean, correct, and complete. No blockers.

[brendancol]

@Melissari1997 hey do you have a preference of WhatsApp vs. Slack vs. something else for chat / direct messaging?

[Melissari1997]

I think slack is ok!