-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Disambiguate structured and abstract matrix multiplication #52464
Conversation
446cfe9
to
e7361e7
Compare
That is nice. I believe we have similar |
I have pushed a commit that removes the clumsy |
Hm, in fact, |
Looks possible. I'll try to get to this in a couple of days. Thanks for making this simpler! |
BTW, note JuliaLang/LinearAlgebra.jl#705. |
Is there anything left to be done here? |
I have some local stuff for the determination of the eltype of the destination array, but that could go into another PR. |
Nice work @jishnub @dkarrasch, thank you! |
The various methods of the form
*(::AbstractMatrix, ::Diagonal)
for structured matrices lead to method ambiguities in packages that define*(::MyMatrix, ::AbstractMatrix)
. However, these methods only seem to be defined here to use the first argument to construct the destination, as opposed to the fallback method that uses the second argument. Instead of specializing*
, we might as well specialize the call tosimilar
that chooses which matrix to use to construct the destination. This reduces code duplication, as well as avoids these ambiguities altogether. This also makes the destination types ofA * B
andB * A
identical if one of the two is aStructuredMatrix
.After this, for example, both of these produce sparse matrices:
The former is a
Matrix
on master.Close JuliaLang/LinearAlgebra.jl#1022
Address JuliaStats/PDMats.jl#176 on nightly
This PR changes the following behavior:
to
I'm unsure if the choice of
_init_eltype
was deliberate here (there is a test that specifically checks that_init_eltype
isn't being used for general matrices).