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

Added multiplication of transpose / adjoint matrices by diagonal matrices #2518

Merged
merged 5 commits into from
Oct 7, 2024

Conversation

amontoison
Copy link
Member

Updated version of #2482

@maleadt maleadt added enhancement New feature or request cuda libraries Stuff about CUDA library wrappers. needs changes Changes are needed. labels Oct 7, 2024
@maleadt
Copy link
Member

maleadt commented Oct 7, 2024

CI failures look related.

test/libraries/cublas.jl Outdated Show resolved Hide resolved
@amontoison
Copy link
Member Author

Julia wants to dispatch to another method with Transpose and Adjoint:

mul!(d_YA, d_Y, transpose(d_A))

If you want to allow scalar iteration, use `allowscalar` or `@allowscalar`
to enable scalar iteration globally or for the operations in question.
Stacktrace:
  [1] error(s::String)
    @ Base ./error.jl:35
  [2] errorscalar(op::String)
    @ GPUArraysCore ~/.julia/packages/GPUArraysCore/GMsgk/src/GPUArraysCore.jl:155
  [3] _assertscalar(op::String, behavior::GPUArraysCore.ScalarIndexing)
    @ GPUArraysCore ~/.julia/packages/GPUArraysCore/GMsgk/src/GPUArraysCore.jl:128
  [4] assertscalar(op::String)
    @ GPUArraysCore ~/.julia/packages/GPUArraysCore/GMsgk/src/GPUArraysCore.jl:116
  [5] getindex
    @ ~/.julia/packages/GPUArrays/qt4ax/src/host/indexing.jl:50 [inlined]
  [6] macro expansion
    @ ~/Applications/julia/julia-1.10.5/share/julia/stdlib/v1.10/LinearAlgebra/src/diagonal.jl:318 [inlined]
  [7] macro expansion
    @ ./simdloop.jl:77 [inlined]
  [8] __muldiag!(out::CuArray{Float64, 2, CUDA.DeviceMemory}, D::Diagonal{ComplexF64, CuArray{…}}, B::Transpose{ComplexF64, CuArray{…}}, _add::LinearAlgebra.MulAddMul{true, true, Bool, Bool})
    @ LinearAlgebra ~/Applications/julia/julia-1.10.5/share/julia/stdlib/v1.10/LinearAlgebra/src/diagonal.jl:317
  [9] _mul_diag!
    @ ~/Applications/julia/julia-1.10.5/share/julia/stdlib/v1.10/LinearAlgebra/src/diagonal.jl:391 [inlined]
 [10] _mul!
    @ ~/Applications/julia/julia-1.10.5/share/julia/stdlib/v1.10/LinearAlgebra/src/diagonal.jl:397 [inlined]
 [11] mul!
    @ ~/Applications/julia/julia-1.10.5/share/julia/stdlib/v1.10/LinearAlgebra/src/bidiag.jl:427 [inlined]
 [12] mul!(C::CuArray{Float64, 2, CUDA.DeviceMemory}, A::Diagonal{ComplexF64, CuArray{ComplexF64, 1, CUDA.DeviceMemory}}, B::Transpose{ComplexF64, CuArray{ComplexF64, 2, CUDA.DeviceMemory}})
    @ LinearAlgebra ~/Applications/julia/julia-1.10.5/share/julia/stdlib/v1.10/LinearAlgebra/src/matmul.jl:237
 [13] top-level scope
    @ REPL[71]:1
Some type information was truncated. Use `show(err)` to see complete types.

@dkarrasch Do you have an idea how I can fix it?
Should I add a method for generic_matmatmul! instead?

@dkarrasch
Copy link
Contributor

Tests seem to pass? Or where is the problem? I think you're overloading the right method, which is the three-arg mul! in line [12]. That one is completely generic, so any method that specifies the arguments (to arrays that it owns) should have precedence.

@amontoison
Copy link
Member Author

I forgot to change the branch when I tested the PR on a cluster...
The issue was between the chair and the screen (me) 😄
Thanks for confirming that I overloaded the right method.

@amontoison amontoison merged commit 2486af3 into JuliaGPU:master Oct 7, 2024
1 check passed
@amontoison amontoison deleted the adjoint_diag_mul branch October 7, 2024 19:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cuda libraries Stuff about CUDA library wrappers. enhancement New feature or request needs changes Changes are needed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants