Skip to content

Matmul overloaded for correlator class. #199

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

Merged
merged 12 commits into from
Jul 17, 2023
Merged

Matmul overloaded for correlator class. #199

merged 12 commits into from
Jul 17, 2023

Conversation

fjosw
Copy link
Owner

@fjosw fjosw commented Jul 13, 2023

I overloaded the matmul class which should now allow for the multiplication of matrix valued Corr objects with matrices. I will write a few tests and look into edge cases but this solution seems to work.

@fjosw
Copy link
Owner Author

fjosw commented Jul 13, 2023

I know extended the matmul method to also work on two correlator objects.

Additionally, I added a trace method for matrix correlators.

@fjosw fjosw requested a review from s-kuberski July 13, 2023 11:26
@fjosw
Copy link
Owner Author

fjosw commented Jul 13, 2023

I now also implemented a __rmatmul__ method for the multiplication of an array with a Corr. For this to work I had to set __array_priority__ to a high value (see https://numpy.org/doc/stable/reference/arrays.classes.html#numpy.class.__array_priority__). I hope this does not cause any unintended effects.

@fjosw
Copy link
Owner Author

fjosw commented Jul 13, 2023

Another bug in Corr.roll for matrix correlators fixed.

Comment on lines 259 to 260
if self.N == 1:
raise TypeError("Only works for correlator matrices.")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Strictly speaking, it is not necessary to catch this, since the trace of a 1x1 matrix should be well defined. But of course it does not make much sense.

Copy link
Owner Author

Choose a reason for hiding this comment

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

I just checked and numpy does not like the trace of an array with a single entry:

np.trace(np.array([1.1]))
> ValueError: diag requires an array of at least two dimensions

@s-kuberski
Copy link
Collaborator

Hi,

I did not (yet?) check the code by running it, but so-far everything looks fine. Thanks for taking care of this.

While you are at it: Currently, it is not possible to initialize a Corr object from a one-dimensional array:

li = [pe.cov_Obs(o, 0, '') for o in [1, 2, 3, 4]]
ar = np.array(li)
pe.Corr(li)
pe.Corr(ar)
----> 4 pe.Corr(ar)

     46             # This only works, if the array fulfills the conditions below
---> 47             if not len(data_input.shape) == 2 and data_input.shape[0] == data_input.shape[1]:
     48                 raise Exception("Incompatible array shape")
     49             if not all([isinstance(item, Corr) for item in data_input.flatten()]):

IndexError: tuple index out of range

would you mind looking into this?

@fjosw fjosw merged commit f1150f0 into develop Jul 17, 2023
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.

2 participants