-
Notifications
You must be signed in to change notification settings - Fork 42
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
Algebra.Matrix #32
Algebra.Matrix #32
Conversation
, OrdField r | ||
, Prim r | ||
, Storable r | ||
, Numeric.LinearAlgebra.Field r |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the other matrix implementation, these constraints were needed because HMatrix required them. It's probably possible to remove them here, which would be needed to remove the HMatrix dependency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can get rid of them, but I didn't want to dig into the main code, and I wanted to test HMatrix vs Native mmult to see where the performance difference lies.
Thanks for looking into this! I'd love to get something like this merged in if we can fix the issues I brought up in the inline comments. |
Your intuition was spot on. Neat looking code, a tiny ValidMatrix, no MatrixField and mmult might even work! I added an example as well to test with. |
ping! |
Whoops! I totally missed this. Thanks for following up. It's a bit hard for me to read the pull request now that it's scattered across two commits. Do you think you could squash them together for me? Also, do you have any intuition about why the category instance isn't working? I'd like to get that fixed too before accepting the pull request. |
So I can get a category instance, once wrapped with an Id and Zero (which I called Matrix' to avoid name clash with +>). What I can't work out is this next step - how to make a Matrix' vect r a b into an a +> b. I also think Id in Matrix' should be type Matrix' vect r (a::Symbol) (b::Symbol) - id doesn't need to be square. But it doesn't compile. Also I was wondering about all the |
An identity matrix will always be square.
Ideally, there'd be two cases. If the user uses a I'd say supporting the |
All good then, except for how to turn a Matrix' vect r a b into a a +> b? |
Thanks! Looks great! I'll have to think a bit about a better way to architect Also, sorry I didn't see this until now... somehow github isn't sending me all the email updates that I think it should be... As a useful follow on, I've added issue #37 that I think partly motivated this pull request. |
I had a go at a native subhask matrix and got to mmult (see last line of Matrix.hs). I just couldn't close the lid on the +> category.