Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: xarray-contrib/xarray-spatial
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: main
Choose a base ref
...
head repository: xarray-contrib/xarray-spatial
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: issue-3738
Choose a head ref
Checking mergeability… Don’t worry, you can still create the pull request.
  • 2 commits
  • 2 files changed
  • 1 contributor

Commits on Sep 4, 2026

  1. Replace the D8 code-to-offset if/elif chain with a lookup table (#3738) 

    _code_to_offset mapped a D8 direction code to its (dy, dx) neighbour
offset with an eight-way if/elif chain. _flow_accum_cpu calls it twice
per cell and stream_link_d8, flow_length_d8, flow_path_d8,
stream_order_d8 and hand_d8 import it for their own per-cell loops.

Index two module-level int64 arrays by the code instead. numba freezes
module-level arrays as compile-time constants, so the call compiles to a
range check and two loads. The range check is written as an
inside-the-box test (0 <= c <= 128) on purpose: int(nan) is INT64_MIN in
numba and there is no bounds checking, so a rejection-form guard would
let a NaN code index out of bounds (same class as the erode fix in
#3703). Non-power-of-two codes in range and anything outside 0..128 keep
returning (0, 0), and the returns stay integer.

On a 1000x2000 float64 flow-direction raster _flow_accum_cpu dropped
from 56.3 ms to 39.7 ms (median of 7 after warmup) with identical
results, on both the clean raster and one built from a DEM with 30%
NaN.

Add a parametrised test pinning the mapping for all eight codes, 0,
in-range non-codes, out-of-range ints, floats and NaN, plus a check that
only the eight D8 entries of the tables are populated.
    @brendancol
    brendancol committed Sep 4, 2026
    Configuration menu
    Copy the full SHA
    d5e8533 View commit details
    Browse the repository at this point in the history

  2. Address review: explicit NaN check in _code_to_offset, module-level t… 

    …est imports (#3738)

Return (0, 0) for a NaN code before calling int() rather than relying
on the integer guard catching whatever the float-to-int conversion
produced (INT64_MIN on x86, 0 on aarch64, poison to LLVM). Reword the
guard comment to describe the real hazard, a rejection test on the
float that is False for NaN, instead of claiming an integer rejection
test would miss it.

The extra compare is free: 36.5 ms and 37.1 ms on two runs of the
1000x2000 case versus 39.7 ms before it, all within noise, output still
identical to origin/main.

Move the private imports in the new tests up to module level to match
the rest of the file.
    @brendancol
    brendancol committed Sep 4, 2026
    Configuration menu
    Copy the full SHA
    2405f0c View commit details
    Browse the repository at this point in the history
Loading