Skip to content

convolution: document the cross-correlation convention in convolve_2d and pin it with a test#3619

Open
brendancol wants to merge 2 commits into
xarray-contrib:mainfrom
brendancol:deep-sweep-reference-validation-convolution-2026-07-02
Open

convolution: document the cross-correlation convention in convolve_2d and pin it with a test#3619
brendancol wants to merge 2 commits into
xarray-contrib:mainfrom
brendancol:deep-sweep-reference-validation-convolution-2026-07-02

Conversation

@brendancol

Copy link
Copy Markdown
Contributor

convolve_2d applies the kernel as-is, without flipping it, so it computes cross-correlation and not a true convolution. I checked it against scipy.ndimage 1.16.1: interior pixels match scipy.ndimage.correlate exactly (max abs diff 0.0) under all four boundary modes, and they differ from scipy.ndimage.convolve once the kernel is asymmetric.

The docstring just called this a "2D convolution" and never mentioned the kernel isn't flipped. Anyone arriving from scipy.ndimage.convolve with an asymmetric kernel gets different numbers than they'd expect. The built-in circle_kernel and annulus_kernel are symmetric, so the usual path is fine; only asymmetric custom kernels are affected.

Two changes:

  • A note in the convolution_2d docstring spelling out the correlation convention, and that you can pass kernel[::-1, ::-1] if you want scipy.ndimage.convolve semantics.
  • A golden test with values pinned against scipy 1.16.1, so a later refactor can't quietly start flipping the kernel.

Repro:

import numpy as np, scipy.ndimage as ndi
from xrspatial.convolution import convolve_2d, custom_kernel

data = np.arange(24, dtype=np.float64).reshape(4, 6)
ker = custom_kernel(np.array([[1., 0., 0.],
                              [1., 1., 0.],
                              [1., 0., 0.]]))
out = convolve_2d(data, ker)
# out[1:-1, 1:-1] == scipy.ndimage.correlate: [[25, 29, 33, 37], [49, 53, 57, 61]]
# scipy.ndimage.convolve would give:          [[31, 35, 39, 43], [55, 59, 63, 67]]

Also updates the reference-validation sweep state CSV. Verdict for convolution: CONVENTION-DIFF (interior matches scipy.ndimage.correlate exactly; correlation-vs-convolution naming was undocumented). Reference tool: scipy 1.16.1; gdal and astropy unavailable on this host; cupy parity confirmed on GPU.

… pin it with a test

convolve_2d applies the kernel without flipping it, so it computes
cross-correlation, not a true convolution. Verified against scipy.ndimage
1.16.1: interior pixels match scipy.ndimage.correlate exactly across all
boundary modes and differ from scipy.ndimage.convolve on an asymmetric
kernel. The docstring called this a convolution without stating the kernel
is not flipped.

Add a note to the convolution_2d docstring stating the convention, and a
golden test pinning the correlation semantics.

reference-validation sweep 2026-07-02
@github-actions github-actions Bot added the performance PR touches performance-sensitive code label Jul 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

performance PR touches performance-sensitive code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant