convolution: document the cross-correlation convention in convolve_2d and pin it with a test#3619
Open
brendancol wants to merge 2 commits into
Conversation
… 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
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.
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.correlateexactly (max abs diff 0.0) under all four boundary modes, and they differ fromscipy.ndimage.convolveonce 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.convolvewith an asymmetric kernel gets different numbers than they'd expect. The built-incircle_kernelandannulus_kernelare symmetric, so the usual path is fine; only asymmetric custom kernels are affected.Two changes:
convolution_2ddocstring spelling out the correlation convention, and that you can passkernel[::-1, ::-1]if you wantscipy.ndimage.convolvesemantics.Repro:
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.