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.

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

Override linop instead of _evaluate_linop #818

Conversation

timweiland
Copy link
Collaborator

In a Nutshell

Override linop instead of _evaluate_linop in arithmetic fallback covariance functions

Detailed Description

In arithmetic fallback covariance functions, override linop instead of _evaluate_linop, because the latter has the problem that the input preprocessing destroys attributes of np.array subclasses. Keeping the option of np.array subclassing can be very useful for certain linop implementations, e.g. tensor product covariance functions, where you might want to subclass np.array to represent grid structure in the inputs.

Copy link
Contributor

@JonathanWenger JonathanWenger left a comment

Choose a reason for hiding this comment

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

LGTM, but maybe @marvinpfoertner should take a quick look as well. I think he may have written this oroginally.

Copy link
Collaborator

@marvinpfoertner marvinpfoertner left a comment

Choose a reason for hiding this comment

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

LGTM! 🙏🏻

timweiland added 2 commits May 9, 2023 17:27
In arithmetic fallback covariance functions, override `linop` instead of
`_evaluate_linop`, because the latter has the problem that the input
preprocessing destroys attributes of np.array subclasses. Keeping the
option of np.array subclassing can be very useful for certain linop
implementations, e.g. tensor product covariance functions.
@timweiland timweiland force-pushed the linop_array_subclass_compatibility branch from 1a00636 to ba3e3d1 Compare May 9, 2023 15:27
@codecov
Copy link

codecov bot commented May 9, 2023

Codecov Report

Merging #818 (18224cc) into main (e7f332e) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #818   +/-   ##
=======================================
  Coverage   91.15%   91.15%           
=======================================
  Files         218      218           
  Lines        8216     8216           
  Branches     1061     1061           
=======================================
  Hits         7489     7489           
  Misses        499      499           
  Partials      228      228           
Impacted Files Coverage Δ
...robnum/randprocs/covfuncs/_arithmetic_fallbacks.py 82.05% <100.00%> (ø)

@timweiland timweiland merged commit 555485f into probabilistic-numerics:main May 10, 2023
@timweiland timweiland deleted the linop_array_subclass_compatibility branch May 10, 2023 08:55
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.

3 participants