Skip to content

Covariance #2

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 17 commits into from
Sep 21, 2018
Merged

Conversation

LukeMathWalker
Copy link
Member

A basic implementation for the computation of the covariance matrix starting from a 2-dimensional array of observations.

Ideally, this should be implemented for any n-dimensional array with n>=2, treating two user-specified axes as random_variable axis and observation axis, while using the remaining ones as batch axes. To implement the generalized version we have to be able to do an efficient batched matrix multiplication - the implementation of this routine is currently missing in ndarray.

@LukeMathWalker
Copy link
Member Author

LukeMathWalker commented Sep 20, 2018

The compiler complained about the lifetime of type A and to "calm it" I slapped 'static in front of A. Can you help me understand what the issue was? @jturner314

Copy link
Member

@jturner314 jturner314 left a comment

Choose a reason for hiding this comment

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

The compiler complained about the lifetime of type A and to "calm it" I slapped 'static in front of A. Can you help me understand what the issue was?

The issue was the A: LinalgScalar bound on .mean_axis(), since LinalgScalar requires 'static. I've submitted rust-ndarray/ndarray#491 to remove the unnecessary bounds.

Ideally, this should be implemented for any n-dimensional array with n>=2, treating two user-specified axes as random_variable axis and observation axis, while using the remaining ones as batch axes. To implement the generalized version we have to be able to do an efficient batched matrix multiplication - the implementation of this routine is currently missing in ndarray.

IMO, only supporting 2-D arrays is fine for an initial version. This type of thing occurs for a lot of other methods too (e.g. quantiles for n-D arrays or changing .mean_axis() to .mean_axes()).

Feel free to merge this when you think it's ready; it looks good to me. It would be nice to choose the "squash and merge" option to keep the history clean.

@LukeMathWalker
Copy link
Member Author

IMO, only supporting 2-D arrays is fine for an initial version. This type of thing occurs for a lot of other methods too (e.g. quantiles for n-D arrays or changing .mean_axis() to .mean_axes()).

I agree - it was more of a remainder for the future :)
I added the additional panic case (plus one more, for empty arrays).

I'll merge this and open issue to track the status of LinAlgScalar in ndarray.

@LukeMathWalker LukeMathWalker merged commit d75844e into rust-ndarray:master Sep 21, 2018
@LukeMathWalker LukeMathWalker deleted the correlation branch September 24, 2018 14:43
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