Skip to content

Preserve filter axis order when embedding convolution kernels - #818

Open
antonsoo wants to merge 2 commits into
PyLops:devfrom
antonsoo:fix/embedded-convolution-axes
Open

antonsoo wants to merge 2 commits into
PyLops:devfrom
antonsoo:fix/embedded-convolution-axes

Conversation

@antonsoo

Copy link
Copy Markdown

Embedding a lower-dimensional filter along unsorted model axes currently reshapes its values without transposing them. For example, a 2D kernel applied to a 3D model with axes=(2, 0) represents a different spatial filter from the requested one. Its forward and adjoint can still pass a dot-test because both use the same incorrect kernel.

Transpose the filter into physical-axis order before inserting singleton dimensions. Ordered axes keep the identity permutation, negative axes use the existing normalization, and the full-rank filter path is unchanged. Add coordinate-based regressions and document the mapping between filter dimensions and model axes.

This example differs by 565 on dev and agrees to floating-point precision with the fix:

import numpy as np
from scipy.signal import convolve2d
from pylops.signalprocessing import Convolve2D

x = np.arange(5 * 3 * 7, dtype=float).reshape(5, 3, 7)
h = np.arange(1, 7, dtype=float).reshape(2, 3)
op = Convolve2D(x.shape, h=h, offset=(0, 1), axes=(2, 0))
expected = np.stack([
    convolve2d(x[:, i, :], h.T, mode="full")[1:6, :7]
    for i in range(3)
], axis=1)
print(np.max(np.abs(op @ x - expected)))

Validation:

  • Before: six new cases fail and two ordered-axis controls pass. After: all eight pass on CPU and CuPy, including negative axes, complex kernels and a 3D filter embedded in a 4D model. The complete CPU convolution suite passes: 366 cases.
  • On a recorded RGB photograph in three channel layouts, all 24 affected blur/deblur cases per backend now agree with independent sparse convolution and a direct sparse solve. Maximum relative inverse error drops from 0.03659 to 2.06e-7 on CPU and 2.59e-7 on an RTX 5070. All 24 ordinary-axis controls per backend remain byte-identical.
  • Project lint/hooks, wheel build, documentation build and the example pass; mypy diagnostics are unchanged. A broader integration run encountered a randomized float32 ChirpRadon dot-test tolerance failure, also reproduced on unmodified dev with identical seeded dot products.

AI disclosure: Codex was used to investigate the failure, implement the change, write and run the independent checks, and prepare the documentation and PR text.

@codacy-production

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 0 complexity · 0 duplication

Metric Results
Complexity 0
Duplication 0

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant