Add diffusion test coverage for cupy/dask+cupy, reflect boundary, and edge shapes#3424
Open
brendancol wants to merge 3 commits into
Open
Add diffusion test coverage for cupy/dask+cupy, reflect boundary, and edge shapes#3424brendancol wants to merge 3 commits into
brendancol wants to merge 3 commits into
Conversation
…3422) The diffuse() dispatch table registers all four backends but the tests only exercised numpy and dask+numpy. Add: - cupy and dask+cupy parity tests (numpy reference) - reflect boundary mode across numpy/dask/cupy/dask+cupy - 1x1 single-pixel and Nx1/1xN strip rasters - all-NaN and Inf inputs Test-only; no source changes.
brendancol
commented
Jun 20, 2026
brendancol
left a comment
Contributor
Author
There was a problem hiding this comment.
PR Review: Add diffusion test coverage for cupy/dask+cupy, reflect boundary, and edge shapes
Test-only PR. No source changed, so the correctness checks apply to the tests themselves, not to new library code.
Blockers
None.
Suggestions
None.
Nits
test_inf_input_propagates_like_nan: the assertion on the Inf cell's neighbour isnp.isnan(out[1, 2]) or np.isinf(out[1, 2]). That is a deliberately loose smoke check given Inf arithmetic is brittle, which is fine, but it would still pass if the behaviour flipped from inf to nan. Worth a one-line comment saying that is intentional so a future reader does not tighten it by mistake.
What looks good
- All four backends are exercised. The cupy and dask+cupy tests are guarded with
cuda_and_cupy_available, so they skip on non-GPU hosts and run on CI's GPU runner. - Cross-backend parity uses the shared
general_checks.pyhelpers with reasonable tolerances (1e-7 cupy, 1e-6 dask+cupy for reduction-order differences). - The edge cases line up with the issue: 1x1, Nx1, 1xN, all-NaN, Inf, plus the previously untested reflect boundary.
- The dask strip test chunks across the strip (
chunks=(4,1)), so it exercises the overlap path instead of collapsing to one chunk.
Checklist
- All implemented backends produce consistent results (parity tests)
- NaN handling correct (equal_nan, all-NaN and propagation tests)
- Edge cases covered
- Dask chunk boundaries handled (strip chunked across the overlap)
- No source changes -> no benchmark/README/docstring updates needed
- Tests pass locally: 39 passed, including 4 GPU tests on a CUDA host
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #3422
diffuse()dispatches to all four backends throughArrayTypeFunctionMapping, but the test file only exercised numpy and dask+numpy. This adds the missing coverage. Tests only, no source changes.reflectboundary mode across numpy, dask, cupy, and dask+cupy (previously onlynearestandwrapwere tested)Backend coverage: numpy, cupy, dask+numpy, dask+cupy. The four GPU tests ran on a CUDA host before this PR; CI reruns them on its own GPU runner. The reflect/strip/NaN/Inf cases also confirmed all paths produce correct results today, so #3422 is a coverage gap, not a latent bug.
Test plan:
pytest xrspatial/tests/test_diffusion.py-> 39 passed (was 24)