search

AtheMathmo / rulinalg

A linear algebra library written in Rust
https://crates.io/crates/rulinalg
MIT License
288 stars 58 forks source link

Cholesky: Fix misleading error message #189

Open vks opened 7 years ago

vks commented 7 years ago

The diagonals of the decomposed matrix were negative, which means the original matrix was not positive definite (even if the diagonal was positive).

AtheMathmo commented 7 years ago

I think this is a good change from the user perspective! @Andlon am I missing anything here?

Andlon commented 7 years ago

Thanks for the PR @vks! Yes, I agree that the original error message is indeed misleading.

Note that the documentation for the method also needs to be updated as it has a similar wording as the original error message. Would you mind changing this as well @vks? In this case, we need to be a bit careful with how it is worded.

I see now that also the other documented error message is misleading, since it states that:

A diagonal entry is effectively zero to working precision.

As you pointed out, it is not the diagonal entry in the original matrix, but the diagonal entry in the decomposition which is being carried out. Perhaps just replace this by "The matrix is effectively singular to working precision"?