JuliaStats / PDMats.jl

Uniform Interface for positive definite matrices of various structures
Other
104 stars 43 forks source link

Implement matrix square root for PDMats #158

Closed wil-j-wil closed 2 years ago

wil-j-wil commented 2 years ago

The matrix square root, LinearAlgebra.sqrt(), is not currently implemented for PDMats, i.e., sqrt(::AbstractPDMat) throws an error.

This PR implements this by applying sqrt(Matrix(A)) in the generic case, sqrt(A.mat) where possible, plus specific cases for PDiagMat, ScaleMat and PDSparseMat.

Tests also written which check the specific cases return the same result as sqrt(Matrix(A)), which I believe is sufficient.

codecov-commenter commented 2 years ago

Codecov Report

Merging #158 (d87bda1) into master (84df00b) will increase coverage by 0.09%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #158      +/-   ##
==========================================
+ Coverage   90.14%   90.24%   +0.09%     
==========================================
  Files           8        8              
  Lines         406      410       +4     
==========================================
+ Hits          366      370       +4     
  Misses         40       40              
Impacted Files Coverage Δ
src/pdiagmat.jl 98.96% <100.00%> (+0.01%) :arrow_up:
src/pdmat.jl 98.21% <100.00%> (+0.03%) :arrow_up:
src/pdsparsemat.jl 94.23% <100.00%> (+0.11%) :arrow_up:
src/scalmat.jl 96.61% <100.00%> (+0.05%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 84df00b...d87bda1. Read the comment docs.

wil-j-wil commented 2 years ago

@devmotion thanks a lot for the review. I've made some changes. Let me know if you have any other thoughts / requests.