From 35e8d2e5424888f9b17763d24c97b8fd4603c041 Mon Sep 17 00:00:00 2001 From: ST John Date: Wed, 29 Jan 2025 11:23:48 +0200 Subject: [PATCH 1/2] remove copy on property access --- src/cholesky.jl | 8 ++++---- test/cholesky.jl | 24 +++++++++++++++++++++--- 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/src/cholesky.jl b/src/cholesky.jl index 03f7c27..cc9860c 100644 --- a/src/cholesky.jl +++ b/src/cholesky.jl @@ -65,7 +65,7 @@ julia> C.U ⋅ ⋅ 3.0 julia> C.L -3×3 LowerTriangular{Float64, Matrix{Float64}}: +3×3 LowerTriangular{Float64, Adjoint{Matrix{Float64}}}: 2.0 ⋅ ⋅ 6.0 1.0 ⋅ -8.0 5.0 3.0 @@ -530,7 +530,7 @@ julia> C.U ⋅ ⋅ 3.0 julia> C.L -3×3 LowerTriangular{Float64, Matrix{Float64}}: +3×3 LowerTriangular{Float64, Adjoint{Matrix{Float64}}}: 2.0 ⋅ ⋅ 6.0 1.0 ⋅ -8.0 5.0 3.0 @@ -668,14 +668,14 @@ function _choleskyUfactor(Cfactors, Cuplo) if Cuplo === 'U' return UpperTriangular(Cfactors) else - return copy(LowerTriangular(Cfactors)') + return LowerTriangular(Cfactors)' end end function _choleskyLfactor(Cfactors, Cuplo) if Cuplo === 'L' return LowerTriangular(Cfactors) else - return copy(UpperTriangular(Cfactors)') + return UpperTriangular(Cfactors)' end end diff --git a/test/cholesky.jl b/test/cholesky.jl index 6ba7243..c897bb7 100644 --- a/test/cholesky.jl +++ b/test/cholesky.jl @@ -595,9 +595,9 @@ end B = cholesky(A) B32 = cholesky(Float32.(A)) @test B isa Cholesky{Float16, Matrix{Float16}} - @test B.U isa UpperTriangular{Float16, Matrix{Float16}} - @test B.L isa LowerTriangular{Float16, Matrix{Float16}} - @test B.UL isa UpperTriangular{Float16, Matrix{Float16}} + @test B.U isa UpperTriangular{Float16, <:AbstractMatrix{Float16}} + @test B.L isa LowerTriangular{Float16, <:AbstractMatrix{Float16}} + @test B.UL isa UpperTriangular{Float16, <:AbstractMatrix{Float16}} @test B.U ≈ B32.U @test B.L ≈ B32.L @test B.UL ≈ B32.UL @@ -658,4 +658,22 @@ end end end +@testset "accessing both L and U factors should avoid allocations" begin + n = 30 + A = rand(n, n) + Apd = A'A + allowed_cost_of_overhead = 32 + @assert sizeof(Apd) > 4allowed_cost_of_overhead # ensure that we could positively identify extra copies + + for uplo in (:L, :U) + C = Symmetric(Apd, uplo) + for val in (Val(true), Val(false)) + B = cholesky(C, val) + B.L, B.U # access once to ensure the accessor is compiled already + @test (@allocated B.L) <= allowed_cost_of_overhead + @test (@allocated B.U) <= allowed_cost_of_overhead + end + end +end + end # module TestCholesky From 15249bff0582f608f8de9a3dee3e9a83d27bb85f Mon Sep 17 00:00:00 2001 From: ST John Date: Mon, 3 Feb 2025 11:51:03 +0200 Subject: [PATCH 2/2] test fixes --- test/cholesky.jl | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/cholesky.jl b/test/cholesky.jl index c897bb7..6a3fa45 100644 --- a/test/cholesky.jl +++ b/test/cholesky.jl @@ -604,9 +604,9 @@ end @test Matrix(B) ≈ A B = cholesky(A, RowMaximum()) B32 = cholesky(Float32.(A), RowMaximum()) - @test B isa CholeskyPivoted{Float16,Matrix{Float16}} - @test B.U isa UpperTriangular{Float16, Matrix{Float16}} - @test B.L isa LowerTriangular{Float16, Matrix{Float16}} + @test B isa CholeskyPivoted{Float16, <:AbstractMatrix{Float16}} + @test B.U isa UpperTriangular{Float16, <:AbstractMatrix{Float16}} + @test B.L isa LowerTriangular{Float16, <:AbstractMatrix{Float16}} @test B.U ≈ B32.U @test B.L ≈ B32.L @test Matrix(B) ≈ A @@ -667,7 +667,7 @@ end for uplo in (:L, :U) C = Symmetric(Apd, uplo) - for val in (Val(true), Val(false)) + for val in (NoPivot(), RowMaximum()) B = cholesky(C, val) B.L, B.U # access once to ensure the accessor is compiled already @test (@allocated B.L) <= allowed_cost_of_overhead