Skip to content
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

Speed of multiplication of Diagonal #19

Closed
vincentcp opened this issue Jan 24, 2019 · 6 comments
Closed

Speed of multiplication of Diagonal #19

vincentcp opened this issue Jan 24, 2019 · 6 comments

Comments

@vincentcp
Copy link

The Mul of two Diagonals might create a full matrix somewhere when applied to a vector.

julia> using LinearAlgebra, LazyArrays
julia> d = Diagonal(rand(1000));
julia> b = rand(1000);
julia> d*d*b;@time d*d*b;
  0.000004 seconds (7 allocations: 16.047 KiB)
julia> M = Mul(d,d);
julia> M*b;@time M*b;
  0.691181 seconds (18 allocations: 7.638 MiB, 0.84% gc time)
julia> LazyArrays.materialize(Mul(M.factors[1], M.factors[2])) isa Array
true
@vincentcp
Copy link
Author

Could

function default_blasmul!(α, A::AbstractMatrix, B::AbstractMatrix, β, C::AbstractMatrix)
and
function default_blasmul!(α, A::AbstractMatrix, B::AbstractVector, β, C::AbstractVector)
use mul! instead?

Or should materialize! be implemented for Diagonal in order to get an efficient materialize?

@dlfivefifty
Copy link
Member

No. The whole point is to support the 5-argument version (αAx + β*y) which mul! does not.

@vincentcp
Copy link
Author

Ok, I understand. Which functions need to be specialised for Diagonal/DiagonalLayout to get the LazyArrays machinery working?

@dlfivefifty
Copy link
Member

It loosely follows the broadcast interface, so you need to

  1. Overload similar
  2. Overload _copyto!

Banded matrices give a nice example: https://github.com/JuliaMatrices/BandedMatrices.jl/blob/master/src/generic/matmul.jl

Note that many cases first lower to the 5-argument version α*A*x + β*y, which is implemented via MulAdd. For these you overload materialize! and overwrite the y argument with the value.

@dlfivefifty
Copy link
Member

This is closer to being fixed, though now the issue is that its creating a (slow) SparseMatrixCSC:

julia> apply(*,d,d) |> typeof
SparseMatrixCSC{Float64,Int64}

And we still neeed a special materialize! for diagonal*diagonal.

@dlfivefifty
Copy link
Member

This has probably been fixed and is now in ArrayLayouts.jl

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants