Skip to content

Commit

Permalink
Deprecate cholfact! to cholesky!.
Browse files Browse the repository at this point in the history
  • Loading branch information
Sacha0 committed May 23, 2018
1 parent d76d364 commit 6927882
Show file tree
Hide file tree
Showing 10 changed files with 60 additions and 39 deletions.
2 changes: 1 addition & 1 deletion stdlib/LinearAlgebra/docs/src/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ LinearAlgebra.lu
LinearAlgebra.lu!
LinearAlgebra.chol
LinearAlgebra.cholesky
LinearAlgebra.cholfact!
LinearAlgebra.cholesky!
LinearAlgebra.lowrankupdate
LinearAlgebra.lowrankdowndate
LinearAlgebra.lowrankupdate!
Expand Down
2 changes: 1 addition & 1 deletion stdlib/LinearAlgebra/src/LinearAlgebra.jl
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ export
bunchkaufman!,
chol,
cholesky,
cholfact!,
cholesky!,
cond,
condskeel,
copyto!,
Expand Down
34 changes: 17 additions & 17 deletions stdlib/LinearAlgebra/src/cholesky.jl
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,14 @@
# Cholesky Factorization #
##########################

# The dispatch structure in the chol!, chol, cholesky, and cholfact! methods is a bit
# The dispatch structure in the chol!, chol, cholesky, and cholesky! methods is a bit
# complicated and some explanation is therefore provided in the following
#
# In the methods below, LAPACK is called when possible, i.e. StridedMatrices with Float32,
# Float64, Complex{Float32}, and Complex{Float64} element types. For other element or
# matrix types, the unblocked Julia implementation in _chol! is used. For cholesky
# and cholfact! pivoting is supported through a Val(Bool) argument. A type argument is
# necessary for type stability since the output of cholesky and cholfact! is either
# and cholesky! pivoting is supported through a Val(Bool) argument. A type argument is
# necessary for type stability since the output of cholesky and cholesky! is either
# Cholesky or PivotedCholesky. The latter is only
# supported for the four LAPACK element types. For other types, e.g. BigFloats Val(true) will
# give an error. It is required that the input is Hermitian (including real symmetric) either
Expand All @@ -21,7 +21,7 @@
# The internal structure is as follows
# - _chol! returns the factor and info without checking positive definiteness
# - chol/chol! returns the factor and checks for positive definiteness
# - cholesky/cholfact! returns Cholesky without checking positive definiteness
# - cholesky/cholesky! returns Cholesky without checking positive definiteness

