geotiff: gds chunked gpu chunk task casts to declared dtype (#1909)#1918
Merged
Conversation
The GDS chunked GPU read path declared the dask graph dtype as float64 whenever the source had an in-range integer nodata sentinel, matching the CPU dask path's always-promote contract from #1597. The per-chunk _chunk_task ran the eager-style _apply_nodata_mask_gpu which only promotes when a sentinel pixel actually hits, so chunks with no sentinel hit returned the raw source dtype while the graph advertised float64 -- a silent declared/actual dtype mismatch. The fix moves the declared_dtype computation above _chunk_task so the closure can capture it, then casts arr to declared_dtype before returning. The cast is gated on arr.dtype != declared_dtype to skip the no-op astype(copy=True) allocation (mirrors the #1624 fix on the CPU dask path). 6 regression tests in test_chunked_gpu_declared_dtype_1909.py cover declared vs computed parity, CPU/GPU dask declared-dtype agreement, eager paths preserving source dtype, no-nodata round-trip, explicit dtype= kwarg, and sentinel-hit float64 promotion. Caught by the deep-sweep metadata propagation audit on 2026-05-15.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a declared-vs-computed dtype mismatch in the GeoTIFF dask+cupy GDS chunked read path, ensuring that each computed chunk is cast to the same dtype that the dask graph advertises (notably for integer rasters with an in-range nodata sentinel).
Changes:
- Compute
declared_dtypebefore defining the GDS_chunk_task, and cast each chunk result todeclared_dtype(skipping no-op casts) inxrspatial/geotiff/_backends/gpu.py. - Add regression tests intended to cover declared vs computed dtype parity and related backend behaviors.
- Update the
.claudesweep/audit state log to record the new finding and fix.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
xrspatial/geotiff/_backends/gpu.py |
Ensures GDS chunk tasks cast outputs to the dask graph’s declared dtype to avoid silent dtype mismatches. |
xrspatial/geotiff/tests/test_chunked_gpu_declared_dtype_1909.py |
Adds regression tests around chunked GPU dtype declaration/computation parity and nodata/dtype interactions. |
.claude/sweep-metadata-state.csv |
Updates audit metadata to reflect the #1909 finding and verification notes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| to_geotiff, | ||
| ) | ||
|
|
||
| cupy = pytest.importorskip("cupy") |
Comment on lines
+48
to
+52
| def test_chunked_gpu_declared_dtype_matches_computed(uint16_no_sentinel_path): | ||
| """Declared dask graph dtype must equal the computed chunk dtype.""" | ||
| da = read_geotiff_gpu(str(uint16_no_sentinel_path), chunks=4) | ||
| declared = da.data.dtype | ||
| computed = da.data.compute().dtype |
| assert da.data.dtype == np.float64 | ||
| computed = da.data.compute() | ||
| assert computed.dtype == np.float64 | ||
| assert np.isnan(computed[0, 0]) |
Address copilot review on #1918: - Use _gpu_only gate (cupy + cupy.cuda.is_available) instead of bare importorskip so the suite skips on hosts without CUDA, matching test_gds_chunked_gpu_parity_1896.py. - Call _read_geotiff_gpu_chunked_gds directly with a tiled fixture so the GDS chunked path is exercised regardless of kvikio availability. read_geotiff_gpu(chunks=...) only enters the GDS path when _gds_chunk_path_available qualifies; the previous fixture used a stripped file, so the test asserted a property that already held on the CPU-dask + upload fallback. - Use computed.get() before np.isnan so the host-side NumPy check is unambiguous on cupy buffers.
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
Fixes #1909. The GDS chunked GPU read path (
_read_geotiff_gpu_chunked_gdsinxrspatial/geotiff/_backends/gpu.py) declared the dask graph dtype asfloat64whenever the source had an in-range integer nodata sentinel, matching the CPU dask path's always-promote contract from #1597. The per-chunk_chunk_taskran the eager-style_apply_nodata_mask_gpuwhich only promotes when a sentinel pixel actually hits, so chunks with no sentinel hit returned the raw source dtype while the graph advertised float64 -- a silent declared/actual dtype mismatch.Reproducer (pre-fix)
Fix
Move the
declared_dtypecomputation above_chunk_taskso the closure can capture it, then castarrtodeclared_dtypebefore returning. The cast is gated onarr.dtype != declared_dtypeto skip the no-opastype(copy=True)allocation (mirrors the #1624 fix on the CPU dask path).Backend parity matrix (post-fix)
uint16file,nodata=9999declared, no sentinel pixel hits:open_geotiff)read_geotiff_gpu)read_geotiff_dask(chunks=...))read_geotiff_gpu(chunks=...))The eager paths only promote when a sentinel pixel hits (preserves source dtype for memory-budgeted users); the dask paths always promote so chunk concatenation cannot silently downcast (#1597 contract). Both contracts are valid; the bug was the dask+cupy graph declaring one contract and the chunks returning the other.
Test plan
xrspatial/geotiff/tests/test_chunked_gpu_declared_dtype_1909.py:dtype=kwarg threads throughtest_gds_chunked_gpu_parity_1896.pyandtest_dask_cupy_combined.pypass (11 tests).xrspatial/geotiff/tests/): 2875 passed, 7 skipped. The pre-existing failures intest_predictor2_big_endian_gpu_1517.py(AttributeError: module 'xrspatial.geotiff' has no attribute 'read_to_array'after the geotiff: split __init__.py into per-backend modules with shared validation #1813 modular refactor renamed it to_read_to_array) andtest_size_param_validation_gpu_vrt_1776.py(tile_size=4rejected by stricter_validate_tile_size_arg) exist on main and are unrelated to this fix.Detection
Caught by the deep-sweep metadata propagation audit on 2026-05-15. Cat 4 (dtype/nodata semantics) and Cat 5 (backend-inconsistent metadata).