-
-
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
Implement multiply-add interface in LinearAlgebra #29634
Conversation
Wouldn't the two functions defined do exactly the same thing for the default arguments of |
The idea is that ternary |
It may not be defined for Matrix{BigFloat}.
and addmul!(C, X, s::Number, alpha, beta)
I'd forgotten that. It's quite rare that there are no changes detected at all. Then let's get thing merged such that we can finally start using the feature. It's been in demand for quite some time. |
The tests for this are failing sometimes. |
I hope that by mentioning the following issue/inconsistency here, the relevant people are directly informed: The function
tries to call BLAS/gemm whenever the scalars can be converted to type T. However, for e.g. adjoint
which requires the scalars to be of type
which directly calls
instead of BLAS/gemm. The same seems to be true for other combinations of adjoint and transpose. |
Also, I notice that with a code which uses many small matrix multiplications, the construction of the |
As for Adjoint/Transpose, I think we just need something like #33229 (as you've already discovered).
N = 10
A = randn(N, N)
B = randn(N, N)
C = similar(A)
function demo(C, A, B, alpha, beta, n)
for _ in 1:n
mul!(C, A, B, alpha, beta)
end
end
demo(C, A, B, 1, 0, 1)
@time @profile demo(C, A, B, 1, 0, 10^5) while function demo(C, A, B, n)
for _ in 1:n
mul!(C, A, B, 1, 0)
end
end Maybe something like #29634 (comment) would be the solution? Maybe we can also add an early branch in |
Thanks for the quick response and the pointers @tkf . For the |
Yeah, propagating constants for long call chains is not super robust... FYI, one option may be to use StaticNumbers.jl. This calls using StaticNumbers
demo(C, A, B, static(1), static(0), 1)
@time @profile demo(C, A, B, static(1), static(0), 10^5) I also tried #29634 (comment) and it works (i.e., |
This is suggested by chethega in: JuliaLang#29634 (comment)
This is suggested by chethega in: JuliaLang#29634 (comment)
* Construct MulAddMul at gemm_wrapper! call sites * Add branches manually in MulAddMul constructor This is suggested by chethega in: #29634 (comment) * Update stdlib/LinearAlgebra/src/generic.jl Co-Authored-By: Kristoffer Carlsson <kristoffer.carlsson@chalmers.se> Co-authored-by: Kristoffer Carlsson <kristoffer.carlsson@chalmers.se>
* Construct MulAddMul at gemm_wrapper! call sites * Add branches manually in MulAddMul constructor This is suggested by chethega in: #29634 (comment) * Update stdlib/LinearAlgebra/src/generic.jl Co-Authored-By: Kristoffer Carlsson <kristoffer.carlsson@chalmers.se> Co-authored-by: Kristoffer Carlsson <kristoffer.carlsson@chalmers.se> (cherry picked from commit 2da42e0)
* Construct MulAddMul at gemm_wrapper! call sites * Add branches manually in MulAddMul constructor This is suggested by chethega in: #29634 (comment) * Update stdlib/LinearAlgebra/src/generic.jl Co-Authored-By: Kristoffer Carlsson <kristoffer.carlsson@chalmers.se> Co-authored-by: Kristoffer Carlsson <kristoffer.carlsson@chalmers.se>
I started working on "multiply-add" interface for computing C = αAB + βC for
LinearAlgebra
. The detail of the API is currently discussed in JuliaLang/LinearAlgebra.jl#473. I go ahead and implement the feature using the nameaddmul!
. I use this name only because it's easy to switch to other alternatives by simple replace. Let's do (and finish 😄) the naming discussion in JuliaLang/LinearAlgebra.jl#473.Remaining TODOs:
mul!
mul!
Matrix Multiplication API LinearAlgebra.jl#473EDIT: I updated the summary "inplace" since I noticed there were some now irrelevant comments. (You can click
edited ▼
above to see the old version.)