Don't throw in cholfact if matrix is not Hermitian

[fredrikekre]

As discussed here and comments below.

We could perhaps dedicate an info code for non-hermitianness, like -1 or something instead? Then we could throw a more targeted error message (similar to what we had before this PR) or throw a HermitianException, instead of

julia> A = rand(5,5); x = rand(5); cholfact(A)\x
ERROR: Base.LinAlg.PosDefException(1)
Stacktrace:
 [1] A_ldiv_B! at ./linalg/cholesky.jl:412 [inlined]
 [2] \(::Base.LinAlg.Cholesky{Float64,Array{Float64,2}}, ::Array{Float64,1}) at ./linalg/factorization.jl:71

Thoughts?

[KristofferC]

Make an Enum for info codes?

[andreasnoack]

In LAPACK, a positive info code corresponds to the index of the non-positive diagonal entry. However, we don't use that information right now so it is not critical. We could use a negative error code because they are only used in LAPACK when the input is invalid so that should never happen.

[andreasnoack]

Maybe test with !issuccess instead.

[fredrikekre]

Yea, that's better.

[andreasnoack]

Make an Enum for info codes?

Right now the info field encodes where the factorization failed. Would you just have two values? One for non-Hermitian and one for non-positive pivot?

[fredrikekre]

Right, since a negative info code will error before we exit the LAPACK module, we could use a negative code for non-hermitian. And add a show method for PosDefException, and, in case of info = -1 print something like: ERROR: Base.LinAlg.PosDefException(3): matrix is not Hermitian. Thoughts on that? :)

[fredrikekre]

ERROR: PosDefException: matrix is not Hermitian. is maybe not the nicest though. Should we instead have a CholeskyException that print one of

ERROR: CholeskyException: Cholesky factorization failed; matrix is not Hermitian.
ERROR: CholeskyException: Cholesky factorization failed; matrix is not positive definite.

Edit: Cholesky probably not needed, instead:

ERROR: CholeskyException: factorization failed; matrix is not Hermitian.
ERROR: CholeskyException: factorization failed; matrix is not positive definite.

[andreasnoack]

I think PosDef is fine. We use the definition that requires the matrix to be Hermitian consistently. Some users have been taught the other definition that allows for non-Hermitian matrices but I think the error message is sufficiently clear about the reason for the failure.

[fredrikekre]

So, as the PR currently? Or should I drop the second commit?

[andreasnoack]

No. I think it is fine as it is.