Skip to content

Conjugate complex filters in Convolve1D adjoints - #817

Open
antonsoo wants to merge 2 commits into
PyLops:devfrom
antonsoo:fix/complex-convolution-adjoint
Open

antonsoo wants to merge 2 commits into
PyLops:devfrom
antonsoo:fix/complex-convolution-adjoint

Conversation

@antonsoo

Copy link
Copy Markdown

Convolve1D reverses the filter for its adjoint but does not conjugate it. With complex filters, the forward result is correct while the adjoint and iterative inverse are wrong. Both short and long filters are affected.

Conjugate the reversed filter before the existing contiguous copy, preserving the CuPy stride handling from #797. Add regressions against explicitly constructed convolution matrices, correct the adjoint equations, and extend the example with a complex channel inverse.

For example, this adjoint error is about 42.85 on dev and near machine precision with the fix:

import numpy as np
from scipy.linalg import convolution_matrix
from pylops.signalprocessing import Convolve1D

h = np.array([1 + .5j, -.2 + .3j, .1 - .1j])
y = np.arange(12) + 1j * np.arange(12)[::-1]
op = Convolve1D(12, h=h, offset=1, dtype="complex128")
matrix = convolution_matrix(h, 12, "same")
print(np.linalg.norm(op.H @ y - matrix.conj().T @ y))

Validation:

  • All 24 new cases fail before and pass after, covering complex64/complex128, short/long filters, direct/FFT/overlap-add, multidimensional filters and broadcasting. The complete convolution suite passes on CPU and CuPy: 382 cases each.
  • Recorded speech and guitar deconvolution/calibration agree with independent dense damped least squares in all 48 complex-filter cases per backend. Maximum relative inverse error drops from 6.62 to 9.60e-7 on CPU and 1.80e-6 on an RTX 5070. All forward outputs and 48 real-filter controls per backend remain byte-identical.
  • Project lint/hooks, wheel build, documentation build and the convolution example pass. Mypy diagnostics are unchanged from dev. A broader integration run encountered a randomized float32 ChirpRadon dot-test tolerance failure; a 500-seed comparison gives identical results on unmodified dev and the patched tree, including two failing seeds.

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