Skip to content

Fix: scale the operator when dividing it by a scalar - #811

Merged
mrava87 merged 2 commits into
PyLops:devfrom
raashish1601:fix/803-scalar-division
Oct 11, 2026
Merged

mrava87 merged 2 commits into
PyLops:devfrom
raashish1601:fix/803-scalar-division

Conversation

@raashish1601

Copy link
Copy Markdown
Contributor

Op / y solves y = Op x, but LinearOperator.__truediv__ didn't check whether y is a scalar. So Op / np.sqrt(2) ran a solve against a constant data vector and returned an array, while Op / 2.0 failed deep inside cgls or lstsq (#803).

This takes option 1 from the issue: a scalar divisor (anything with np.ndim(y) == 0, so Python, NumPy and 0-d array scalars) now returns self * (1 / y), the same scaled operator Op * a gives. Non-scalar y still goes through the solve as before.

The FFT, FFT2D and FFTND operators override __truediv__ with their exact inverse, so they had the same problem (FFT(dims=8) / 2.0 raised AttributeError). Each of those overrides now hands a scalar to the base class first. I also added a sentence to the div docstring.

Tests: test_scalar_division in pytests/test_linearoperator.py (an explicit MatrixMult and a non-explicit VStack, real and complex, square and overdetermined) and in pytests/test_ffts.py (FFT, FFT2D and FFTND for each engine, skipping fftw and mkl_fft when they aren't installed). Both fail on dev and pass here. pytests/test_ffts.py, test_linearoperator.py and test_solver.py pass locally (fftw/mkl_fft not installed here), and ruff check / ruff format are clean on the changed files.

AI assistance: this PR was prepared with an AI coding agent (Claude Code, Anthropic). It reproduced the issue, wrote the fix and tests, and ran them locally as described above.

Closes #803

@codacy-production

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 12 complexity · 19 duplication

Metric Results
Complexity 12
Duplication 19

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.

@mrava87 mrava87 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@raashish1601 thanks for tackling this issue.

Overall the fix is good and clean... I just left a few comments, mostly things that did not match with other common patterns in the codebase, please fix them before I can approve and merge 😄

Comment thread pylops/linearoperator.py Outdated
Comment thread pytests/test_linearoperator.py Outdated
Comment thread pylops/signalprocessing/fft.py Outdated
Comment thread pylops/linearoperator.py
Comment thread pytests/test_ffts.py Outdated
Comment thread pytests/test_ffts.py Outdated
Only real scalars are now tested, and the FFT test skips the fftw and
scipy engines with CuPy arrays.

@mrava87 mrava87 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@raashish1601 thanks for the changes!

I just left two minor comments, after they are addressed this is ready to be merged 😄

Comment thread pylops/linearoperator.py
Comment thread pylops/linearoperator.py
@mrava87
mrava87 merged commit 68986b0 into PyLops:dev Oct 11, 2026
32 of 33 checks passed
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.

Op / scalar silently runs a solve (or fails obscurely) instead of scaling the operator

2 participants