Fix open_geotiff(gpu=True) silently dropping window= and band= (#1605)#1608
Merged
brendancol merged 3 commits intoMay 11, 2026
Conversation
…y-contrib#1605) The GPU dispatch path in open_geotiff forwarded dtype/overview_level/ name/chunks/max_pixels but not window or band, and read_geotiff_gpu did not declare either kwarg. Callers asking for a windowed or single-band GPU read got the full multi-band file back without warning -- a silent data-correctness drop that broke backend substitutability against the CPU eager path and read_geotiff_dask. Add window and band kwargs to read_geotiff_gpu and forward them through open_geotiff. The fix slices on device after the full-image GPU decode completes, which preserves correctness today; tile-level short-circuit is a future optimisation. Coords and attrs['transform'] match the eager path bit-for-bit so the only observable difference is the array backend. Regression tests cover direct read_geotiff_gpu calls, dispatch through open_geotiff(gpu=True), single-band-and-windowed combinations, and the out-of-bounds error paths.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a correctness gap in the GeoTIFF GPU read path where open_geotiff(gpu=True, window=..., band=...) previously ignored window and band, returning the full raster instead of the requested subset. It brings the GPU entrypoint closer to CPU/dask behavior by adding window/band support to read_geotiff_gpu and forwarding those kwargs through open_geotiff, with slicing performed post-decode on the GPU array.
Changes:
- Forward
windowandbandthroughopen_geotiff(..., gpu=True)intoread_geotiff_gpu. - Add
window/bandtoread_geotiff_gpuwith validation and post-decode slicing + coordinate/transform alignment. - Add GPU regression tests covering dispatcher behavior, direct GPU reads, combined window+band, and bounds validation.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
xrspatial/geotiff/__init__.py |
Threads window/band through the GPU dispatcher and implements post-decode window/band slicing + validation in read_geotiff_gpu. |
xrspatial/geotiff/tests/test_gpu_window_band_1605.py |
Adds regression tests for issue #1605 to ensure GPU reads honor window and band. |
.claude/sweep-api-consistency-state.csv |
Updates internal audit state to record issue/PR status. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…nks for stripped TIFFs Two Copilot review notes on read_geotiff_gpu: 1. Orientation tag (274) != 1 combined with window= has ambiguous semantics (file pixel space vs. display pixel space). _reader.read_to_array rejects this combo with a specific error; read_geotiff_gpu previously validated window only against the pre-orientation IFD extent and then applied the slice after the remap, which could disagree with the CPU path. Raise the same ValueError -- reusing the CPU reader's wording -- so the GPU and CPU backends agree. 2. The stripped-file fallback returned a fully-realised CuPy DataArray without honouring chunks=, so callers asking for a Dask+CuPy result on a stripped TIFF got an eager array. Mirror the tiled branch's .chunk() step before returning so chunks= works consistently across layouts. Regression tests: - An Orientation=2 stripped TIFF + window= raises ValueError matching the CPU reader. - A stripped TIFF + chunks=N produces a Dask-backed DataArray whose blocks are CuPy arrays of the requested shape; tuple chunks accepted.
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
open_geotiff(source, gpu=True, window=..., band=...)used to silently return the full multi-band file: the dispatcher inopen_geotiffforwardeddtype,overview_level,name,chunks,max_pixelstoread_geotiff_gpu, but notwindoworband, andread_geotiff_gpudid not declare either kwarg.windowandbandtoread_geotiff_gpuand forward both throughopen_geotiff. The fix slices on device after the full-image GPU decode completes, so correctness matches the CPU eager path bit-for-bit; tile-level I/O short-circuit is a future optimisation.attrs['transform']line up withopen_geotiff/read_geotiff_dask, so the only observable difference is the array backend.Fixes #1605.
Test plan
pytest xrspatial/geotiff/tests/test_gpu_window_band_1605.py(7 new tests pass: direct calls, dispatch viaopen_geotiff(gpu=True), window+band combined, bounds validation)pytest xrspatial/geotiff/tests/ -k "gpu or window or band"(279 passed, no regressions)open_geotiff(gpu=True, chunks=8, window=..., band=...)produces a Dask+CuPy DataArray whose.compute()equals the source slice