-
-
Notifications
You must be signed in to change notification settings - Fork 398
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
Refactor matmul code with mul! #1995
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1995 +/- ##
==========================================
+ Coverage 88.86% 88.99% +0.13%
==========================================
Files 33 33
Lines 4282 4306 +24
==========================================
+ Hits 3805 3832 +27
+ Misses 477 474 -3
Continue to review full report at Codecov.
|
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.
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.
LGTM assuming tests pass.
The new implementation catches
mul!
calls for which the eltype of the result is a JuMP affine or quadratic expressions (resolving #1990).This allows the generic codes using
mul!
to benefit from our specialized multiplication methods.For SparseArrays, the situation is less ideal as some calls are redirected to
mul!
and some have custom implementation that do not work for JuMP as they start by promoting the arrays to the same eltype...This PR redirects some of them so that all tests pass but more are needed (more tests and more redirections) but this is left for future work. I don't think there is any regression as we already had this issue: #1276
Closes #1990
Closes #1716