Skip to content

Commit dd907b8

Browse files
authored
Fix COG overview poisoned by nodata sentinel (#1613) (#1618)
* Fix COG overview generation poisoned by nodata sentinel (#1613) The NaN-to-sentinel rewrite in to_geotiff and write_geotiff_gpu ran before _make_overview / make_overview_gpu, so np.nanmean and the GPU counterparts saw the sentinel as a finite value and biased every overview pixel. A raster with NaN pixels and nodata=-9999 produced overview cells like -4998.75 where the correct nan-aware mean was 1.5. Thread a nodata kwarg through the reducers so they mask the sentinel back to NaN before aggregating. The writer's overview loop passes nodata in, then rewrites any all-sentinel cells (which surface as NaN from the reducer) back to the sentinel for the on-disk pyramid. CPU and GPU paths both fixed. New regression tests cover mean / min / max / median, partial-NaN blocks, all-NaN blocks, integer dtype passthrough, and CPU-GPU agreement. * Address Copilot review on #1618 - Drop redundant np.isfinite gate in _block_reduce_2d (CPU + GPU) so nodata=+/-inf is masked back to NaN like a finite sentinel, matching the upstream NaN->sentinel rewrite gate (`not np.isnan(nodata)` used at _writer.py:1171,1525,1620). - Suppress RuntimeWarning from nanmean/nanmin/nanmax/nanmedian on all-NaN blocks locally; the all-NaN output is the desired signal that the overview loop rewrites to the sentinel, so the warning was noise on every nodata-border COG write. - Fix the comment above the overview loop: NaN from an all-sentinel reduction is rewritten back to the sentinel before _write_tiled / _write_stripped runs, not serialised as NaN. - Add regression tests covering nodata=inf (CPU + GPU) and the warning-suppression contract for all-NaN blocks.
1 parent 4c8a5d2 commit dd907b8

5 files changed

Lines changed: 433 additions & 28 deletions

File tree

.claude/sweep-accuracy-state.csv

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.

xrspatial/geotiff/__init__.py

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2919,9 +2919,25 @@ def _gpu_compress_to_part(gpu_arr, w, h, spp):
29192919
if oh > 0 and ow > 0:
29202920
overview_levels.append(len(overview_levels) + 1)
29212921

2922+
# Pass ``nodata`` so the GPU reducer masks the sentinel back to
2923+
# NaN before averaging. Without this, the NaN->sentinel rewrite
2924+
# done above on ``arr`` leaks the sentinel into the overview
2925+
# reduction and poisons the pyramid (issue #1613). Rewrite any
2926+
# all-sentinel cell NaN back to the sentinel after each level
2927+
# so the on-disk overview tiles still carry the sentinel value
2928+
# external readers expect.
29222929
current = arr
29232930
for _ in overview_levels:
2924-
current = make_overview_gpu(current, method=overview_resampling)
2931+
current = make_overview_gpu(current, method=overview_resampling,
2932+
nodata=nodata)
2933+
if (nodata is not None
2934+
and np.dtype(str(current.dtype)).kind == 'f'
2935+
and not np.isnan(float(nodata))):
2936+
nan_mask = cupy.isnan(current)
2937+
if bool(nan_mask.any().item()):
2938+
current = current.copy()
2939+
current[nan_mask] = np.dtype(
2940+
str(current.dtype)).type(nodata)
29252941
oh, ow = current.shape[:2]
29262942
parts.append(_gpu_compress_to_part(current, ow, oh, samples))
29272943

xrspatial/geotiff/_gpu_decode.py

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2891,9 +2891,17 @@ def gpu_compress_tiles(d_image, tile_width, tile_height,
28912891
GPU_OVERVIEW_METHODS = ('mean', 'nearest', 'min', 'max', 'median', 'mode')
28922892

28932893

2894-
def _block_reduce_2d_gpu(arr2d, method):
2895-
"""2x block-reduce a single 2D CuPy plane using *method*."""
2894+
def _block_reduce_2d_gpu(arr2d, method, nodata=None):
2895+
"""2x block-reduce a single 2D CuPy plane using *method*.
2896+
2897+
When ``nodata`` is supplied and ``arr2d`` is a float dtype, cells that
2898+
equal the sentinel are masked back to NaN before the reduction so the
2899+
``cupy.nan*`` aggregation routines correctly skip them. Mirrors the
2900+
CPU helper :func:`xrspatial.geotiff._writer._block_reduce_2d` so the
2901+
two backends produce identical overviews when ``nodata`` is set.
2902+
"""
28962903
import cupy
2904+
import numpy as np
28972905

28982906
h, w = arr2d.shape
28992907
h2 = (h // 2) * 2
@@ -2908,12 +2916,25 @@ def _block_reduce_2d_gpu(arr2d, method):
29082916
# Mode is expensive on GPU; fall back to CPU
29092917
cpu_arr = arr2d.get()
29102918
from ._writer import _block_reduce_2d
2911-
cpu_result = _block_reduce_2d(cpu_arr, 'mode')
2919+
cpu_result = _block_reduce_2d(cpu_arr, 'mode', nodata=nodata)
29122920
return cupy.asarray(cpu_result)
29132921

29142922
# Block reshape for mean/min/max/median
29152923
if arr2d.dtype.kind == 'f':
29162924
blocks = cropped.reshape(oh, 2, ow, 2)
2925+
# Mask the sentinel back to NaN so cupy.nanmean and friends
2926+
# honour it as missing-data (issue #1613). Match the upstream
2927+
# NaN->sentinel rewrite gate so ``nodata=+/-inf`` is masked here.
2928+
if nodata is not None and not np.isnan(nodata):
2929+
try:
2930+
sentinel = np.dtype(str(arr2d.dtype)).type(nodata)
2931+
except (OverflowError, ValueError):
2932+
sentinel = None
2933+
if sentinel is not None:
2934+
mask = blocks == sentinel
2935+
if bool(mask.any().item()):
2936+
blocks = cupy.where(
2937+
mask, cupy.float64('nan'), blocks)
29172938
else:
29182939
blocks = cropped.astype(cupy.float64).reshape(oh, 2, ow, 2)
29192940

@@ -2936,7 +2957,7 @@ def _block_reduce_2d_gpu(arr2d, method):
29362957
return result.astype(arr2d.dtype)
29372958

29382959

2939-
def make_overview_gpu(arr, method='mean'):
2960+
def make_overview_gpu(arr, method='mean', nodata=None):
29402961
"""Generate a 2x decimated overview on GPU.
29412962
29422963
Parameters
@@ -2946,6 +2967,12 @@ def make_overview_gpu(arr, method='mean'):
29462967
method : str
29472968
Resampling method: 'mean', 'nearest', 'min', 'max', 'median',
29482969
or 'mode'.
2970+
nodata : scalar or None
2971+
When supplied and ``arr`` is a float dtype, cells equal to the
2972+
sentinel are masked back to NaN before the reduction so the
2973+
sentinel does not bias the result. Required for COG output that
2974+
sets ``nodata=...`` (issue #1613). Ignored for integer arrays
2975+
and for ``nearest`` / ``mode``.
29492976
29502977
Returns
29512978
-------
@@ -2955,7 +2982,7 @@ def make_overview_gpu(arr, method='mean'):
29552982
import cupy
29562983

29572984
if arr.ndim == 3:
2958-
bands = [_block_reduce_2d_gpu(arr[:, :, b], method)
2985+
bands = [_block_reduce_2d_gpu(arr[:, :, b], method, nodata=nodata)
29592986
for b in range(arr.shape[2])]
29602987
return cupy.stack(bands, axis=2)
2961-
return _block_reduce_2d_gpu(arr, method)
2988+
return _block_reduce_2d_gpu(arr, method, nodata=nodata)

xrspatial/geotiff/_writer.py

Lines changed: 94 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
import math
55
import struct
6+
import warnings
67

78
import numpy as np
89

@@ -139,8 +140,17 @@ def _compression_tag(compression_name: str) -> int:
139140
_MAX_OVERVIEW_LEVELS = 8
140141

141142

142-
def _block_reduce_2d(arr2d, method):
143-
"""2x block-reduce a single 2D plane using *method*."""
143+
def _block_reduce_2d(arr2d, method, nodata=None):
144+
"""2x block-reduce a single 2D plane using *method*.
145+
146+
When ``nodata`` is supplied and ``arr2d`` is a float dtype, cells that
147+
equal the sentinel are treated as NaN during the reduction so the
148+
``nan*`` aggregation routines correctly skip them. The reduced output
149+
keeps NaN wherever every contributing input cell was the sentinel
150+
(so callers can rewrite that NaN back to the sentinel after the
151+
reduction). The sentinel is ignored entirely for integer dtypes and
152+
for non-aggregation methods (``nearest``, ``mode``, ``cubic``).
153+
"""
144154
h, w = arr2d.shape
145155
h2 = (h // 2) * 2
146156
w2 = (w // 2) * 2
@@ -177,29 +187,59 @@ def _block_reduce_2d(arr2d, method):
177187
# Block reshape for mean/min/max/median
178188
if arr2d.dtype.kind == 'f':
179189
blocks = cropped.reshape(oh, 2, ow, 2)
190+
# When a sentinel was used in place of NaN by an upstream
191+
# NaN-to-sentinel rewrite, mask it back to NaN here so nanmean /
192+
# nanmin / nanmax / nanmedian honour the missing-data semantic.
193+
# Without this the sentinel value participates in the reduction
194+
# and poisons the overview (issue #1613). Match the upstream
195+
# NaN->sentinel rewrite gate (``not np.isnan(nodata)``) so that
196+
# ``nodata=+/-inf`` is masked here too.
197+
if nodata is not None and not np.isnan(nodata):
198+
try:
199+
sentinel = arr2d.dtype.type(nodata)
200+
except (OverflowError, ValueError):
201+
sentinel = None
202+
if sentinel is not None:
203+
mask = blocks == sentinel
204+
if mask.any():
205+
# ``np.where(mask, nan, blocks)`` produces a fresh
206+
# array so the caller's input is not mutated.
207+
blocks = np.where(mask, np.float64('nan'), blocks)
180208
else:
181209
blocks = cropped.astype(np.float64).reshape(oh, 2, ow, 2)
182-
183-
if method == 'mean':
184-
result = np.nanmean(blocks, axis=(1, 3))
185-
elif method == 'min':
186-
result = np.nanmin(blocks, axis=(1, 3))
187-
elif method == 'max':
188-
result = np.nanmax(blocks, axis=(1, 3))
189-
elif method == 'median':
190-
flat = blocks.transpose(0, 2, 1, 3).reshape(oh, ow, 4)
191-
result = np.nanmedian(flat, axis=2)
192-
else:
193-
raise ValueError(
194-
f"Unknown overview resampling method: {method!r}. "
195-
f"Use one of: {OVERVIEW_METHODS}")
210+
# Integer rasters can also carry a sentinel that an upstream
211+
# promotion already converted to NaN; cropped is integer so no
212+
# masking is needed here. The blocks.astype(float64) cast above
213+
# would lose any NaN anyway -- integer sentinels are handled at
214+
# the call site by promoting to float64 before reduction.
215+
216+
# nanmean / nanmin / nanmax / nanmedian emit RuntimeWarning when a
217+
# 2x2 block is all-NaN (typical at nodata borders). The all-NaN
218+
# output is the desired signal that the caller rewrites to the
219+
# sentinel, so suppress the warning locally to keep COG writes quiet.
220+
with warnings.catch_warnings():
221+
warnings.simplefilter('ignore', RuntimeWarning)
222+
if method == 'mean':
223+
result = np.nanmean(blocks, axis=(1, 3))
224+
elif method == 'min':
225+
result = np.nanmin(blocks, axis=(1, 3))
226+
elif method == 'max':
227+
result = np.nanmax(blocks, axis=(1, 3))
228+
elif method == 'median':
229+
flat = blocks.transpose(0, 2, 1, 3).reshape(oh, ow, 4)
230+
result = np.nanmedian(flat, axis=2)
231+
else:
232+
raise ValueError(
233+
f"Unknown overview resampling method: {method!r}. "
234+
f"Use one of: {OVERVIEW_METHODS}")
196235

197236
if arr2d.dtype.kind != 'f':
198237
return np.round(result).astype(arr2d.dtype)
199238
return result.astype(arr2d.dtype)
200239

201240

202-
def _make_overview(arr: np.ndarray, method: str = 'mean') -> np.ndarray:
241+
def _make_overview(arr: np.ndarray, method: str = 'mean',
242+
nodata=None) -> np.ndarray:
203243
"""Generate a 2x decimated overview.
204244
205245
Parameters
@@ -209,16 +249,23 @@ def _make_overview(arr: np.ndarray, method: str = 'mean') -> np.ndarray:
209249
method : str
210250
Resampling method: 'mean' (default), 'nearest', 'min', 'max',
211251
'median', 'mode', or 'cubic'.
252+
nodata : scalar or None
253+
When supplied and ``arr`` is a float dtype, cells equal to the
254+
sentinel are masked back to NaN before the reduction so the
255+
sentinel does not bias the result. Required for COG output that
256+
sets ``nodata=...`` (issue #1613). Ignored for integer arrays
257+
and for ``nearest`` / ``mode`` / ``cubic`` methods.
212258
213259
Returns
214260
-------
215261
np.ndarray
216262
Half-resolution array.
217263
"""
218264
if arr.ndim == 3:
219-
bands = [_block_reduce_2d(arr[:, :, b], method) for b in range(arr.shape[2])]
265+
bands = [_block_reduce_2d(arr[:, :, b], method, nodata=nodata)
266+
for b in range(arr.shape[2])]
220267
return np.stack(bands, axis=2)
221-
return _block_reduce_2d(arr, method)
268+
return _block_reduce_2d(arr, method, nodata=nodata)
222269

223270

224271
# ---------------------------------------------------------------------------
@@ -1100,9 +1147,36 @@ def write(data: np.ndarray, path: str, *,
11001147
if oh > 0 and ow > 0:
11011148
overview_levels.append(len(overview_levels) + 1)
11021149

1150+
# Overview reductions need the *unmasked* float array so that
1151+
# ``np.nanmean`` / ``np.nanmin`` / ``np.nanmax`` / ``np.nanmedian``
1152+
# honour the sentinel as missing-data. The CPU writer's caller
1153+
# (``to_geotiff``) currently rewrites NaN to ``nodata`` before
1154+
# ``write()`` runs (so the on-disk full-resolution tile bytes
1155+
# match the sentinel-aware reader). We pass ``nodata`` into
1156+
# ``_make_overview`` here so the reducer masks the sentinel back
1157+
# to NaN before averaging; without this, the sentinel poisons
1158+
# the overview (issue #1613). After reduction any block that was
1159+
# all-sentinel comes back as NaN; we rewrite those NaNs back to
1160+
# ``nodata`` below so the on-disk overview tiles use the same
1161+
# sentinel convention as the full-resolution band (external
1162+
# readers without NaN awareness still see a well-defined pixel).
11031163
current = data
11041164
for _ in overview_levels:
1105-
current = _make_overview(current, method=overview_resampling)
1165+
current = _make_overview(current, method=overview_resampling,
1166+
nodata=nodata)
1167+
# Rewrite any NaN produced by the all-sentinel reduction
1168+
# back to the sentinel so the overview pyramid carries the
1169+
# same masking convention as the full-resolution band. The
1170+
# original ``data`` already underwent the NaN->sentinel
1171+
# rewrite upstream, so the only new NaNs here come from the
1172+
# reducer itself.
1173+
if (nodata is not None
1174+
and current.dtype.kind == 'f'
1175+
and not np.isnan(nodata)):
1176+
nan_mask = np.isnan(current)
1177+
if nan_mask.any():
1178+
current = current.copy()
1179+
current[nan_mask] = current.dtype.type(nodata)
11061180
oh, ow = current.shape[:2]
11071181
if tiled:
11081182
o_off, o_bc, o_data = _write_tiled(current, comp_tag, pred_int,

0 commit comments

Comments
 (0)