Skip to content

Preserve input float dtype in apply() and focal_stats() (#2769)#2805

Merged
brendancol merged 2 commits into
mainfrom
issue-2769
Jun 1, 2026
Merged

Preserve input float dtype in apply() and focal_stats() (#2769)#2805
brendancol merged 2 commits into
mainfrom
issue-2769

Conversation

@brendancol
Copy link
Copy Markdown
Contributor

Summary

apply() and focal_stats() cast every input to float32 internally, which silently downcast float64 rasters on all four backends (numpy, cupy, dask+numpy, dask+cupy). convolve_2d() already preserves the input floating dtype via the _promote_float helper, but focal did not.

  • Route the focal backends through the same _promote_float helper so a float64 input yields a float64 output.
  • Non-float inputs still promote to float32, matching the previous behavior.
  • Covers _apply_numpy, _apply_numpy_boundary, _apply_dask_numpy, _apply_cupy, _apply_dask_cupy, _focal_stats_func_cupy, _focal_stats_cupy, and _focal_stats_dask_cupy. The CPU focal_stats path already routes through apply, so it inherits the fix.

Backend coverage

numpy, cupy, dask+numpy, dask+cupy all preserve float64 end to end.

Test plan

  • Added test_apply_preserves_float64 and test_focal_stats_preserves_float64, parametrized over all four backends, mirroring the existing convolve_2d float64 coverage.
  • pytest xrspatial/tests/test_focal.py passes (158 tests, including the GPU paths on this machine).
  • flake8 clean on the changed lines.

Closes #2769

apply() and focal_stats() cast every input to float32 internally, which
silently downcast float64 rasters across the numpy, cupy, dask+numpy, and
dask+cupy paths. convolve_2d() already preserves the input floating dtype
via _promote_float; focal did not.

Route the focal backends through the same _promote_float helper so a
float64 input produces a float64 output. Non-float inputs still promote to
float32 as before.

Add float64-preservation tests for apply() and focal_stats() on all four
backends, mirroring the existing convolve_2d coverage.
@github-actions github-actions Bot added the performance PR touches performance-sensitive code label Jun 1, 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: Preserve input float dtype in apply() and focal_stats() (#2769)

Blockers (must fix before merge)

None.

Suggestions (should fix, not blocking)

None.

Nits (optional improvements)

  • xrspatial/tests/test_focal.py: the new tests only assert float64 preservation. They don't cover the float32-stays-float32 case or the documented dask lazy-dtype quirk (a float32 dask input reports float64 lazily but computes to float32). This matches the existing convolve_2d coverage, so it's consistent, but a float32 case would pin down the full contract. Optional.

What looks good

  • The fix routes all six focal helpers through the same _promote_float helper that convolve_2d already uses, so apply() and focal_stats() now share one dtype contract instead of two.
  • _apply_numpy dropped its in-kernel float32 cast and relies on the caller. Both callers (_apply_numpy_boundary and the _apply_dask_numpy partial) promote first, so the jitted function still sees a float input. zonal.py defines its own separate _apply_numpy with a different signature, so there's no collision.
  • The CPU focal_stats path delegates to apply(), so it inherits the fix without a separate change.
  • Tests run on all four backends, and the cupy / dask+cupy variants actually execute here (GPU present) rather than skip.

Checklist

  • Algorithm matches reference/paper: n/a, dtype-only change
  • All implemented backends produce consistent results: yes, float64 preserved on all four
  • NaN handling is correct: unchanged
  • Edge cases covered by tests: float64 covered on all backends
  • Dask chunk boundaries handled correctly: unchanged, ragged (2,3) chunks used
  • No premature materialization or unnecessary copies: yes
  • Benchmark exists or not needed: not needed, dtype-only fix
  • README feature matrix updated: n/a, no new function or backend change
  • Docstrings present and accurate: yes; the apply() docstring example already showed float64 output and now matches reality

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 (follow-up): Preserve input float dtype in apply() and focal_stats() (#2769)

Re-reviewed after the follow-up commit c3b186b4.

Blockers

None.

Suggestions

None.

Nits

None remaining. The earlier nit (no float32 case) is addressed by test_apply_keeps_float32, which asserts a float32 input computes to float32 on all four backends and documents the dask lazy-dtype quirk in a comment.

What looks good

  • Both sides of the dtype contract are now covered: float64 stays float64, float32 stays float32, across numpy / cupy / dask+numpy / dask+cupy.
  • Full test_focal.py suite passes locally (162 tests), with the GPU variants actually executing here.

This review is informational; rockout does not approve or block its own PRs.

@brendancol brendancol merged commit 308c433 into main Jun 1, 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.

apply() and focal_stats() silently downcast float64 input to float32

1 participant