Skip to content

focal: honour boundary on the cupy backend (#2730)#2736

Merged
brendancol merged 3 commits into
mainfrom
deep-sweep-accuracy-focal-2026-05-29
May 30, 2026
Merged

focal: honour boundary on the cupy backend (#2730)#2736
brendancol merged 3 commits into
mainfrom
deep-sweep-accuracy-focal-2026-05-29

Conversation

@brendancol
Copy link
Copy Markdown
Contributor

Closes #2730

What

The single-GPU cupy backend of mean(), apply(), and focal_stats()
ignored the boundary parameter. It always behaved as boundary='nan'
(edge clamping), while numpy and dask honoured 'nearest', 'reflect',
and 'wrap'. The cupy dispatch functions were registered in the
ArrayTypeFunctionMapping without a boundary partial.

Change

  • Add _mean_cupy_boundary, _apply_cupy_boundary, and
    _focal_stats_cupy_boundary wrappers that pad the input per the
    boundary mode with _pad_array and trim the result, mirroring the
    existing numpy boundary path. The 'nan' mode is unchanged.
  • The focal_stats wrapper rebuilds the output DataArray so the
    original coords, dims, and attrs are preserved after trimming.

Backend coverage

numpy and dask were already correct. This fixes the cupy path. dask+cupy
already routed boundary through map_overlap. All four backends now match
for a given boundary value.

Test plan

  • mean/apply/focal_stats cupy results match numpy across
    nan/nearest/reflect/wrap (new parametrized tests)
  • cupy focal_stats preserves coords/dims/attrs under non-nan boundary
  • full test_focal.py suite: 129 passed (CUDA host)

mean(), apply(), and focal_stats() ignored the boundary parameter on the
single-GPU cupy backend, always behaving as boundary='nan' (edge clamping)
while numpy and dask honoured 'nearest'/'reflect'/'wrap'. Pad the cupy input
per the boundary mode (reusing _pad_array) and trim the result, mirroring the
numpy boundary path. Add cupy-vs-numpy boundary-equivalence tests.
@github-actions github-actions Bot added the performance PR touches performance-sensitive code label May 30, 2026
Copy link
Copy Markdown
Contributor Author

@brendancol brendancol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review: focal honours boundary on the cupy backend (#2730)

Blockers (must fix before merge)

None.

Suggestions (should fix, not blocking)

None.

Nits (optional improvements)

  • xrspatial/focal.py: _apply_cupy_boundary and _focal_stats_cupy_boundary repeat the r0/r1/c0/c1 trim slicing that already lives in _apply_numpy_boundary. A small shared _trim(result, pad_h, pad_w) helper would drop the duplication. I'd leave it as-is though, since the current form matches the existing numpy code and a one-off helper for three call sites is a wash.

What looks good

  • The three cupy wrappers follow the existing _mean_numpy_boundary / _apply_numpy_boundary shape, so the boundary semantics line up with numpy without a separate code path to keep in sync.
  • boundary='nan' short-circuits to the old code, so the default path is untouched.
  • _focal_stats_cupy_boundary rebuilds the output DataArray and reattaches the original coords and attrs after trimming. The new coord-preservation test covers that.
  • Tests check cupy against numpy across all four boundary modes for mean, apply, and focal_stats, run on a real CUDA host.

Checklist

  • Algorithm matches the numpy boundary reference (pad then trim)
  • All implemented backends produce consistent results (numpy vs cupy verified across modes)
  • NaN handling correct ('nan' mode unchanged)
  • Edge cases covered (1x1 kernel pads 0, trims to full array via the if pad else None guard)
  • Dask chunk boundaries unchanged (dask+cupy already forwarded boundary)
  • No premature materialization or unnecessary copies
  • Benchmark not needed (bug fix, no new function)
  • README feature matrix not applicable
  • Docstrings accurate (boundary already documented on each public function)

@brendancol brendancol merged commit 4760ae6 into main May 30, 2026
7 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.

focal mean/apply/focal_stats ignore boundary on the cupy backend

1 participant