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

LinearAlgebra: unstable return value, affected by lazy string & @show in error path #1064

Closed
IanButterworth opened this issue Apr 15, 2024 · 4 comments · Fixed by JuliaLang/julia#55143
Assignees
Labels
bug Something isn't working compiler:simd instruction-level vectorization complex Complex numbers existential crisis heisenbug This bug occurs unpredictably

Comments

@IanButterworth
Copy link
Member

Seen on this 1.10 backports PR JuliaLang/julia#53714 but the problematic commit was reverted on that PR feceefe (#53714)

Quoting JuliaLang/julia#53714 (comment)


I confirmed locally that this error was introduced by feceefe (#53714)

tr: Test Failed at /Users/ian/Documents/GitHub/alts/julia/stdlib/LinearAlgebra/test/symmetric.jl:224
  Expression: tr(aherm) == tr(Hermitian(aherm))
   Evaluated: 2.6298493464368606 + 0.0im == 2.62984934643686

which is weird for just a change switching to lazy strings?

diff --git a/stdlib/LinearAlgebra/src/LinearAlgebra.jl b/stdlib/LinearAlgebra/src/LinearAlgebra.jl
index 494572b1e8dd8..7b3ff8f0db53d 100644
--- a/stdlib/LinearAlgebra/src/LinearAlgebra.jl
+++ b/stdlib/LinearAlgebra/src/LinearAlgebra.jl
@@ -238,14 +238,14 @@ julia> LinearAlgebra.checksquare(A, B)
 """
 function checksquare(A)
     m,n = size(A)
-    m == n || throw(DimensionMismatch("matrix is not square: dimensions are $(size(A))"))
+    m == n || throw(DimensionMismatch(lazy"matrix is not square: dimensions are $(size(A))"))
     m
 end
 
 function checksquare(A...)
     sizes = Int[]
     for a in A
-        size(a,1)==size(a,2) || throw(DimensionMismatch("matrix is not square: dimensions are $(size(a))"))
+        size(a,1)==size(a,2) || throw(DimensionMismatch(lazy"matrix is not square: dimensions are $(size(a))"))
         push!(sizes, size(a,1))
     end
     return sizes

Specifically, reverting just this one makes tests pass
https://github.com/JuliaLang/julia/blob/97b0571cbd74cfc7959fe065b5dfc47d0636fc4e/stdlib/LinearAlgebra/src/LinearAlgebra.jl#L241

Also leaving it lazy and adding a @show m under that line makes test pass

@IanButterworth
Copy link
Member Author

Oscar here (grabbed Ian's phone)
@aviatesk can you look into this? It seems like we must have some horrible effects bug here (broken nothrow analysis possibly?)

@aviatesk
Copy link
Member

Thanks for the ping. I'll look into it.

@aviatesk
Copy link
Member

It appears that this error is caused by the @simd usage within tr(::Matrix{T}) where T rather than the effects modeling issues. It seems like using a lazy string in checksquare is not directly causing this error. This error happens non-deterministically, indicating that the issue is randomness caused by the macro.
MRE:

using LineaAlgebra, Test

for _ = 1:100; begin
    n = 10
    areal = randn(n,n)/2
    a = convert(Matrix{Float64}, areal)
    aherm = a' + a
    @test tr(aherm) == tr(Hermitian(aherm))

    aimg  = randn(n,n)/2
    areal = randn(n,n)/2
    a = convert(Matrix{ComplexF32}, complex.(areal, aimg))
    aherm = a' + a
    @test tr(aherm) == tr(Hermitian(aherm))
end; end

@aviatesk aviatesk added the compiler:simd instruction-level vectorization label Jul 16, 2024
aviatesk referenced this issue in JuliaLang/julia Jul 16, 2024
After investigating JuliaLang/julia#54090, I found that the issue was
not caused by the effects of `checksquare`, but by the use of the
`@simd` macro within `tr(::Matrix)`:
https://github.com/JuliaLang/julia/blob/0945b9d7740855c82a09fed42fbf6bc561e02c77/stdlib/LinearAlgebra/src/dense.jl#L373-L380

While simply removing the `@simd` macro was considered, the strict
left-to-right summation without `@simd` otherwise is not necessarily
more accurate, so I concluded that the problem lies in the test code,
which tests the (strict) equality of two different `tr` execution
results. I have modified the test code to use `≈` instead of `==`.

- fixes #54090
aviatesk referenced this issue in JuliaLang/julia Jul 16, 2024
…c.jl

After investigating JuliaLang/julia#54090, I found that the issue was
not caused by the effects of `checksquare`, but by the use of the
`@simd` macro within `tr(::Matrix)`:
https://github.com/JuliaLang/julia/blob/0945b9d7740855c82a09fed42fbf6bc561e02c77/stdlib/LinearAlgebra/src/dense.jl#L373-L380

While simply removing the `@simd` macro was considered, the strict
left-to-right summation without `@simd` otherwise is not necessarily
more accurate, so I concluded that the problem lies in the test code,
which tests the (strict) equality of two different `tr` execution
results. I have modified the test code to use `≈` instead of `==`.

- fixes #54090
@IanButterworth
Copy link
Member Author

Thanks for investigating & fixing. I had thought this was a return type change from real to complex, hence the alarm.

@IanButterworth IanButterworth changed the title LinearAlgebra: unstable return type, affected by lazy string & @show in error path LinearAlgebra: unstable return value, affected by lazy string & @show in error path Jul 16, 2024
aviatesk referenced this issue in JuliaLang/julia Jul 17, 2024
…#55143)

After investigating JuliaLang/julia#54090, I found that the issue was
not caused by the effects of `checksquare`, but by the use of the
`@simd` macro within `tr(::Matrix)`:

https://github.com/JuliaLang/julia/blob/0945b9d7740855c82a09fed42fbf6bc561e02c77/stdlib/LinearAlgebra/src/dense.jl#L373-L380

While simply removing the `@simd` macro was considered, the strict
left-to-right summation without `@simd` otherwise is not necessarily
more accurate, so I concluded that the problem lies in the test code,
which tests the (strict) equality of two different `tr` execution
results. I have modified the test code to use `≈` instead of `==`.

- fixes #54090
aviatesk referenced this issue in JuliaLang/julia Jul 17, 2024
…#55143)

After investigating JuliaLang/julia#54090, I found that the issue was
not caused by the effects of `checksquare`, but by the use of the
`@simd` macro within `tr(::Matrix)`:

https://github.com/JuliaLang/julia/blob/0945b9d7740855c82a09fed42fbf6bc561e02c77/stdlib/LinearAlgebra/src/dense.jl#L373-L380

While simply removing the `@simd` macro was considered, the strict
left-to-right summation without `@simd` otherwise is not necessarily
more accurate, so I concluded that the problem lies in the test code,
which tests the (strict) equality of two different `tr` execution
results. I have modified the test code to use `≈` instead of `==`.

- fixes #54090
KristofferC referenced this issue Nov 14, 2024
… (#55143)

After investigating JuliaLang/julia#54090, I found that the issue was
not caused by the effects of `checksquare`, but by the use of the
`@simd` macro within `tr(::Matrix)`:

https://github.com/JuliaLang/julia/blob/0945b9d7740855c82a09fed42fbf6bc561e02c77/stdlib/LinearAlgebra/src/dense.jl#L373-L380

While simply removing the `@simd` macro was considered, the strict
left-to-right summation without `@simd` otherwise is not necessarily
more accurate, so I concluded that the problem lies in the test code,
which tests the (strict) equality of two different `tr` execution
results. I have modified the test code to use `≈` instead of `==`.

- fixes #54090
@KristofferC KristofferC transferred this issue from JuliaLang/julia Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working compiler:simd instruction-level vectorization complex Complex numbers existential crisis heisenbug This bug occurs unpredictably
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants