generate_terrain: map dask backends over an empty skeleton instead of the template#3575
Merged
Conversation
The dask backends regenerate every cell from coordinates and never read the template's values, but they mapped the per-chunk generator over the template array itself. For a from_template() grid that meant computing a da.full(shape, nan) and immediately discarding it. Map over a da.empty_like skeleton instead so the template drops out of the graph. Also removes the leftover data * 0 in the dask+cupy path, which built a NaN array the per-chunk cupy.zeros discarded.
brendancol
commented
Jun 29, 2026
brendancol
left a comment
Contributor
Author
There was a problem hiding this comment.
PR Review: generate_terrain dask backends map over an empty skeleton
Blockers
None.
Suggestions
- The new tests cover worley on and off for the dask+numpy non-materialization case, but
test_terrain_dask_cupy_does_not_materialize_templateonly runs with worley off. The worley pre-passmap_blockswas also switched to the skeleton, so a worley-on cupy case would exercise that second call site on GPU too. Low priority, since the numpy parametrize already covers both sites and the existingtest_terrain_all_nan_template_dask_cupy_matches_numpycovers the default path. (xrspatial/tests/test_terrain.py)
Nits
- The skeleton still produces one uninitialized
np.empty/cupy.emptyallocation per chunk feedingmap_blocks. That is the expected pattern and there is no memset, so the real win is dropping theda.full(nan)fill, not the allocation. No code change needed.
What looks good
- Verified the
from_template -> generate_terraingraph drops the full/NaN layer (onlyempty_like+xrspatial.terrainremain) and the output stays finite and matches numpy. - Removes the leftover
data * 0in the dask+cupy path, the same NaN*0 pattern #3525 fixed on the eager backends. - The poison-template tests are a direct proof that the template is never read: blocks that raise on compute, yet the result computes fine.
empty_likepreserves shape, dtype, and chunking, soblock_infocoordinates are unchanged and output values are identical (parity test passes).
Checklist
- No algorithm change (output verified identical)
- All four backends produce consistent results
- NaN handling correct (template NaN dropped, output finite)
- Edge cases covered (poison template, worley on/off)
- Dask chunk boundaries handled correctly (empty_like preserves chunking)
- Removes a premature materialization rather than adding one
- Benchmark exists (benchmarks/benchmarks/terrain.py)
- README feature matrix update not needed (no new function or backend)
- Comments added; no public API change
brendancol
commented
Jun 29, 2026
brendancol
left a comment
Contributor
Author
There was a problem hiding this comment.
Follow-up review (after 8a9b1eb)
The earlier Suggestion is addressed: test_terrain_dask_cupy_does_not_materialize_template is now parametrized over worley_blend [0.0, 0.2], so the worley pre-pass map_blocks call site is exercised on the dask+cupy path as well.
- Blockers: none
- Suggestions: none outstanding
- Nits: the earlier allocation note stands as documentation only; no code change warranted.
Local run: test_terrain.py template/materialize/skeleton tests pass (10 passed), full file green earlier (56 passed) on the CUDA box. Nothing new introduced by the follow-up.
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.
Summary
generate_terrainregenerate every cell from coordinates and never read the template's values. They now map the per-chunk generator over ada.empty_like(data)skeleton instead of the template array, so a NaN-filled template (e.g.from_template()'sda.full(shape, nan)) drops out of the graph instead of being memset and discarded.data * 0in the dask+cupy path, which built a NaN array the per-chunkcupy.zerosimmediately discarded.Output values are unchanged; this only removes wasted allocation and fill from the dask graph. Verified that the
from_template -> generate_terraingraph no longer carries afull/NaN layer (onlyempty_like+xrspatial.terrainremain).Closes #3574
Backends
data * 0removedTest plan
test_terrain_dask_does_not_materialize_template(worley on and off): a poisoned template whose blocks raise if computed must still produce finite terraintest_terrain_dask_skeleton_matches_numpy: skeleton output matches the numpy referencetest_terrain_dask_cupy_does_not_materialize_templatetest_terrain.pysuite green (56 passed, all four backends on the CUDA box)