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

Unnecessary add operations performed when multiplying in-place by diagonal matrix #47312

Closed
tristanmontoya opened this issue Oct 24, 2022 · 2 comments
Labels
linear algebra Linear algebra performance Must go faster

Comments

@tristanmontoya
Copy link

tristanmontoya commented Oct 24, 2022

Left-multiplication of a vector of length N by an N-by-N diagonal matrix should require a total of N multiplications. However, there are certain cases when (at least according to GFlops.jl) Julia appears to perform an additional N additions, effectively doubling the cost of such an operation. For example, the following code, which uses an out-of-place matrix-vector multiplication,

using LinearAlgebra, GFlops
A = Diagonal(rand(10))
x = rand(10)
@count_ops y = A * x

produces the following output:

Flop Counter: 10 flop
┌─────┬─────────┐
│     │ Float64 │
├─────┼─────────┤
│ mul │      10 │
└─────┴─────────┘

However, the equivalent in-place operation,

y = similar(x)
@count_ops mul!(y, A, x)

returns the following:

Flop Counter: 20 flop
┌─────┬─────────┐
│     │ Float64 │
├─────┼─────────┤
│ add │      10 │
│ mul │      10 │
└─────┴─────────┘

It seems like these additions are not necessary; the same operation could be done without any addition as follows:

function LinearAlgebra.mul!(y::AbstractVector, 
    A::Diagonal, x::AbstractVector)
    y[:] = A.diag .* x
    return y
end

Is this an oversight or is there a reason for such behaviour?

@oscardssmith oscardssmith added performance Must go faster linear algebra Linear algebra labels Oct 24, 2022
@mcabbott
Copy link
Contributor

What version is this on? GFlops.jl appears not to work on 1.9.

This might have been fixed by #44651, as the present code seems to check β and not add if it is zero:

@tristanmontoya
Copy link
Author

@mcabbott - you are right, this was fixed by that PR. My tests were on 1.7 but it's all fine on 1.8. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
linear algebra Linear algebra performance Must go faster
Projects
None yet
Development

No branches or pull requests

3 participants