[RFC] Clean up test/tridiag #23609
[RFC] Clean up test/tridiag #23609
Conversation
kshyatt
added
linear algebra
test/linalg/tridiag.jl
Outdated
| end | ||
| end | ||
| ε = eps(abs2(float(one(elty)))) |
There was a problem hiding this comment.
ε is not actually used in this file.
test/linalg/tridiag.jl
Outdated
| end | ||
| @testset "interconversion of Tridiagonal and SymTridiagonal" begin | ||
| @test Tridiagonal(dl, d, dl) == SymTridiagonal(d, dl) | ||
| @test SymTridiagonal(d, dl) == Tridiagonal(dl, d, dl) |
There was a problem hiding this comment.
This test is completely equivalent with the one on the line above.
There was a problem hiding this comment.
A bit of a theme in this file...
test/linalg/tridiag.jl
Outdated
| @test (A[3, 2] = A[3, 2]; A == fA) # test assignment on the subdiagonal | ||
| @test (A[2, 3] = A[2, 3]; A == fA) # test assignment on the superdiagonal | ||
| @test ((A[1, 3] = 0) == 0; A == fA) # test zero assignment off the main/sub/super diagonal | ||
| else |
There was a problem hiding this comment.
Add a comment else # mat_type == SymTridiagonal for clarity?
test/linalg/tridiag.jl
Outdated
| @test diag(A,0) == a | ||
| @test diag(A,n-1) == zeros(elty,1) | ||
| @test_throws ArgumentError diag(A,n+1) | ||
| @test diag(A, 1) == (mat_type == Tridiagonal ? du : dl) |
There was a problem hiding this comment.
This could test with ===. and likewise the next two tests.
| @test ((A[3, 3] = A[3, 3]) == A[3, 3]; A == fA) # test assignment on the main diagonal | ||
| @test_throws ArgumentError A[3, 2] = 1 # test assignment on the subdiagonal | ||
| @test_throws ArgumentError A[2, 3] = 1 # test assignment on the superdiagonal | ||
| end | ||
| end | ||
| @testset "Diagonal extraction" begin |
There was a problem hiding this comment.
Could also be worth adding a test for diag(A), i.e. without specifying a diagonal.
test/linalg/tridiag.jl
Outdated
| @test isa(floor.(Int,A), SymTridiagonal) | ||
| for f in (round, trunc, floor, ceil) | ||
| fds = [f.(d) for d in ds] | ||
| @test f.(A) == mat_type(fds...) |
There was a problem hiding this comment.
Perhaps remove the isa tests here and do @test (f.(A))::mat_type == mat_type(fds...) for even more code deletion.
test/linalg/tridiag.jl
Outdated
| @test isa(ceil.(Int,A), Tridiagonal) | ||
| @test floor.(Int,A) == floor.(Int,fA) | ||
| @test isa(floor.(Int,A), Tridiagonal) | ||
| if elty <: Real |
There was a problem hiding this comment.
Seems like these two ifs could be collapsed to one level with if elty <: LinAlg.BlasReal
|
Thanks for the detailed review, @fredrikekre. These are great suggestions <3! |
We had two loops doing very similar things. I tried to combine all the tests in a reasonable way. There's probably more that could be done here.