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

axpby! no longer works with Hermitian matrices #44734

Closed
KristofferC opened this issue Mar 24, 2022 · 3 comments · Fixed by #44736
Closed

axpby! no longer works with Hermitian matrices #44734

KristofferC opened this issue Mar 24, 2022 · 3 comments · Fixed by #44736
Labels
linear algebra Linear algebra regression Regression in behavior compared to a previous version
Milestone

Comments

@KristofferC
Copy link
Member

using LinearAlgebra
a = rand(5,5)
A = Hermitian(a' * a)
B = similar(a)
LinearAlgebra.BLAS.axpby!(1, A, 1.0, B)

1.7:

julia> LinearAlgebra.BLAS.axpby!(1, A, 1.0, B)
5×5 Matrix{Float64}:
 1.66822   1.02419   0.774382  1.26736   1.48034

1.8:

julia> LinearAlgebra.BLAS.axpby!(1, A, 1.0, B)
ERROR: MethodError: no method matching strides(::Hermitian{Float64, Matrix{Float64}})
Closest candidates are:
  strides(::SubArray) at subarray.jl:356
  strides(::Base.ReinterpretArray{T, N, S, A, true} where {T, N, S, A<:(AbstractArray{S})}) at reinterpretarray.jl:155
  strides(::Base.ReinterpretArray{T, N, S, A, false} where {T, N, S, A<:AbstractArray{S, N}}) at reinterpretarray.jl:165
  ...
Stacktrace:
 [1] vec_pointer_stride
   @ ~/julia/usr/share/julia/stdlib/v1.8/LinearAlgebra/src/blas.jl:173 [inlined]
 [2] vec_pointer_stride
   @ ~/julia/usr/share/julia/stdlib/v1.8/LinearAlgebra/src/blas.jl:172 [inlined]
 [3] axpby!(alpha::Int64, x::Hermitian{Float64, Matrix{Float64}}, beta::Float64, y::Matrix{Float64})
   @ LinearAlgebra.BLAS ~/julia/usr/share/julia/stdlib/v1.8/LinearAlgebra/src/blas.jl:590
 [4] top-level scope
   @ REPL[9]:1

Probably introduced in #42957. cc @N5N3

@KristofferC KristofferC added regression Regression in behavior compared to a previous version linear algebra Linear algebra labels Mar 24, 2022
@KristofferC KristofferC added this to the 1.8 milestone Mar 24, 2022
@N5N3
Copy link
Member

N5N3 commented Mar 24, 2022

On 1.7, LinearAlgebra.BLAS.axpby!(1, A, 1.0, B) use a general fallback thus not BLAS based.
As axpby!(1.0, 1.:10, 1.0, 1.:10) fallbacks to BLAS-based fallback on 1.7. I'm not sure previous behavior has no problem.

julia> VERSION
v"1.7.2"

julia> axpby!(1.0, 1.:10, 1.0, 1.:10)
ERROR: MethodError: no method matching strides(::StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64})

I'd expect BLAS.f(....) always based on BLAS. Thus I suggest to distinguish BLAS.axpy! and LinearAlgebra.axpy!.
But I guess that change is not acceptable for 1.8.

@KristofferC
Copy link
Member Author

Not sure, I just saw a PkgEval failure from it. But I agree that it makes sense to separate the BLAS and LinearAlgebra part in the long run. For now, is there an easy way to restore the functionality?

N5N3 added a commit to N5N3/julia that referenced this issue Mar 24, 2022
@N5N3
Copy link
Member

N5N3 commented Mar 24, 2022

PR opened.

N5N3 added a commit to N5N3/julia that referenced this issue Mar 25, 2022
N5N3 added a commit to N5N3/julia that referenced this issue Mar 25, 2022
KristofferC pushed a commit that referenced this issue Mar 25, 2022
KristofferC pushed a commit that referenced this issue Mar 25, 2022
Fix #44734.

(cherry picked from commit 68e2969)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
linear algebra Linear algebra regression Regression in behavior compared to a previous version
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants