Define functions for Cholesky
#168
Conversation
Codecov ReportAttention:
📢 Thoughts on this report? Let us know!. |
|
It might be worth extending addition as well. Below uses a full rank update to add two matrices directly using their Cholesky decompositions. Obviously slower than just adding the matrices if you have them, but if you're starting and ending with the Cholesky this seems the way to go. IIRC this is more numerically stable as well. Unfortunately, this doesn't work for using LinearAlgebra, PDMats, BenchmarkTools
function fullrankupdate(c1::Cholesky, c2::Cholesky)
cout = copy(c1)
return fullrankupdate!(cout, c2)
end
function fullrankupdate!(c1::Cholesky, c2::Cholesky)
for c in eachcol(PDMats.chol_lower(c2))
lowrankupdate!(c1, c)
end
return c1
end
c1 = cholesky([1.0 0.5; 0.5 2.0])
c2 = cholesky([2.0 0.2; 0.2 1.0])
@btime PDMat($c1) + PDMat($c2);
@btime fullrankupdate($c1, $c2);
@assert PDMats.chol_lower(cholesky(PDMat(c1) + PDMat(c2))) ≈ PDMats.chol_lower(fullrankupdate(c1, c2))
|
|
I don't think this should be done in this PR. In general, I'm not sure if it's a good idea at all to define addition for |
test/specialarrays.jl
Outdated
| @@ -52,6 +55,33 @@ using StaticArrays | |||
| @test Xt_invA_X(A, Y) isa SMatrix{10, 10, Float64} | |||
| @test Xt_invA_X(A, Y) ≈ Matrix(Y)' * (Matrix(A) \ Matrix(Y)) | |||
| end | |||
|
|
|||
| # TODO: Fix for `PDMat` and `PDiagMat` | |||
There was a problem hiding this comment.
is there an issue tracking this TODO?
There was a problem hiding this comment.
No, no issue. But it will be fixed by #183 🙂
| size_of_one_copy = sizeof(C) | ||
| @assert size_of_one_copy > 100 # ensure the matrix is large enough that few-byte allocations don't matter | ||
|
|
||
| @test chol_lower(C) ≈ chol_upper(C)' | ||
| @test (@allocated chol_lower(C)) < 1.05 * size_of_one_copy # allow 5% overhead | ||
| @test (@allocated chol_upper(C)) < 1.05 * size_of_one_copy | ||
|
|
||
| X = randn(d, 10) |
There was a problem hiding this comment.
d should be 100 and used above as well. It's a remaining (final?) issue from the merge commit afaf699 that broke tests a bit.
This PR defines
dimwhiten,whiten!unwhiten,unwhiten!quad,quad!invquad,invquad!X_A_Xt,Xt_A_X,X_invA_Xt,Xt_invA_Xfor
Cholesky.Ultimately, I think we should forward (some of) the generic fallbacks for
AbstractMatrixto these functions (I noticed they could also be cleaned a bit since e.g.PDMats.jl/src/generics.jl
Line 124 in 8d88c15
x::AbstractMatrixdue to the definition inPDMats.jl/src/generics.jl
Line 125 in 8d88c15
StaticArrays that out-of-place methods fall back to in-place methods). However, I assumed it would be cleaner to not touch the dispatch pipeline in this PR but only add and test definitions forCholesky.This PR also provides a solution to #140 @st--: If one does not want to construct the full matrix, it is possible to work with
Choleskydirectly (at least when using the PDMats-specific API).