Cover write_geotiff_gpu compression modes (zstd default + jpeg) (#1646)#1647
Merged
brendancol merged 2 commits intoMay 12, 2026
Merged
Conversation
Cover the documented compression= modes that had no targeted round-trip tests: - zstd (the default, "fastest on GPU"): pixel-exact round-trip on int32 plus default-codec pinning via TIFF tag 259. - jpeg (nvJPEG with Pillow fallback): round-trip for 3-band uint8 RGB and single-band uint8 with mean-abs-diff bounds; pin compression tag 7. Exercises the live nvJPEG encoder on a GPU host, not just the Pillow fallback. - deflate + none: plain round-trips outside the COG / nodata-sentinel paths so a regression in the basic tiled assembly is visible. - Cross-codec parity: zstd, deflate, none must produce pixel-identical read-backs for the same input (catches predictor / codec mis-wiring). 11 tests, all passing on the GPU host. Update .claude/sweep-test-coverage-state.csv to record pass 7.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds targeted GPU-only test coverage for xrspatial.geotiff.write_geotiff_gpu(..., compression=...) to close a parameter-coverage gap around the documented compression modes (including the default zstd and GPU JPEG), plus updates the local coverage-sweep tracking file.
Changes:
- Add a new GPU-only pytest module with round-trip and TIFF Compression-tag assertions for
zstd,deflate,jpeg, andnone. - Add cross-codec parity checks for lossless codecs and additional explicit round-trip coverage outside COG/sentinel-focused paths.
- Update
.claude/sweep-test-coverage-state.csvto record this coverage pass.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
xrspatial/geotiff/tests/test_gpu_writer_compression_modes_2026_05_11.py |
Adds GPU-only tests covering write_geotiff_gpu compression modes, round-trips, and TIFF Compression tag checks. |
.claude/sweep-test-coverage-state.csv |
Records the coverage sweep “Pass 7” status and notes for the geotiff module. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+129
to
+134
| """Omitting ``compression=`` defaults to zstd; bytes must match an | ||
| explicit ``compression='zstd'`` call. | ||
|
|
||
| Pins the default so a silent change to the default codec (eg. to | ||
| 'deflate') would fail this test. | ||
| """ |
Comment on lines
+155
to
+171
| JPEG is lossy so we tolerate a moderate per-pixel error budget but | ||
| require the mean error to stay within typical JPEG quality bounds | ||
| (well under 50 for default-quality 8-bit). | ||
| """ | ||
| da, arr = _make_rgb_uint8_da() | ||
| path = str(tmp_path / "jpeg_rgb.tif") | ||
|
|
||
| write_geotiff_gpu(da, path, compression='jpeg') | ||
|
|
||
| out = open_geotiff(path) | ||
| assert out.shape == arr.shape | ||
| assert out.dtype == arr.dtype | ||
| diff = np.abs(out.values.astype(np.int32) - arr.astype(np.int32)) | ||
| # Random uint8 is the worst case for JPEG; we just want to catch a | ||
| # codec that emits all-zero or all-255 output rather than measure | ||
| # quality. Mean-abs-diff below 50 is comfortable for default quality. | ||
| assert diff.mean() < 50, ( |
Comment on lines
+151
to
+173
| @_gpu_only | ||
| def test_write_geotiff_gpu_jpeg_rgb_roundtrip(tmp_path): | ||
| """``compression='jpeg'`` round-trips a 3-band uint8 RGB raster. | ||
|
|
||
| JPEG is lossy so we tolerate a moderate per-pixel error budget but | ||
| require the mean error to stay within typical JPEG quality bounds | ||
| (well under 50 for default-quality 8-bit). | ||
| """ | ||
| da, arr = _make_rgb_uint8_da() | ||
| path = str(tmp_path / "jpeg_rgb.tif") | ||
|
|
||
| write_geotiff_gpu(da, path, compression='jpeg') | ||
|
|
||
| out = open_geotiff(path) | ||
| assert out.shape == arr.shape | ||
| assert out.dtype == arr.dtype | ||
| diff = np.abs(out.values.astype(np.int32) - arr.astype(np.int32)) | ||
| # Random uint8 is the worst case for JPEG; we just want to catch a | ||
| # codec that emits all-zero or all-255 output rather than measure | ||
| # quality. Mean-abs-diff below 50 is comfortable for default quality. | ||
| assert diff.mean() < 50, ( | ||
| f"JPEG round-trip mean diff {diff.mean()} suggests encoder/decoder break" | ||
| ) |
- Rewrite test_write_geotiff_gpu_zstd_default_matches_explicit so the docstring matches what it asserts: pin the compression tag and the decoded-array equality, not byte-for-byte file equality (the writer may legitimately vary tile padding/ordering between runs). - Swap the JPEG RGB test input from random uint8 noise to a deterministic smooth gradient (mirroring test_jpeg.py::_gradient_rgb). Tighten the mean-abs-diff bound from 50 to 8 for RGB and to 5 for the monochrome variant; the looser bound only existed because random noise is the worst case for JPEG. - Add test_write_geotiff_gpu_jpeg_uses_nvjpeg_when_available: spy on _gpu_decode._nvjpeg_batch_encode via monkeypatch and assert it fires at least once when libnvjpeg is loadable. Without this spy a regression breaking nvJPEG would silently fall through to the Pillow fallback and the round-trip tests would still pass. The new test is guarded by _nvjpeg_only so it only runs on hosts where libnvjpeg is actually loadable.
This was referenced May 12, 2026
brendancol
added a commit
that referenced
this pull request
May 12, 2026
…ff (#1652) to_geotiff has long rejected cog=True + file-like destinations, but the explicit write_geotiff_gpu entry point silently accepted the combo and emitted a COG into the buffer. The two writers should agree on which inputs they refuse: to_geotiff(gpu=True, cog=True, path=BytesIO) raises, so write_geotiff_gpu(da, BytesIO, cog=True) should too. Mirror the existing to_geotiff guard on the GPU entry point. Non-cog file-like writes remain supported on this path (the gate is targeted at cog=True only). Add regression coverage in test_bytesio_source.py. Also clarify the path/compression docstring on write_geotiff_gpu: - path: document that file-like destinations are accepted (cog=True requires a string path). - compression: list the full codec set the function actually accepts and note the deliberate JPEG asymmetry with to_geotiff (#1651 downgraded to docs-only after PR #1647 confirmed advanced-API intent). Update .claude/sweep-api-consistency-state.csv with the 2026-05-11 re-audit row.
brendancol
added a commit
that referenced
this pull request
May 12, 2026
…ff (#1652) (#1653) * Block write_geotiff_gpu(file_like, cog=True) for parity with to_geotiff (#1652) to_geotiff has long rejected cog=True + file-like destinations, but the explicit write_geotiff_gpu entry point silently accepted the combo and emitted a COG into the buffer. The two writers should agree on which inputs they refuse: to_geotiff(gpu=True, cog=True, path=BytesIO) raises, so write_geotiff_gpu(da, BytesIO, cog=True) should too. Mirror the existing to_geotiff guard on the GPU entry point. Non-cog file-like writes remain supported on this path (the gate is targeted at cog=True only). Add regression coverage in test_bytesio_source.py. Also clarify the path/compression docstring on write_geotiff_gpu: - path: document that file-like destinations are accepted (cog=True requires a string path). - compression: list the full codec set the function actually accepts and note the deliberate JPEG asymmetry with to_geotiff (#1651 downgraded to docs-only after PR #1647 confirmed advanced-API intent). Update .claude/sweep-api-consistency-state.csv with the 2026-05-11 re-audit row. * Address Copilot review feedback on #1653 - Mirror to_geotiff's cog=True + file-like rejection verbatim (same error string), so callers see identical messages from either entry point. Previously write_geotiff_gpu raised a different message that added "...on the GPU writer..." and dropped the BytesIO hint. - Add a TypeError gate for non-str, non-file-like path arguments, so passing e.g. an int falls through to a clear TypeError instead of an os.path / unicode error deep in the writer. Mirrors to_geotiff's existing TypeError verbatim. - Harden the BytesIO + write_geotiff_gpu tests with the repo's standard _gpu_available() helper (cupy.cuda.is_available() + ImportError guard) instead of pytest.importorskip('cupy'), so CI hosts where CuPy imports but CUDA is unavailable skip cleanly rather than hard-failing in cupy.asarray(). - Add two new regression tests: one pinning byte-for-byte parity of the cog/file-like error message between the two writers, and one pinning the new TypeError on invalid path types. Skip: the low-confidence type annotation suggestion (path: str|os.PathLike|SupportsWrite[bytes]). The other path-accepting functions in this module (to_geotiff, write_vrt, etc.) deliberately leave path untyped; adding a precise union here would diverge from the local convention for marginal benefit. All 17 tests in test_bytesio_source.py pass.
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
test_gpu_writer_compression_modes_2026_05_11.pywith 11 tests covering the documentedwrite_geotiff_gpucompression=modes (zstd,deflate,jpeg,none).compression='zstd'and'jpeg'had no targeted round-trip coverage before this PR; only'deflate'and'none'did. The new tests exercise the live nvJPEG GPU encoder and the nvCOMP zstd path on a CUDA host..claude/sweep-test-coverage-state.csv(pass 7).Closes #1646.
Test plan
pytest xrspatial/geotiff/tests/test_gpu_writer_compression_modes_2026_05_11.py-> 11 passed locally on a CUDA host._nvjpeg_batch_encode.