JuliaStats / PDMats.jl

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

Fix logdet(PDiagMat(zeros(0))) #139

Closed st-- closed 2 years ago

st-- commented 2 years ago

Resolves #138.

~The init keyword argument to sum is only available from julia 1.6, so there's a fallback implementation for older versions.~ Edit: init keyword isn't type-stable, anyways, so dropping the version-branching.

codecov-commenter commented 2 years ago

Codecov Report

Merging #139 (182782f) into master (213c37a) will increase coverage by 0.30%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #139      +/-   ##
==========================================
+ Coverage   88.31%   88.62%   +0.30%     
==========================================
  Files           8        8              
  Lines         351      378      +27     
==========================================
+ Hits          310      335      +25     
- Misses         41       43       +2     
Impacted Files Coverage Δ
src/pdiagmat.jl 97.75% <100.00%> (+0.25%) :arrow_up:
src/utils.jl 91.66% <0.00%> (-2.09%) :arrow_down:
src/pdmat.jl 96.15% <0.00%> (+0.07%) :arrow_up:
src/scalmat.jl 94.11% <0.00%> (+0.11%) :arrow_up:
src/pdsparsemat.jl 94.33% <0.00%> (+0.33%) :arrow_up:
src/generics.jl 81.48% <0.00%> (+0.71%) :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 213c37a...182782f. Read the comment docs.

st-- commented 2 years ago

I was originally thinking of changing it to make use of @testsets which avoids the issue of a new test changing random numbers and making something fall over elsewhere... would you be interested in seeing that (happy to do it later, in a separate PR as well)?

devmotion commented 2 years ago

I was originally thinking of changing it to make use of @testsets which avoids the issue of a new test changing random numbers and making something fall over elsewhere... would you be interested in seeing that (happy to do it later, in a separate PR as well)?

Yes, it would be useful to do this :+1: I think it's better to do it in a separate PR.