Skip to content

Fix D-inf flow direction odd facet decomposition (Tarboton 1997)#1005

Merged
brendancol merged 3 commits intomasterfrom
fix/dinf-facet-decomposition
Mar 14, 2026
Merged

Fix D-inf flow direction odd facet decomposition (Tarboton 1997)#1005
brendancol merged 3 commits intomasterfrom
fix/dinf-facet-decomposition

Conversation

@brendancol
Copy link
Contributor

Summary

  • Odd facets (1,3,5,7) used consecutive CCW neighbor pairs (e.g. facet 1: e1=NE, e2=N) with diagonal-based d1 distances. Tarboton (1997) requires e1=cardinal, e2=diagonal with alternating angle sign (e.g. facet 1: e1=N, e2=NE, angle=pi/2-r). Even facets (0,2,4,6) were already correct.
  • Fixed both CPU (_cpu) and GPU (_gpu) kernels.
  • Added two targeted odd-facet angle tests (test_odd_facet_1_tarboton, test_odd_facet_7_tarboton) and a full-grid comparison against a pure-Python reference Tarboton (test_reference_tarboton_agreement).

Test plan

  • All 62 tests in test_flow_direction_dinf.py pass (numpy, dask, cupy, dask+cupy)
  • Odd-facet tests verify angle = acpi/4 + afr for facets 1 and 7
  • Reference test confirms cell-by-cell match on a 51x51 cone DEM

Odd facets (1,3,5,7) incorrectly used consecutive CCW neighbor pairs
(e.g. facet 1: e1=NE, e2=N) with diagonal-based d1 distances.
Tarboton requires e1=cardinal, e2=diagonal with alternating angle sign
(e.g. facet 1: e1=N, e2=NE, angle=pi/2-r). Even facets were already
correct. Fixed both CPU and GPU kernels.
@github-actions github-actions bot added the performance PR touches performance-sensitive code label Mar 14, 2026
- Increase GPU scanline fill MAX_ISECT from 512 to 2048 to handle dense
  polygon inputs (1000+ irregular polygons would silently drop edges,
  causing wrong pixel values and missing coverage)
- Add host-side warning when active edge count exceeds the GPU limit
- Vectorize _classify_geometries using shapely 2.0 array ops instead of
  per-geometry Python calls (178x faster for the classification step)
- Defer bounds computation in _parse_input so callers who supply explicit
  bounds skip the unnecessary bbox scan
- Add cross-library rasterizer benchmark (benchmarks/rasterizer_benchmarks.py)
  covering 10 geometry types with consistency checks and markdown output
…ology API

- Move D-inf flow accumulation from flow_accumulation.py into
  flow_accumulation_dinf.py with its own test file
- Update basin, watershed, stream_order, and stream_order_mfd to use
  D8 flow direction consistently
- Add flow_accumulation_dinf to public API and docs
- Simplify flow_accumulation.py to D8-only logic
@brendancol brendancol merged commit 94cf1f6 into master Mar 14, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

performance PR touches performance-sensitive code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant