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

Optimize real matrix * complex vector by reinterpreting as real #44011

Merged
merged 1 commit into from
Feb 8, 2022

Conversation

jishnub
Copy link
Contributor

@jishnub jishnub commented Feb 2, 2022

julia> M = rand(Float32, 1000, 1000); V = rand(ComplexF32, size(M,2))

julia> C32_1 = @btime $M * $V; # current master
  714.915 μs (1 allocation: 7.94 KiB)

julia> Revise.track(LinearAlgebra)

julia> C32_2 = @btime $M * $V; # this PR
  342.096 μs (1 allocation: 7.94 KiB)

julia> C32_1  C32_2
true

and similarly for Float64. Currently this hits generic_matvecmul!, which is presumably slow.

I had also tried to add a similar method for real matrix * complex matrix, but for some reason that I don't understand this makes BLAS.gemm! (consequently M1 * M2) worse. I have removed that method from this PR.

Sorry about the numerous whitespace changes introduced by the VSCode plugin, however hopefully these will improve readability. I haven't figured out how to disable this.

@N5N3 N5N3 added linear algebra Linear algebra performance Must go faster labels Feb 2, 2022
@jishnub jishnub requested a review from N5N3 February 2, 2022 14:25
@N5N3 N5N3 requested a review from dkarrasch February 2, 2022 14:31
@N5N3

This comment has been minimized.

@N5N3 N5N3 self-requested a review February 2, 2022 15:17
base/reinterpretarray.jl Outdated Show resolved Hide resolved
@N5N3

This comment was marked as off-topic.

@dkarrasch
Copy link
Member

Sorry about the numerous whitespace changes introduced by the VSCode plugin, however hopefully these will improve readability. I haven't figured out how to disable this.

You could commit only the relevant changes and leave all others outside the PR. It's quite challenging finding the new code.

Is this ready now? The code part looks good to me, so do you think there is something left untested?

@jishnub
Copy link
Contributor Author

jishnub commented Feb 8, 2022

I think it's ready, as I've rebased it on #44027 which is fairly well tested. Further tests for strides are perhaps not necessary in this PR.

Apologies for the whitespace again. Perhaps in the future I'll sequester whitespace changes in a single commit, which would make code changes easier to find.

@dkarrasch dkarrasch merged commit e865d33 into JuliaLang:master Feb 8, 2022
@jishnub jishnub deleted the realmatmulcomplexvec branch February 8, 2022 11:33
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

Successfully merging this pull request may close these issues.

3 participants