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

failure on v1.11-dev #126

Closed
stevengj opened this issue Oct 26, 2023 · 5 comments · Fixed by #127
Closed

failure on v1.11-dev #126

stevengj opened this issue Oct 26, 2023 · 5 comments · Fixed by #127
Labels
bug Something isn't working

Comments

@stevengj
Copy link
Member

Currently failing on v"1.11.0-DEV.745" with:

exp.jl: Error During Test at /Users/stevenj/.julia/dev/SkewLinearAlgebra/test/runtests.jl:224
  Test threw exception
  Expression: tan(B) ≈ tan(A)
  StackOverflowError:
  Stacktrace:
   [1] SkewHermitian{Float64, SkewHermitian{Float64, Matrix{Float64}}}(A::SkewHermitian{Float64, Matrix{Float64}}) (repeats 79984 times)
     @ SkewLinearAlgebra ~/.julia/dev/SkewLinearAlgebra/src/skewhermitian.jl:57
exp.jl: Error During Test at /Users/stevenj/.julia/dev/SkewLinearAlgebra/test/runtests.jl:224
  Test threw exception
  Expression: tan(B) ≈ tan(A)
  StackOverflowError:
  Stacktrace:
   [1] SkewHermitian{Float64, SkewHermitian{Float64, Matrix{Float64}}}(A::SkewHermitian{Float64, Matrix{Float64}}) (repeats 79984 times)
This was referenced Oct 26, 2023
@stevengj
Copy link
Member Author

Errors occur for 1x1 and 2x2 matrices, it looks like:

julia> A = skewhermitian(rand(2,2))
2×2 SkewHermitian{Float64, Matrix{Float64}}:
 0.0       -0.259484
 0.259484   0.0

julia> tan(A)
ERROR: StackOverflowError:
Stacktrace:
 [1] SkewHermitian{Float64, SkewHermitian{Float64, Matrix{Float64}}}(A::SkewHermitian{Float64, Matrix{Float64}}) (repeats 79984 times)
   @ SkewLinearAlgebra ~/.julia/dev/SkewLinearAlgebra/src/skewhermitian.jl:57

@stevengj
Copy link
Member Author

stevengj commented Oct 26, 2023

Looks like we just need to add a constructor SkewHermitian(A::SkewHermitian) = A

Though fixing that exposes another failure on 1.11 in this test for (T, n) = (ComplexF32, 1):

exp.jl: Test Failed at /Users/stevenj/.julia/dev/SkewLinearAlgebra/test/runtests.jl:225
  Expression: tan(B) ≈ tan(A)
   Evaluated: ComplexF32[0.0f0 + 0.7081147f0im;;] ≈ ComplexF32[0.0f0 + 0.0f0im;;]

In particular, we have this incorrect result on 1.11:

julia> tan(skewhermitian([0.1im;;]))
1×1 SkewHermitian{ComplexF64, Matrix{ComplexF64}}:
 0.0 + 0.0im

whereas it gives the correct result [0.0 + 0.0996679946249559im;;] on 1.9.

Indeed, SkewHermitian(skewhermitian(rand(4,4))) gives a stack overflow even on earlier versions, so it seems like the tan code is somehow triggering a new code path on 1.11.

@stevengj
Copy link
Member Author

stevengj commented Oct 26, 2023

In particular, I'm getting the following wrong result on 1.11 that is the source of the tan error:

julia> C = Hermitian([1.0+0im;;]); S = SkewHermitian([0+1.0im;;])
1×1 SkewHermitian{ComplexF64, Matrix{ComplexF64}}:
 0.0 + 1.0im

julia> C \ S
1×1 SkewHermitian{ComplexF64, Matrix{ComplexF64}}:
 0.0 + 0.0im

In both 1.9 and 1.11 it's calling a Base method here, so this might be an error in Base on 1.11:

julia> @which C \ S
\(A::Union{Hermitian{T, S}, Symmetric{T, S}} where {T, S}, B::AbstractMatrix)
     @ LinearAlgebra ~/Documents/Code/julia/usr/share/julia/stdlib/v1.11/LinearAlgebra/src/symmetric.jl:584

@stevengj
Copy link
Member Author

stevengj commented Oct 26, 2023

It looks like the problem was exposed to the Diagonal optimization in JuliaLang/julia#48189, which effectively uses Diagonal(C) \ S in 1.11 (and maybe 1.10)? Even in 1.9, if we explicitly use Diagonal then it fails:

julia> C \ S
1×1 Matrix{ComplexF64}:
 0.0 + 1.0im

julia> Diagonal(C) \ S
1×1 SkewHermitian{ComplexF64, Matrix{ComplexF64}}:
 0.0 + 0.0im

@stevengj stevengj added the bug Something isn't working label Oct 26, 2023
@smataigne
Copy link
Collaborator

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants