Skip to content

Conversation

@Sacha0
Copy link
Member

@Sacha0 Sacha0 commented Jan 5, 2018

#25287 modified a LAPACK least squares solver test to guard against failure due to poor conditioning of the test matrices. Unfortunately that modification proved insufficient as I ran into the motivating failure again locally

julia> srand(0xab6820ee372333e0a07585ed1a906b41);

julia> @testset "gelsy, gelsd" begin
           let elty = ComplexF64
               A = rand(elty, 10, 10)
               B = rand(elty, 10, 10)
               C, j = LAPACK.gelsd!(copy(A),copy(B))
               D, k = LAPACK.gelsy!(copy(A),copy(B))
               @test C ≈ D rtol=eps(cond(A))
           end
       end
gelsy, gelsd: Test Failed at REPL[34]:7
  Expression: ≈(C, D, rtol=eps(cond(A)))
   Evaluated: Complex{Float64}[-0.0262218+0.441148im 0.0346061-0.375153im … -0.0593423+0.645078im -0.166242+0.71145im; 0.392516+0.673879im 0.727514+0.518957im … 0.864918-0.22036im 1.0831+0.158312im; … ; 0.283946-0.078362im -0.161999-0.0885647im … -0.173693+0.390815im -0.474193-0.293889im; 0.41677-0.422742im 1.48165+0.0538742im … -0.153264-0.742447im 0.636379-0.666813im] ≈ Complex{Float64}[-0.0262218+0.441148im 0.0346061-0.375153im … -0.0593423+0.645078im -0.166242+0.71145im; 0.392516+0.673879im 0.727514+0.518957im … 0.864918-0.22036im 1.0831+0.158312im; … ; 0.283946-0.078362im -0.161999-0.0885647im … -0.173693+0.390815im -0.474193-0.293889im; 0.41677-0.422742im 1.48165+0.0538742im … -0.153264-0.742447im 0.636379-0.666813im] (rtol=3.552713678800501e-15)
Stacktrace:
 [1] macro expansion at ./REPL[34]:7 [inlined]
 [2] macro expansion at /Users/sacha/pkg/julia/usr/share/julia/site/v0.7/Test/src/Test.jl:980 [inlined]
 [3] top-level scope at ./REPL[34]:2
Test Summary: | Fail  Total
gelsy, gelsd  |    1      1
ERROR: Some tests did not pass: 0 passed, 1 failed, 0 errored, 0 broken.

This pull request relaxes the test tolerance just enough for the test to pass 100,000 consecutive repetitions:

julia> srand(0xab6820ee372333e0a07585ed1a906b41);

julia> @testset "gelsy, gelsd" begin
           @testset for elty in (Float32, Float64, ComplexF32, ComplexF64)
               for k in 1:10^5
                   A = rand(elty, 10, 10)
                   B = rand(elty, 10, 10)
                   C, j = LAPACK.gelsd!(copy(A),copy(B))
                   D, k = LAPACK.gelsy!(copy(A),copy(B))
                   @test C ≈ D rtol=4*eps(cond(A))
               end
           end
       end
Test Summary: |   Pass   Total
gelsy, gelsd  | 400000  400000
Test.DefaultTestSet("gelsy, gelsd", Any[Test.DefaultTestSet("elty = Float32", Any[], 100000, false), Test.DefaultTestSet("elty = Float64", Any[], 100000, false), Test.DefaultTestSet("elty = Complex{Float32}", Any[], 100000, false), Test.DefaultTestSet("elty = Complex{Float64}", Any[], 100000, false)], 0, false)

(Factors of 2 and 3 rather than 4 were insufficient.) Best!

@Sacha0 Sacha0 added linear algebra Linear algebra test This change adds or pertains to unit tests labels Jan 5, 2018
@Sacha0 Sacha0 requested a review from andreasnoack January 5, 2018 18:48
@Sacha0
Copy link
Member Author

Sacha0 commented Jan 6, 2018

(AV timeout seems unrelated.)

@Sacha0
Copy link
Member Author

Sacha0 commented Jan 6, 2018

CI being happy, Fredrik having approved, Andreas having suggested the approach on slack, and this being a trivial change, absent objections or requests for time I plan to merge this pull request tomorrow midday PT at the earliest. Best!

@fredrikekre fredrikekre merged commit 08620e5 into JuliaLang:master Jan 6, 2018
@Sacha0 Sacha0 deleted the gelsy branch January 6, 2018 18:22
@Sacha0
Copy link
Member Author

Sacha0 commented Jan 6, 2018

Thanks all! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

linear algebra Linear algebra test This change adds or pertains to unit tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants