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

Fix: Complex SubArray times real Matrix #29246

Merged

Conversation

jarlebring
Copy link
Contributor

This fixes #29224 concerning multiplication of a complex SubArray with a real matrix. The only substantial change is

gemm_wrapper!(Cfl, 'N', 'N', Afl, B)

to

mul!(Cfl, Afl, B)

This allows dispatch to determine if we can use gemm for the reinterpreted matrix, or just do it through the fallback generic_matmatmul!.

@testset "Combination $T1 $T2 $arg1_real $arg2_real" for arg2_real in (true,false)
A0 = reshape(Vector{T1}(1:25),5,5) .+
(arg1_real ? 0 : 1im*reshape(Vector{T1}(-3:21),5,5))
A = view(A0,1:2,1:2)
Copy link
Member

@andreasnoack andreasnoack Sep 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe also add a test with view(A0,:,1:2) to test the code path that dispatches to BLAS.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Member

@andreasnoack andreasnoack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, would you mind making the same change in the two other places where we use the reinterpret trick?

@jarlebring
Copy link
Contributor Author

jarlebring commented Sep 18, 2018

Sure. For the record: This leads to another situation the trick is used and it also does not work:

julia> A=view(complex.(randn(2,2),randn(2,2)),1:2,1:2);
julia> B=transpose(randn(2,2))
julia> A*B

@jarlebring
Copy link
Contributor Author

Done. All three reinterpret tricks are now changed.

@andreasnoack andreasnoack merged commit 82503cd into JuliaLang:master Sep 25, 2018
@KristofferC KristofferC added linear algebra Linear algebra bugfix This change fixes an existing bug backport pending 1.0 labels Sep 30, 2018
KristofferC pushed a commit that referenced this pull request Sep 30, 2018
* gemm_wrapper! -> mul! (#29224)

* testing for #29224

* code review update: More tests

* Complex times real reinterpret trick fix for vectors and transposed matrices

(cherry picked from commit 82503cd)
KristofferC pushed a commit that referenced this pull request Feb 11, 2019
* gemm_wrapper! -> mul! (#29224)

* testing for #29224

* code review update: More tests

* Complex times real reinterpret trick fix for vectors and transposed matrices

(cherry picked from commit 82503cd)
@jarlebring jarlebring deleted the complex_reshape_times_real_mat_fix branch March 4, 2019 14:30
KristofferC pushed a commit that referenced this pull request Feb 20, 2020
* gemm_wrapper! -> mul! (#29224)

* testing for #29224

* code review update: More tests

* Complex times real reinterpret trick fix for vectors and transposed matrices

(cherry picked from commit 82503cd)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix This change fixes an existing bug linear algebra Linear algebra
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SubArray x Matrix fails for different types
3 participants