Skip to content

Commit 12855fe

Browse files
authored
convolution: validate kernel and agg inputs (#3623) (#3624)
* Validate kernel and DataArray inputs to convolution entry points convolve_2d checked the raster but never the kernel. A None, 1D, 3D, or list kernel reached the numba kernel and raised a TypingError that named nothing the user controls. An even-sided kernel was accepted and produced a silently off-center result, even though custom_kernel already rejects even kernels. Add _validate_kernel (2D, odd side lengths, duck-typed on ndim/shape so numpy and cupy kernels both pass) and call it in convolve_2d. Also call _validate_raster in convolution_2d so a non-DataArray agg raises a clear TypeError instead of "'memoryview' object has no attribute 'astype'". Internal callers (focal, edge_detection, emerging_hotspots) all pass odd 2D arrays, so they are unaffected. * sweep(error-handling): record convolution audit state
1 parent f1f81b8 commit 12855fe

3 files changed

Lines changed: 80 additions & 4 deletions

File tree

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1-
module,last_inspected,issue,severity_max,categories_found,notes
2-
bump,2026-07-02,,HIGH,1;2;3;4,"bump() agg template unvalidated: plain ndarray -> ArrayTypeFunctionMapping 'Unsupported Array Type'; 3D/1D DataArray -> 'too many values to unpack'. count/spread unvalidated (internal numpy errors / silent). Added _validate_raster(agg,ndim=2) + _validate_scalar for count,spread. all 4 backends verified (CUDA present)."
3-
geotiff,2026-07-02,3604,MEDIUM,2;4,"to_geotiff 0D/1D DataArray raised opaque IndexError from _coords.py coords_to_transform (dims[-2]) instead of clean 'Expected 2D or 3D' ValueError; numpy path + 4D DataArray already clean. Fixed via early ndim guard before dispatch (eager/vrt/gpu) + 3 tests; PR #3604. Read-side param validation + typed-error hierarchy + allow_rotated/allow_invalid_nodata VRT+chunked opt-in threading verified clean (CUDA available, GPU paths run). gh issue create blocked by auto-mode; PR opened. Cat 2+4."
1+
module,last_inspected,issue,severity_max,categories_found,notes
2+
bump,2026-07-02,,HIGH,1;2;3;4,"bump() agg template unvalidated: plain ndarray -> ArrayTypeFunctionMapping 'Unsupported Array Type'; 3D/1D DataArray -> 'too many values to unpack'. count/spread unvalidated (internal numpy errors / silent). Added _validate_raster(agg,ndim=2) + _validate_scalar for count,spread. all 4 backends verified (CUDA present)."
3+
convolution,2026-07-02,,HIGH,1;2;3;4,"convolve_2d/convolution_2d skipped kernel + DataArray validation: None/1D/3D/list kernel -> numba TypingError, even kernel silently off-center (custom_kernel rejects it), numpy agg -> memoryview astype error. Fixed via _validate_kernel + _validate_raster; branch deep-sweep-error-handling-convolution-2026-07-02 pushed to fork; issue/PR create blocked by auto-mode, open from parent. MEDIUM(unfixed): annulus_kernel inner>outer -> cryptic np.pad 'index cant contain negative values'. LOW: circle_kernel cellsize=0 ZeroDivisionError, cellsize<0 cryptic linspace; calc_cellsize non-DataArray -> AttributeError attrs. cupy verified."
4+
geotiff,2026-07-02,3604,MEDIUM,2;4,"to_geotiff 0D/1D DataArray raised opaque IndexError from _coords.py coords_to_transform (dims[-2]) instead of clean 'Expected 2D or 3D' ValueError; numpy path + 4D DataArray already clean. Fixed via early ndim guard before dispatch (eager/vrt/gpu) + 3 tests; PR #3604. Read-side param validation + typed-error hierarchy + allow_rotated/allow_invalid_nodata VRT+chunked opt-in threading verified clean (CUDA available, GPU paths run). gh issue create blocked by auto-mode; PR opened. Cat 2+4."

xrspatial/convolution.py

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -372,6 +372,34 @@ def custom_kernel(kernel):
372372
return kernel
373373

374374

375+
def _validate_kernel(kernel, func_name='convolve_2d'):
376+
"""Validate a convolution kernel: a 2D array with odd side lengths.
377+
378+
Duck-typed on ``ndim``/``shape`` so numpy and cupy kernels both pass.
379+
Rejects up front so a malformed kernel raises a clear ValueError
380+
instead of an inscrutable numba ``TypingError`` (or silent off-center
381+
output for even side lengths). Mirrors ``custom_kernel``'s odd-shape
382+
contract.
383+
"""
384+
if not hasattr(kernel, 'ndim') or not hasattr(kernel, 'shape'):
385+
raise ValueError(
386+
f"{func_name}(): `kernel` must be a 2D array with odd side "
387+
f"lengths, got {type(kernel).__module__}."
388+
f"{type(kernel).__qualname__}"
389+
)
390+
if kernel.ndim != 2:
391+
raise ValueError(
392+
f"{func_name}(): `kernel` must be a 2D array, got {kernel.ndim}D "
393+
f"with shape {tuple(kernel.shape)}"
394+
)
395+
rows, cols = kernel.shape
396+
if rows % 2 == 0 or cols % 2 == 0:
397+
raise ValueError(
398+
f"{func_name}(): `kernel` must have odd side lengths so it has a "
399+
f"well-defined center, got shape {(rows, cols)}"
400+
)
401+
402+
375403
# Numba parallel=True kernels must not be launched concurrently from multiple
376404
# Python threads: the default 'workqueue' threading layer is not threadsafe and
377405
# aborts the process (SIGABRT on macOS) when two host threads enter a parallel
@@ -558,6 +586,7 @@ def convolve_2d(data, kernel, boundary='nan'):
558586
agg = xr.DataArray(data)
559587
_validate_raster(agg, func_name='convolve_2d', ndim=2)
560588
_validate_boundary(boundary)
589+
_validate_kernel(kernel, func_name='convolve_2d')
561590
mapper = ArrayTypeFunctionMapping(
562591
numpy_func=_convolve_2d_numpy_boundary,
563592
cupy_func=_convolve_2d_cupy,
@@ -699,6 +728,7 @@ def convolution_2d(agg, kernel, name='convolution_2d', boundary='nan'):
699728
"""
700729

701730
# wrapper of convolve_2d
731+
_validate_raster(agg, func_name='convolution_2d', ndim=2)
702732
out = convolve_2d(agg.data, kernel, boundary)
703733
return xr.DataArray(out,
704734
name=name,

xrspatial/tests/test_convolution.py

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
import pytest
44
import xarray as xr
55

6-
from xrspatial.convolution import circle_kernel, convolve_2d, custom_kernel
6+
from xrspatial.convolution import circle_kernel, convolution_2d, convolve_2d, custom_kernel
77
from xrspatial.tests.general_checks import cuda_and_cupy_available
88

99
KERNEL = circle_kernel(1, 1, 1)
@@ -86,6 +86,51 @@ def test_convolve_2d_accepts_float64():
8686
assert np.isnan(out[0, 0])
8787

8888

89+
DATA = np.arange(25, dtype=np.float64).reshape(5, 5)
90+
91+
92+
@pytest.mark.parametrize("bad_kernel", [
93+
None,
94+
np.ones(3, dtype=np.float64), # 1D
95+
np.ones((3, 3, 3), dtype=np.float64), # 3D
96+
[[0, 1, 0], [1, 1, 1], [0, 1, 0]], # python list, not an array
97+
])
98+
def test_convolve_2d_rejects_bad_kernel(bad_kernel):
99+
# Bad kernels used to crash deep in numba with a cryptic TypingError.
100+
# convolve_2d must reject them up front with a clear message that names
101+
# `kernel`.
102+
with pytest.raises(ValueError, match="kernel"):
103+
convolve_2d(DATA, bad_kernel)
104+
105+
106+
@pytest.mark.parametrize("even_kernel", [
107+
np.ones((2, 2), dtype=np.float64),
108+
np.ones((4, 4), dtype=np.float64),
109+
np.ones((2, 3), dtype=np.float64),
110+
])
111+
def test_convolve_2d_rejects_even_kernel(even_kernel):
112+
# An even side length has no well-defined center; convolve_2d used to
113+
# silently produce an off-center result. custom_kernel already rejects
114+
# even kernels, so convolve_2d must too.
115+
with pytest.raises(ValueError, match="odd"):
116+
convolve_2d(DATA, even_kernel)
117+
118+
119+
def test_convolution_2d_rejects_non_dataarray():
120+
# Passing a plain numpy array used to fail with an inscrutable
121+
# "'memoryview' object has no attribute 'astype'"; validate up front.
122+
with pytest.raises(TypeError, match="DataArray"):
123+
convolution_2d(DATA, KERNEL)
124+
125+
126+
def test_convolution_2d_accepts_dataarray():
127+
# Positive path unchanged.
128+
agg = xr.DataArray(DATA, dims=['y', 'x'])
129+
out = convolution_2d(agg, KERNEL)
130+
assert isinstance(out, xr.DataArray)
131+
assert out.shape == agg.shape
132+
133+
89134
def test_convolve_2d_uses_correlation_convention():
90135
# Golden values verified against scipy.ndimage 1.16.1: convolve_2d
91136
# applies the kernel by cross-correlation (kernel NOT flipped), so it

0 commit comments

Comments
 (0)