-
-
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
mul!(::Matrix{T}, ::Matrix{T}, ::AbstractTriangular{T, <:Matrix{T}}) where T<:BlasFloat
does not use BLAS
#42550
Comments
mul!(::StridedMatrix{T}, ::StridedMatrix{T}, ::AbstractTriangular{T, <:StridedMatrix{T}}) where T<:BlasFloat
does not use BLASmul!(::Matrix{T}, ::Matrix{T}, ::AbstractTriangular{T, <:Matrix{T}}) where T<:BlasFloat
does not use BLAS
Notice that three argument triangular matrix multiplication is not a BLAS operation. In BLAS this is an in-place operation see https://www.netlib.org/lapack/explore-html/d1/d54/group__double__blas__level3_gaf07edfbb2d2077687522652c9e283e1e.html#gaf07edfbb2d2077687522652c9e283e1e. I'm not in favor of adding extra dispatch versions of the I'm not sure why the generic fallback allocates, though. That shouldn't be necessary so it would be good to figure out why that happens. |
So, to do
am I getting that right? And, why not create a |
I don’t really understand this argument. The case I think it is really unintuitive that |
Because you are using the
Yes but it's probably not worth the effort. My main point is that you shouldn't use the three argument triangular matrix multiplication if you are trying to write optimized code. |
I still think @andreasnoack I think your argument is mainly an argument for adding a line to the docs that says users may want to consider using |
It uses a tiled algorithm. The allocations reported above are on Julia 1.6, where it uses global arrays and
|
I may be missing something, but given that we have julia/stdlib/LinearAlgebra/src/triangular.jl Lines 677 to 680 in 146de38
and julia/stdlib/LinearAlgebra/src/triangular.jl Lines 668 to 675 in 146de38
and that we even fully materialize tranposes/adjoints here julia/stdlib/LinearAlgebra/src/triangular.jl Lines 682 to 685 in 146de38
I feel like the missing methods are more like oversight rather than deliberately missing? Of course, fixing the issue requires adding what feels like a dozen new |
I just tried to refresh my memory on this topic and it looks like I agree with 2015 Andreas, see #10019. That issue was closed without introducing new methods. Then later that year #12812 was opened but it wasn't merged. Two years later I merged it without leaving a comment and I don't remember why I did so. Afterwards, we then had to add all the other methods to avoid ambiguities. So it looks like it might have been a bit random that these methods were introduced in the first. I don't like them and believe that people would be better off without them but given the current state, if somebody makes a PR with these methods and associated tests then I won't try to filibuster it (even though it adds extra compilation to an already very compilation heavy and therefore very slow test file.) |
Noticed a similar (?) issue with
On my M1 Mac with Julia 1.9rc2 this yields a bunch of type combinations where
But there are also some cases where
|
I think the main reason for this is that *(A::HermOrSym, B::HermOrSym) = A * copyto!(similar(parent(B)), B) and subsequently use specialized BLAS routines. In contrast, Another aspect is that you measure computation times with output |
Thanks for your reply! I agree with your arguments, and many cases already seem to be addressed by your PR. I'm just not happy with
I guess it's not ideal to mutate |
See https://discourse.julialang.org/t/mul-matrix-matrix-uppertriangular-is-really-slow/69368. CC @Luapulu
mul!
doesn’t use BLAS when the second matrix is anAbstractTriangular
:However, the
*
methods all use BLAS:The text was updated successfully, but these errors were encountered: