Skip to content

Commit

Permalink
Make generic cholesky throw on non-psd input (#49417)
Browse files Browse the repository at this point in the history
  • Loading branch information
dkarrasch authored Apr 21, 2023
1 parent ecc3751 commit bb83df1
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 17 deletions.
9 changes: 5 additions & 4 deletions stdlib/LinearAlgebra/src/cholesky.jl
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ function _chol!(A::AbstractMatrix, ::Type{UpperTriangular})
A[k,k] = Akk
Akk, info = _chol!(Akk, UpperTriangular)
if info != 0
return UpperTriangular(A), info
return UpperTriangular(A), convert(BlasInt, k)
end
A[k,k] = Akk
AkkInv = inv(copy(Akk'))
Expand All @@ -233,7 +233,7 @@ function _chol!(A::AbstractMatrix, ::Type{LowerTriangular})
A[k,k] = Akk
Akk, info = _chol!(Akk, LowerTriangular)
if info != 0
return LowerTriangular(A), info
return LowerTriangular(A), convert(BlasInt, k)
end
A[k,k] = Akk
AkkInv = inv(Akk)
Expand All @@ -251,11 +251,12 @@ function _chol!(A::AbstractMatrix, ::Type{LowerTriangular})
end

## Numbers
function _chol!(x::Number, uplo)
function _chol!(x::Number, _)
rx = real(x)
iszero(rx) && return (rx, convert(BlasInt, 1))
rxr = sqrt(abs(rx))
rval = convert(promote_type(typeof(x), typeof(rxr)), rxr)
rx == abs(x) ? (rval, convert(BlasInt, 0)) : (rval, convert(BlasInt, 1))
return (rval, convert(BlasInt, rx != abs(x)))
end

## for StridedMatrices, check that matrix is symmetric/Hermitian
Expand Down
29 changes: 16 additions & 13 deletions stdlib/LinearAlgebra/test/cholesky.jl
Original file line number Diff line number Diff line change
Expand Up @@ -260,29 +260,32 @@ end
end
end

@testset "behavior for non-positive definite matrices" for T in (Float64, ComplexF64)
@testset "behavior for non-positive definite matrices" for T in (Float64, ComplexF64, BigFloat)
A = T[1 2; 2 1]
B = T[1 2; 0 1]
C = T[2 0; 0 0]
# check = (true|false)
for M in (A, Hermitian(A), B)
for M in (A, Hermitian(A), B, C)
@test_throws PosDefException cholesky(M)
@test_throws PosDefException cholesky!(copy(M))
@test_throws PosDefException cholesky(M; check = true)
@test_throws PosDefException cholesky!(copy(M); check = true)
@test !LinearAlgebra.issuccess(cholesky(M; check = false))
@test !LinearAlgebra.issuccess(cholesky!(copy(M); check = false))
end
for M in (A, Hermitian(A), B)
@test_throws RankDeficientException cholesky(M, RowMaximum())
@test_throws RankDeficientException cholesky!(copy(M), RowMaximum())
@test_throws RankDeficientException cholesky(M, RowMaximum(); check = true)
@test_throws RankDeficientException cholesky!(copy(M), RowMaximum(); check = true)
@test !LinearAlgebra.issuccess(cholesky(M, RowMaximum(); check = false))
@test !LinearAlgebra.issuccess(cholesky!(copy(M), RowMaximum(); check = false))
C = cholesky(M, RowMaximum(); check = false)
@test_throws RankDeficientException chkfullrank(C)
C = cholesky!(copy(M), RowMaximum(); check = false)
@test_throws RankDeficientException chkfullrank(C)
if T !== BigFloat # generic pivoted cholesky is not implemented
for M in (A, Hermitian(A), B)
@test_throws RankDeficientException cholesky(M, RowMaximum())
@test_throws RankDeficientException cholesky!(copy(M), RowMaximum())
@test_throws RankDeficientException cholesky(M, RowMaximum(); check = true)
@test_throws RankDeficientException cholesky!(copy(M), RowMaximum(); check = true)
@test !LinearAlgebra.issuccess(cholesky(M, RowMaximum(); check = false))
@test !LinearAlgebra.issuccess(cholesky!(copy(M), RowMaximum(); check = false))
C = cholesky(M, RowMaximum(); check = false)
@test_throws RankDeficientException chkfullrank(C)
C = cholesky!(copy(M), RowMaximum(); check = false)
@test_throws RankDeficientException chkfullrank(C)
end
end
@test !isposdef(A)
str = sprint((io, x) -> show(io, "text/plain", x), cholesky(A; check = false))
Expand Down

2 comments on commit bb83df1

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Executing the daily package evaluation, I will reply here when finished:

@nanosoldier runtests(isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your package evaluation job has completed - possible new issues were detected.
A full report can be found here.

Please sign in to comment.