# FixMe? The dispatch below seems overly complicated. One simplification could be to
# merge the two Cholesky types into one. It would remove the need for Val completely but
Expand Down Expand Up @@ -201,10 +201,10 @@ chol(x::Number, args...) = ((C, info) = _chol!(x, nothing); @assertposdef C info



# cholfact!. Destructive methods for computing Cholesky factorization of real symmetric
# cholesky!. Destructive methods for computing Cholesky factorization of real symmetric
# or Hermitian matrix
## No pivoting (default)
function cholfact!(A::RealHermSymComplexHerm, ::Val{false}=Val(false))
function cholesky!(A::RealHermSymComplexHerm, ::Val{false}=Val(false))
if A.uplo == 'U'
CU, info = _chol!(A.data, UpperTriangular)
Cholesky(CU.data, 'U', info)
Expand All @@ -216,7 +216,7 @@ end

### for StridedMatrices, check that matrix is symmetric/Hermitian
"""
cholfact!(A, Val(false)) -> Cholesky
cholesky!(A, Val(false)) -> Cholesky
The same as [`cholesky`](@ref), but saves space by overwriting the input `A`,
instead of creating a copy. An [`InexactError`](@ref) exception is thrown if
Expand All @@ -230,51 +230,51 @@ julia> A = [1 2; 2 50]
1 2
2 50
julia> cholfact!(A)
julia> cholesky!(A)
ERROR: InexactError: Int64(Int64, 6.782329983125268)
Stacktrace:
[...]
```
"""
function cholfact!(A::StridedMatrix, ::Val{false}=Val(false))
function cholesky!(A::StridedMatrix, ::Val{false}=Val(false))
checksquare(A)
if !ishermitian(A) # return with info = -1 if not Hermitian
return Cholesky(A, 'U', convert(BlasInt, -1))
else
return cholfact!(Hermitian(A), Val(false))
return cholesky!(Hermitian(A), Val(false))
end
end


## With pivoting
### BLAS/LAPACK element types
function cholfact!(A::RealHermSymComplexHerm{<:BlasReal,<:StridedMatrix},
function cholesky!(A::RealHermSymComplexHerm{<:BlasReal,<:StridedMatrix},
::Val{true}; tol = 0.0)
AA, piv, rank, info = LAPACK.pstrf!(A.uplo, A.data, tol)
return CholeskyPivoted{eltype(AA),typeof(AA)}(AA, A.uplo, piv, rank, tol, info)
end

### Non BLAS/LAPACK element types (generic). Since generic fallback for pivoted Cholesky
### is not implemented yet we throw an error
cholfact!(A::RealHermSymComplexHerm{<:Real}, ::Val{true}; tol = 0.0) =
cholesky!(A::RealHermSymComplexHerm{<:Real}, ::Val{true}; tol = 0.0) =
throw(ArgumentError("generic pivoted Cholesky factorization is not implemented yet"))

### for StridedMatrices, check that matrix is symmetric/Hermitian
"""
cholfact!(A, Val(true); tol = 0.0) -> CholeskyPivoted
cholesky!(A, Val(true); tol = 0.0) -> CholeskyPivoted
The same as [`cholesky`](@ref), but saves space by overwriting the input `A`,
instead of creating a copy. An [`InexactError`](@ref) exception is thrown if the
factorization produces a number not representable by the element type of `A`,
e.g. for integer types.
"""
function cholfact!(A::StridedMatrix, ::Val{true}; tol = 0.0)
function cholesky!(A::StridedMatrix, ::Val{true}; tol = 0.0)
checksquare(A)
if !ishermitian(A) # return with info = -1 if not Hermitian
return CholeskyPivoted(A, 'U', Vector{BlasInt}(),convert(BlasInt, 1),
tol, convert(BlasInt, -1))
else
return cholfact!(Hermitian(A), Val(true); tol = tol)
return cholesky!(Hermitian(A), Val(true); tol = tol)
end
end

Expand Down Expand Up @@ -324,7 +324,7 @@ true
```
"""
cholesky(A::Union{StridedMatrix,RealHermSymComplexHerm{<:Real,<:StridedMatrix}},
::Val{false}=Val(false)) = cholfact!(cholcopy(A))
::Val{false}=Val(false)) = cholesky!(cholcopy(A))


## With pivoting
Expand All @@ -341,7 +341,7 @@ The argument `tol` determines the tolerance for determining the rank.
For negative values, the tolerance is the machine precision.
"""
cholesky(A::Union{StridedMatrix,RealHermSymComplexHerm{<:Real,<:StridedMatrix}},
::Val{true}; tol = 0.0) = cholfact!(cholcopy(A), Val(true); tol = tol)
::Val{true}; tol = 0.0) = cholesky!(cholcopy(A), Val(true); tol = tol)

## Number
function cholesky(x::Number, uplo::Symbol=:U)
Expand Down
2 changes: 1 addition & 1 deletion stdlib/LinearAlgebra/src/dense.jl
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ julia> A
2.0 6.78233
```
"""
isposdef!(A::AbstractMatrix) = ishermitian(A) && isposdef(cholfact!(Hermitian(A)))
isposdef!(A::AbstractMatrix) = ishermitian(A) && isposdef(cholesky!(Hermitian(A)))

"""
isposdef(A) -> Bool
Expand Down
10 changes: 9 additions & 1 deletion stdlib/LinearAlgebra/src/deprecated.jl
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ using Base: @deprecate, depwarn
@deprecate cond(F::LinearAlgebra.LU, p::Integer) cond(convert(AbstractArray, F), p)

# PR #22188
export cholfact
export cholfact, cholfact!
@deprecate cholfact!(A::StridedMatrix, uplo::Symbol, ::Type{Val{false}}) cholfact!(Hermitian(A, uplo), Val(false))
@deprecate cholfact!(A::StridedMatrix, uplo::Symbol) cholfact!(Hermitian(A, uplo))
@deprecate cholfact(A::StridedMatrix, uplo::Symbol, ::Type{Val{false}}) cholfact(Hermitian(A, uplo), Val(false))
Expand Down Expand Up @@ -1402,3 +1402,11 @@ export eigfact!
@deprecate(cholfact(A::Union{StridedMatrix,RealHermSymComplexHerm{<:Real,<:StridedMatrix}}, ::Val{false}=Val(false)), cholesky(A, Val(false)))
@deprecate(cholfact(A::Union{StridedMatrix,RealHermSymComplexHerm{<:Real,<:StridedMatrix}}, ::Val{true}; tol = 0.0), cholesky(A, Val(true); tol=tol))
@deprecate(cholfact(x::Number, uplo::Symbol=:U), cholesky(x, uplo))

# deprecate cholfact! to cholesky!
# cholfact! exported from deprecation above
@deprecate(cholfact!(A::RealHermSymComplexHerm, ::Val{false}=Val(false)), cholesky!(A, Val(false)))
@deprecate(cholfact!(A::StridedMatrix, ::Val{false}=Val(false)), cholesky!(A, Val(false)))
@deprecate(cholfact!(A::RealHermSymComplexHerm{<:BlasReal,<:StridedMatrix}, ::Val{true}; tol = 0.0), cholesky!(A, Val(true); tol=tol))
@deprecate(cholfact!(A::RealHermSymComplexHerm{<:Real}, ::Val{true}; tol = 0.0), cholesky!(A, Val(true); tol=tol))
@deprecate(cholfact!(A::StridedMatrix, ::Val{true}; tol = 0.0), cholesky!(A, Val(true); tol=tol))
16 changes: 8 additions & 8 deletions stdlib/LinearAlgebra/test/cholesky.jl
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ end
@test_throws DimensionMismatch chol(A)
@test_throws DimensionMismatch LinearAlgebra.chol!(A)
@test_throws DimensionMismatch cholesky(A)
@test_throws DimensionMismatch cholfact!(A)
@test_throws DimensionMismatch cholesky!(A)
end

#Test error bound on reconstruction of matrix: LAWNS 14, Lemma 2.1
Expand Down Expand Up @@ -100,17 +100,17 @@ end
capds = cholesky(apds)
unary_ops_tests(apds, capds, ε*κ*n)
if eltya <: BlasReal
capds = cholfact!(copy(apds))
capds = cholesky!(copy(apds))
unary_ops_tests(apds, capds, ε*κ*n)
end
ulstring = sprint((t, s) -> show(t, "text/plain", s), capds.UL)
@test sprint((t, s) -> show(t, "text/plain", s), capds) == "$(typeof(capds))\nU factor:\n$ulstring"
else
capdh = cholesky(apdh)
unary_ops_tests(apdh, capdh, ε*κ*n)
capdh = cholfact!(copy(apdh))
capdh = cholesky!(copy(apdh))
unary_ops_tests(apdh, capdh, ε*κ*n)
capdh = cholfact!(copy(apd))
capdh = cholesky!(copy(apd))
unary_ops_tests(apd, capdh, ε*κ*n)
ulstring = sprint((t, s) -> show(t, "text/plain", s), capdh.UL)
@test sprint((t, s) -> show(t, "text/plain", s), capdh) == "$(typeof(capdh))\nU factor:\n$ulstring"
Expand Down Expand Up @@ -186,8 +186,8 @@ end
@test_throws PosDefException logdet(C)
end

# Test generic cholfact!
@testset "generic cholfact!" begin
# Test generic cholesky!
@testset "generic cholesky!" begin
if eltya <: Complex
A = complex.(randn(5,5), randn(5,5))
else
Expand Down Expand Up @@ -264,14 +264,14 @@ end
C = complex.(R, R)
for A in (R, C)
@test !LinearAlgebra.issuccess(cholesky(A))
@test !LinearAlgebra.issuccess(cholfact!(copy(A)))
@test !LinearAlgebra.issuccess(cholesky!(copy(A)))
@test_throws PosDefException chol(A)
@test_throws PosDefException LinearAlgebra.chol!(copy(A))
end
end

@testset "fail for non-BLAS element types" begin
@test_throws ArgumentError cholfact!(Hermitian(rand(Float16, 5,5)), Val(true))
@test_throws ArgumentError cholesky!(Hermitian(rand(Float16, 5,5)), Val(true))
end

end # module TestCholesky
2 changes: 1 addition & 1 deletion stdlib/LinearAlgebra/test/dense.jl
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ bimg = randn(n,2)/2
@testset "Positive definiteness" begin
@test !isposdef(ainit)
@test isposdef(apd)
if eltya != Int # cannot perform cholfact! for Matrix{Int}
if eltya != Int # cannot perform cholesky! for Matrix{Int}
@test !isposdef!(copy(ainit))
@test isposdef!(copy(apd))
end
Expand Down
12 changes: 6 additions & 6 deletions stdlib/SuiteSparse/src/cholmod.jl
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import Base: (*), convert, copy, eltype, getindex, getproperty, show, size,

using LinearAlgebra
import LinearAlgebra: (\),
cholesky, cholfact!, det, diag, ishermitian, isposdef,
cholesky, cholesky!, det, diag, ishermitian, isposdef,
issuccess, issymmetric, ldlt, ldlt!, logdet

using SparseArrays
Expand Down Expand Up @@ -1367,7 +1367,7 @@ function fact_(A::Sparse{<:VTypes}, cm::Array{UInt8};
return F
end

function cholfact!(F::Factor{Tv}, A::Sparse{Tv}; shift::Real=0.0) where Tv
function cholesky!(F::Factor{Tv}, A::Sparse{Tv}; shift::Real=0.0) where Tv
# Makes it an LLt
unsafe_store!(common_final_ll[], 1)

Expand All @@ -1378,7 +1378,7 @@ function cholfact!(F::Factor{Tv}, A::Sparse{Tv}; shift::Real=0.0) where Tv
end

"""
cholfact!(F::Factor, A; shift = 0.0) -> CHOLMOD.Factor
cholesky!(F::Factor, A; shift = 0.0) -> CHOLMOD.Factor
Compute the Cholesky (``LL'``) factorization of `A`, reusing the symbolic
factorization `F`. `A` must be a [`SparseMatrixCSC`](@ref) or a [`Symmetric`](@ref)/
Expand All @@ -1393,13 +1393,13 @@ See also [`cholesky`](@ref).
be converted to `SparseMatrixCSC{Float64}` or `SparseMatrixCSC{ComplexF64}`
as appropriate.
"""
cholfact!(F::Factor, A::Union{SparseMatrixCSC{T},
cholesky!(F::Factor, A::Union{SparseMatrixCSC{T},
SparseMatrixCSC{Complex{T}},
Symmetric{T,SparseMatrixCSC{T,SuiteSparse_long}},
Hermitian{Complex{T},SparseMatrixCSC{Complex{T},SuiteSparse_long}},
Hermitian{T,SparseMatrixCSC{T,SuiteSparse_long}}};
shift = 0.0) where {T<:Real} =
cholfact!(F, Sparse(A); shift = shift)
cholesky!(F, Sparse(A); shift = shift)

function cholesky(A::Sparse; shift::Real=0.0,
perm::AbstractVector{SuiteSparse_long}=SuiteSparse_long[])
Expand All @@ -1411,7 +1411,7 @@ function cholesky(A::Sparse; shift::Real=0.0,
F = fact_(A, cm; perm = perm)

# Compute the numerical factorization
cholfact!(F, A; shift = shift)
cholesky!(F, A; shift = shift)

return F
end
Expand Down
13 changes: 13 additions & 0 deletions stdlib/SuiteSparse/src/deprecated.jl
Original file line number Diff line number Diff line change
Expand Up @@ -99,3 +99,16 @@ end
kws...) where {T<:Real},
cholesky(A; kws...))
end

# deprecate cholfact! to cholesky!
@eval SuiteSparse.CHOLMOD begin
import LinearAlgebra: cholfact!
@deprecate(cholfact!(F::Factor{Tv}, A::Sparse{Tv}; shift::Real=0.0) where Tv, cholesky!(F, A; shift=shift))
@deprecate(cholfact!(F::Factor, A::Union{SparseMatrixCSC{T},
SparseMatrixCSC{Complex{T}},
Symmetric{T,SparseMatrixCSC{T,SuiteSparse_long}},
Hermitian{Complex{T},SparseMatrixCSC{Complex{T},SuiteSparse_long}},
Hermitian{T,SparseMatrixCSC{T,SuiteSparse_long}}};
shift = 0.0) where {T<:Real},
cholesyk!(F, A; shift=shift))
end
6 changes: 3 additions & 3 deletions stdlib/SuiteSparse/test/cholmod.jl
Original file line number Diff line number Diff line change
Expand Up @@ -434,19 +434,19 @@ end
@test F1 == F2
end

### cholfact!/ldlt!
### cholesky!/ldlt!
F = cholesky(A1pd)
CHOLMOD.change_factor!(elty, false, false, true, true, F)
@test unsafe_load(pointer(F)).is_ll == 0
CHOLMOD.change_factor!(elty, true, false, true, true, F)
@test CHOLMOD.Sparse(cholfact!(copy(F), A1pd)) CHOLMOD.Sparse(F) # surprisingly, this can cause small ulp size changes so we cannot test exact equality
@test CHOLMOD.Sparse(cholesky!(copy(F), A1pd)) CHOLMOD.Sparse(F) # surprisingly, this can cause small ulp size changes so we cannot test exact equality
@test size(F, 2) == 5
@test size(F, 3) == 1
@test_throws ArgumentError size(F, 0)

F = cholesky(A1pdSparse, shift=2)
@test isa(CHOLMOD.Sparse(F), CHOLMOD.Sparse{elty})
@test CHOLMOD.Sparse(cholfact!(copy(F), A1pd, shift=2.0)) CHOLMOD.Sparse(F) # surprisingly, this can cause small ulp size changes so we cannot test exact equality
@test CHOLMOD.Sparse(cholesky!(copy(F), A1pd, shift=2.0)) CHOLMOD.Sparse(F) # surprisingly, this can cause small ulp size changes so we cannot test exact equality

F = ldlt(A1pd)
@test isa(CHOLMOD.Sparse(F), CHOLMOD.Sparse{elty})
Expand Down

0 comments on commit 6927882

Please sign in to comment.