Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Revisit mechanism supporting math operations involving LinearOperator objects #502

Open
bwohlberg opened this issue Feb 7, 2024 · 1 comment
Open

Revisit mechanism supporting math operations involving LinearOperator objects #502

bwohlberg opened this issue Feb 7, 2024 · 1 comment
Assignees
@bwohlberg @Michael-T-McCann
Labels
discussion required Some discussion necessary to decide how to address this issue improvement Improvement of existing code, including addressing of omissions or inconsistencies

Comments

@bwohlberg
Copy link
Collaborator

The mechanism supporting math operations involving LinearOperator objects (specifically scalar multiplication and addition and subtraction of two LinearOperator objects) should be revisited to determine whether it can be simplified. Specific issues include:

  1. The _wrap_add_sub wrapper defined in _linop.py duplicates the __add__ and __sub__ definitions in class LinearOperator.
  2. It's not clear whether applying the wrapper to methods of classes derived from LinearOperator is really necessary.
@bwohlberg bwohlberg added discussion required Some discussion necessary to decide how to address this issue improvement Improvement of existing code, including addressing of omissions or inconsistencies labels Feb 7, 2024
@bwohlberg bwohlberg mentioned this issue Feb 9, 2024
Merged
@Michael-T-McCann
Copy link
Contributor

For an example of a class that we hope could be simpler, see ScaledIdentity.

bwohlberg added a commit that referenced this issue Feb 9, 2024
@bwohlberg
Clean up _wrap_add_sub. Partially addresses #502
82ea91f
bwohlberg added a commit that referenced this issue Feb 15, 2024
@bwohlberg
Reduce memory requirements of Identity linop (#501)
6e46ca2 
* Clean up

* Work in progress: avoid creating a potentially large diagonal array for every Identity linop

* Trivial edit

* Typo fix

* Resolve some bugs

* Remove dependence of behaviour on order of operands

* Update year

* Remove attempt to impose commutative behaviour

* Clean up post #500 bug fix

* Clean up

* Scaling of an Identity returns a LinearOperator, not a Diagonal

* Add some tests

* Add ScaledIdentity linop

* Make binary operator behaviour commutative

* Add some tests

* Clean up

* Resolve mypy errors

* Address PR review comment

* Fix bug identified in PR review

* Add tests

* Wrapper simplification in progress

* Clean up _wrap_add_sub. Partially addresses #502

* Remove need for overriding __rmul__

* Remove now-unneccesary __rmul__ definitions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bwohlberg bwohlberg

@Michael-T-McCann Michael-T-McCann

Labels
discussion required Some discussion necessary to decide how to address this issue improvement Improvement of existing code, including addressing of omissions or inconsistencies
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@bwohlberg @Michael-T-McCann