gonum / matrix

Matrix packages for the Go language [DEPRECATED]
446 stars 53 forks source link

mat64: add Cholesky.Reset and panic on invalid Cholesky #378

Closed vladimir-ch closed 8 years ago

vladimir-ch commented 8 years ago

Addresses #377

kortschak commented 8 years ago

LGTM but wait for @btracey.

vladimir-ch commented 8 years ago

I modified this a bit: if Cholesky.chol is Reset whenever an invalid factorization is created, we can use it instead of the bool field to indicate validity of the factorization. PTAL

vladimir-ch commented 8 years ago

PTAL

btracey commented 8 years ago

I think this is good. I only wonder about things like Size and the extraction routines. Let's say you do a rank-one update to a Cholesy matrix, and it's not positive definite. Could it be useful to extract the matrix to see what it looks like? Or is the algorithm somewhat unpredictable, and thus the errors should be handled in different ways?

vladimir-ch commented 8 years ago

In case of Factorize, if the matrix is not positive definite, Lapack returns some information in info. We don't expose this information in mat64. Also, Lapack says that "... the factorization could not be completed.". I'm not sure if in this situation it is useful to enable extraction of the factorization in an unknown/unfinished/implementation-dependent state. On the other hand, enabling it is harmless but documenting it would not be elegant anymore. Now it is: Factorize must return true, otherwise the methods will panic. Maybe the solution is to return Lapack's info from Factorize.

In case of SymRankOne, if the updated matrix is not positive definite, it returns without modifying the receiver, so the updated matrix is not available anyway. It can return with a false later on during the update but that would be only due to the effect of floating point and in a general case the probability of getting true in the equal-to-zero check is next to zero (moreover, in the future I'd like to avoid modifying the receiver in this case, too).

kortschak commented 8 years ago

I think we leave it up to the user to engineer an alternative if they want to see that information. It is likely not something that they would use in production, but rather use for debugging.

kortschak commented 8 years ago

LGTM

btracey commented 8 years ago

LGTM