Fix open_geotiff(gpu=True) silently dropping on_gpu_failure (#1615)

[brendancol]

Summary

open_geotiff(source, gpu=True, ...) silently dropped the on_gpu_failure kwarg. The dispatcher did not declare it, and the GPU branch did not forward it to read_geotiff_gpu. Callers wanting strict GPU-failure semantics through the dispatcher had no way to enable them; they had to bypass open_geotiff and call read_geotiff_gpu directly, defeating the auto-dispatch.

Same shape as #1561 (kwargs lost in dispatch) and #1605 (window= / band= lost on the GPU branch). The on_gpu_failure kwarg was added in PR #1590 (Closes #1560); the dispatcher was not updated then.

This PR:

• Adds on_gpu_failure to the open_geotiff signature with a sentinel default.
• Forwards the kwarg to read_geotiff_gpu when gpu=True.
• Raises ValueError when on_gpu_failure is supplied with gpu=False (default or explicit), or with chunks=N-only. The kwarg only applies to the GPU pipeline; refusing up front matches the pattern to_geotiff uses for max_z_error on non-LERC codecs.
• Sentinel-keyed forwarding keeps the dispatcher byte-stable for callers that never set on_gpu_failure: read_geotiff_gpu is called without the kwarg, so its own default ('auto') applies.

The sentinel definitions (_GPU_DEPRECATED_SENTINEL, _ON_GPU_FAILURE_SENTINEL) were moved up in the file so they are defined before open_geotiff uses one in its default value.

Reproducer

Before this PR:

from xrspatial.geotiff import open_geotiff
open_geotiff("file.tif", gpu=True, on_gpu_failure='strict')
# TypeError: open_geotiff() got an unexpected keyword argument 'on_gpu_failure'

After this PR:

from xrspatial.geotiff import open_geotiff
open_geotiff("file.tif", gpu=True, on_gpu_failure='strict')  # works
open_geotiff("file.tif", on_gpu_failure='strict')
# ValueError: on_gpu_failure only applies when gpu=True. ...

Test plan

• New tests in xrspatial/geotiff/tests/test_open_geotiff_on_gpu_failure_1615.py cover: signature presence, gpu=False rejection (default + explicit), chunks-only rejection, CPU dispatch byte-stability for the default case, dask dispatch byte-stability for the default case, GPU-path end-to-end with 'auto' and 'strict', invalid-value validation surfaces through the dispatcher on a GPU host, and an invalid-value test that works on CPU-only CI (validation fires before the cupy import).
• Existing test_gpu_kwarg_rename_1560.py, test_backend_kwarg_parity_1561.py, and test_gpu_window_band_1605.py all still pass (30/30).
• Full xrspatial/geotiff/tests/ suite: 1210 passing on this branch, 7 skipped, 3 pre-existing matplotlib palette failures unrelated to this change (confirmed by reverting __init__.py to origin/main and re-running the palette tests on main).

Closes #1615.

[Copilot]

Pull request overview

Fixes a dispatcher/API parity bug in xrspatial.geotiff.open_geotiff where the GPU failure-policy kwarg (on_gpu_failure) was not accepted/forwarded, preventing callers from requesting strict GPU-failure semantics through the public entry point.

Changes:

• Add on_gpu_failure to open_geotiff’s signature (with a sentinel default) and forward it to read_geotiff_gpu when gpu=True.
• Reject on_gpu_failure up-front when gpu=False (including the chunks=... CPU/dask dispatch), to avoid silent ignoring.
• Add a dedicated regression test module covering signature presence, rejection semantics, byte-stable default behavior, and GPU-path forwarding.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
xrspatial/geotiff/__init__.py	Extends open_geotiff to accept/validate/forward on_gpu_failure; moves sentinel definitions earlier to support sentinel defaults.
xrspatial/geotiff/tests/test_open_geotiff_on_gpu_failure_1615.py	Adds regression coverage for the dispatcher kwarg wiring and failure-mode behavior.

Comments suppressed due to low confidence (1)

xrspatial/geotiff/init.py:516

• open_geotiff now accepts on_gpu_failure, but for .vrt sources it returns read_vrt(..., gpu=gpu, ...) regardless of gpu=True, and on_gpu_failure is neither forwarded nor rejected. This means open_geotiff('file.vrt', gpu=True, on_gpu_failure='strict') will silently ignore the kwarg (the exact class of bug this PR is fixing). Consider explicitly rejecting on_gpu_failure for VRT dispatch (even when gpu=True) with a clear ValueError, or update the docstring/behavior so it’s never silently dropped.

    # ``on_gpu_failure`` is GPU-only. Reject it up front for CPU/dask paths
    # rather than silently dropping it once dispatch is decided -- callers
    # otherwise have no way to learn that the policy is being ignored.
    # ``gpu=False`` (the default) on a ``.vrt`` source still routes through
    # ``read_vrt`` below which has no GPU-failure concept, so the same
    # rejection rule applies there.
    if on_gpu_failure is not _ON_GPU_FAILURE_SENTINEL and not gpu:
        raise ValueError(
            "on_gpu_failure only applies when gpu=True. "
            "Pass gpu=True to enable the GPU pipeline, or drop "
            "on_gpu_failure to keep the default CPU path.")

    # VRT files (string paths only -- VRT XML references other files on disk)
    if isinstance(source, str) and source.lower().endswith('.vrt'):
        return read_vrt(source, dtype=dtype, window=window, band=band,
                        name=name, chunks=chunks, gpu=gpu,
                        max_pixels=max_pixels)

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.