-
Notifications
You must be signed in to change notification settings - Fork 97
Adds multi-dimensionality to ak.matmul to match numpy #5009
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
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
e9e9e68 to
9ecfe31
Compare
9ecfe31 to
d0946f0
Compare
ajpotts
reviewed
Oct 21, 2025
Contributor
ajpotts
left a comment
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.
Logic looks good! I had one question about performance.
1RyanK
approved these changes
Oct 27, 2025
Contributor
1RyanK
left a comment
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.
Looks good!
ajpotts
approved these changes
Oct 27, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #4961.
Adds multi-dim behavior to ak.matmul to match numpy.
As noted in comments in the matmul function, there are multiple cases:
If either of both arguments is/are 1-D, then matmul matches dot, so the args are sent there.
If both arrays are 2-D, the computation is handled as a conventional matrix-matrix multiplication. The shapes must be compatible. (Addendum: I had to fix a circular calling issue here -- dot also allows 2-D matrix-matrix multiplication, and was sending its arguments to matmul. Since that had each calling the other, I pulled the 2-D computation into a separate helper function named _matmul2D, that both dot and matmul call.)
If one array is > 2-D and the other is >= 2-D, the numpy rules for broadcasting shapes are followed, e.g.
Python-side, the matmul function has been significantly rewritten.
There are new chapel functions to handle the multi-dim case (multidimmatmul and matmulShape) in LinalgMsg.chpl.
There are unit tests that check the multi-dim case up to 3-D.
Note about dot:
In the course of sending things to dot, I discovered that it had a case I hadn't handled when I wrote ak.dot: where the left argument is 1-D and the right isn't. numpy.dot does this, although the online documentation doesn't explicitly describe it. So I added that.