Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate getindex(::Factorization, ::Symbol) in favor of dot overloading #25184

Merged
merged 3 commits into from
Dec 22, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -879,6 +879,10 @@ Deprecated or removed

* `sub2ind` and `ind2sub` are deprecated in favor of using `CartesianIndices` and `LinearIndices` ([#24715]).

* `getindex(F::Factorizion, s::Symbol)` (usually seen as e.g. `F[:Q]`) is deprecated
in favor of dot overloading (`getproperty`) so factors should now be accessed as e.g.
`F.Q` instead of `F[:Q]` ([#25184]).

Command-line option changes
---------------------------

Expand Down
9 changes: 9 additions & 0 deletions base/deprecated.jl
Original file line number Diff line number Diff line change
Expand Up @@ -3466,6 +3466,15 @@ workspace() = error("workspace() is discontinued, check out Revise.jl for an alt
@deprecate Ref(x::Ptr) Ref(x, 1)
@deprecate Ref(x::Ref) x # or perhaps, `convert(Ref, x)`

# PR #25184. Use getproperty instead of getindex for Factorizations
function getindex(F::Factorization, s::Symbol)
depwarn("`F[:$s]` is deprecated, use `F.$s` instead.", :getindex)
return getproperty(F, s)
end
@eval Base.LinAlg begin
@deprecate getq(F::Factorization) F.Q
end

# END 0.7 deprecations

# BEGIN 1.0 deprecations
Expand Down
38 changes: 19 additions & 19 deletions base/linalg/bunchkaufman.jl
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,8 @@ convert(::Type{BunchKaufman{T}}, B::BunchKaufman) where {T} =
convert(::Type{Factorization{T}}, B::BunchKaufman{T}) where {T} = B
convert(::Type{Factorization{T}}, B::BunchKaufman) where {T} = convert(BunchKaufman{T}, B)

size(B::BunchKaufman) = size(B.LD)
size(B::BunchKaufman, d::Integer) = size(B.LD, d)
size(B::BunchKaufman) = size(getfield(B, :LD))
size(B::BunchKaufman, d::Integer) = size(getfield(B, :LD), d)
Copy link
Member

@Sacha0 Sacha0 Dec 20, 2017

Choose a reason for hiding this comment

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

Why switch to explicitly calling getfield here?

Copy link
Member Author

Choose a reason for hiding this comment

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

To avoid an infinite cycle between size and getproperty. The reason is that getproperty always calls size.

Copy link
Member

Choose a reason for hiding this comment

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

Cheers, and thanks for the explanation! :)

issymmetric(B::BunchKaufman) = B.symmetric
ishermitian(B::BunchKaufman) = !B.symmetric

Expand Down Expand Up @@ -115,7 +115,7 @@ function _ipiv2perm_bk(v::AbstractVector{T}, maxi::Integer, uplo::Char) where T
end

"""
getindex(B::BunchKaufman, d::Symbol)
getproperty(B::BunchKaufman, d::Symbol)

Extract the factors of the Bunch-Kaufman factorization `B`. The factorization can take the
two forms `L*D*L'` or `U*D*U'` (or `L*D*Transpose(L)` in the complex symmetric case) where `L` is a
Expand Down Expand Up @@ -153,43 +153,43 @@ permutation:
3
2

julia> F[:L]*F[:D]*F[:L]' - A[F[:p], F[:p]]
julia> F.L*F.D*F.L' - A[F.p, F.p]
3×3 Array{Float64,2}:
0.0 0.0 0.0
0.0 0.0 0.0
0.0 0.0 0.0

julia> F = bkfact(Symmetric(A));

julia> F[:U]*F[:D]*F[:U]' - F[:P]*A*F[:P]'
julia> F.U*F.D*F.U' - F.P*A*F.P'
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

oh boy, that is soooo much prettier

3×3 Array{Float64,2}:
0.0 0.0 0.0
0.0 0.0 0.0
0.0 0.0 0.0
```
"""
function getindex(B::BunchKaufman{T}, d::Symbol) where {T<:BlasFloat}
function getproperty(B::BunchKaufman{T}, d::Symbol) where {T<:BlasFloat}
n = size(B, 1)
if d == :p
return _ipiv2perm_bk(B.ipiv, n, B.uplo)
return _ipiv2perm_bk(getfield(B, :ipiv), n, getfield(B, :uplo))
elseif d == :P
return Matrix{T}(I, n, n)[:,invperm(B[:p])]
return Matrix{T}(I, n, n)[:,invperm(B.p)]
elseif d == :L || d == :U || d == :D
if B.rook
LUD, od = LAPACK.syconvf_rook!(B.uplo, 'C', copy(B.LD), B.ipiv)
if getfield(B, :rook)
LUD, od = LAPACK.syconvf_rook!(getfield(B, :uplo), 'C', copy(getfield(B, :LD)), getfield(B, :ipiv))
else
LUD, od = LAPACK.syconv!(B.uplo, copy(B.LD), B.ipiv)
LUD, od = LAPACK.syconv!(getfield(B, :uplo), copy(getfield(B, :LD)), getfield(B, :ipiv))
end
if d == :D
if B.uplo == 'L'
if getfield(B, :uplo) == 'L'
odl = od[1:n - 1]
return Tridiagonal(odl, diag(LUD), B.symmetric ? odl : conj.(odl))
return Tridiagonal(odl, diag(LUD), getfield(B, :symmetric) ? odl : conj.(odl))
else # 'U'
odu = od[2:n]
return Tridiagonal(B.symmetric ? odu : conj.(odu), diag(LUD), odu)
return Tridiagonal(getfield(B, :symmetric) ? odu : conj.(odu), diag(LUD), odu)
end
elseif d == :L
if B.uplo == 'L'
if getfield(B, :uplo) == 'L'
return UnitLowerTriangular(LUD)
else
throw(ArgumentError("factorization is U*D*Transpose(U) but you requested L"))
Expand All @@ -202,7 +202,7 @@ function getindex(B::BunchKaufman{T}, d::Symbol) where {T<:BlasFloat}
end
end
else
throw(KeyError(d))
getfield(B, d)
end
end

Expand All @@ -212,11 +212,11 @@ function Base.show(io::IO, mime::MIME{Symbol("text/plain")}, B::BunchKaufman)
if issuccess(B)
println(io, summary(B))
println(io, "D factor:")
show(io, mime, B[:D])
show(io, mime, B.D)
println(io, "\n$(B.uplo) factor:")
show(io, mime, B[Symbol(B.uplo)])
show(io, mime, B.uplo == 'L' ? B.L : B.U)
println(io, "\npermutation:")
show(io, mime, B[:p])
show(io, mime, B.p)
else
print(io, "Failed factorization of type $(typeof(B))")
end
Expand Down
77 changes: 45 additions & 32 deletions base/linalg/cholesky.jl
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ end
Compute the Cholesky factorization of a dense symmetric positive definite matrix `A`
and return a `Cholesky` factorization. The matrix `A` can either be a [`Symmetric`](@ref) or [`Hermitian`](@ref)
`StridedMatrix` or a *perfectly* symmetric or Hermitian `StridedMatrix`.
The triangular Cholesky factor can be obtained from the factorization `F` with: `F[:L]` and `F[:U]`.
The triangular Cholesky factor can be obtained from the factorization `F` with: `F.L` and `F.U`.
The following functions are available for `Cholesky` objects: [`size`](@ref), [`\\`](@ref),
[`inv`](@ref), [`det`](@ref), [`logdet`](@ref) and [`isposdef`](@ref).

Expand All @@ -305,19 +305,19 @@ U factor:
⋅ 1.0 5.0
⋅ ⋅ 3.0

julia> C[:U]
julia> C.U
3×3 UpperTriangular{Float64,Array{Float64,2}}:
2.0 6.0 -8.0
⋅ 1.0 5.0
⋅ ⋅ 3.0

julia> C[:L]
julia> C.L
3×3 LowerTriangular{Float64,Array{Float64,2}}:
2.0 ⋅ ⋅
6.0 1.0 ⋅
-8.0 5.0 3.0

julia> C[:L] * C[:U] == A
julia> C.L * C.U == A
true
```
"""
Expand All @@ -332,7 +332,7 @@ cholfact(A::Union{StridedMatrix,RealHermSymComplexHerm{<:Real,<:StridedMatrix}},
Compute the pivoted Cholesky factorization of a dense symmetric positive semi-definite matrix `A`
and return a `CholeskyPivoted` factorization. The matrix `A` can either be a [`Symmetric`](@ref)
or [`Hermitian`](@ref) `StridedMatrix` or a *perfectly* symmetric or Hermitian `StridedMatrix`.
The triangular Cholesky factor can be obtained from the factorization `F` with: `F[:L]` and `F[:U]`.
The triangular Cholesky factor can be obtained from the factorization `F` with: `F.L` and `F.U`.
The following functions are available for `PivotedCholesky` objects:
[`size`](@ref), [`\\`](@ref), [`inv`](@ref), [`det`](@ref), and [`rank`](@ref).
The argument `tol` determines the tolerance for determining the rank.
Expand Down Expand Up @@ -361,14 +361,14 @@ CholeskyPivoted{T}(C::CholeskyPivoted) where {T} =
Factorization{T}(C::CholeskyPivoted{T}) where {T} = C
Factorization{T}(C::CholeskyPivoted) where {T} = CholeskyPivoted{T}(C)

AbstractMatrix(C::Cholesky) = C.uplo == 'U' ? C[:U]'C[:U] : C[:L]*C[:L]'
AbstractMatrix(C::Cholesky) = C.uplo == 'U' ? C.U'C.U : C.L*C.L'
AbstractArray(C::Cholesky) = AbstractMatrix(C)
Matrix(C::Cholesky) = Array(AbstractArray(C))
Array(C::Cholesky) = Matrix(C)

function AbstractMatrix(F::CholeskyPivoted)
ip = invperm(F[:p])
(F[:L] * F[:U])[ip,ip]
ip = invperm(F.p)
(F.L * F.U)[ip,ip]
end
AbstractArray(F::CholeskyPivoted) = AbstractMatrix(F)
Matrix(F::CholeskyPivoted) = Array(AbstractArray(F))
Expand All @@ -380,43 +380,56 @@ copy(C::CholeskyPivoted) = CholeskyPivoted(copy(C.factors), C.uplo, C.piv, C.ran
size(C::Union{Cholesky, CholeskyPivoted}) = size(C.factors)
size(C::Union{Cholesky, CholeskyPivoted}, d::Integer) = size(C.factors, d)

function getindex(C::Cholesky, d::Symbol)
d == :U && return UpperTriangular(Symbol(C.uplo) == d ? C.factors : adjoint(C.factors))
d == :L && return LowerTriangular(Symbol(C.uplo) == d ? C.factors : adjoint(C.factors))
d == :UL && return Symbol(C.uplo) == :U ? UpperTriangular(C.factors) : LowerTriangular(C.factors)
throw(KeyError(d))
end
function getindex(C::CholeskyPivoted{T}, d::Symbol) where T<:BlasFloat
d == :U && return UpperTriangular(Symbol(C.uplo) == d ? C.factors : adjoint(C.factors))
d == :L && return LowerTriangular(Symbol(C.uplo) == d ? C.factors : adjoint(C.factors))
d == :p && return C.piv
if d == :P
function getproperty(C::Cholesky, d::Symbol)
Cfactors = getfield(C, :factors)
Cuplo = getfield(C, :uplo)
if d == :U
return UpperTriangular(Symbol(Cuplo) == d ? Cfactors : adjoint(Cfactors))
elseif d == :L
return LowerTriangular(Symbol(Cuplo) == d ? Cfactors : adjoint(Cfactors))
elseif d == :UL
return Symbol(Cuplo) == :U ? UpperTriangular(Cfactors) : LowerTriangular(Cfactors)
else
return getfield(C, d)
end
end
function getproperty(C::CholeskyPivoted{T}, d::Symbol) where T<:BlasFloat
Cfactors = getfield(C, :factors)
Cuplo = getfield(C, :uplo)
if d == :U
return UpperTriangular(Symbol(Cuplo) == d ? Cfactors : adjoint(Cfactors))
elseif d == :L
return LowerTriangular(Symbol(Cuplo) == d ? Cfactors : adjoint(Cfactors))
elseif d == :p
return getfield(C, :piv)
elseif d == :P
n = size(C, 1)
P = zeros(T, n, n)
for i = 1:n
P[C.piv[i],i] = one(T)
P[getfield(C, :piv)[i], i] = one(T)
end
return P
else
return getfield(C, d)
end
throw(KeyError(d))
end

issuccess(C::Cholesky) = C.info == 0

function show(io::IO, mime::MIME{Symbol("text/plain")}, C::Cholesky{<:Any,<:AbstractMatrix})
if issuccess(C)
println(io, summary(C), "\n$(C.uplo) factor:")
show(io, mime, C[:UL])
show(io, mime, C.UL)
else
print(io, "Failed factorization of type $(typeof(C))")
end
end

function show(io::IO, mime::MIME{Symbol("text/plain")}, C::CholeskyPivoted{<:Any,<:AbstractMatrix})
println(io, summary(C), "\n$(C.uplo) factor with rank $(rank(C)):")
show(io, mime, C.uplo == 'U' ? C[:U] : C[:L])
show(io, mime, C.uplo == 'U' ? C.U : C.L)
println(io, "\npermutation:")
show(io, mime, C[:p])
show(io, mime, C.p)
end

ldiv!(C::Cholesky{T,<:AbstractMatrix}, B::StridedVecOrMat{T}) where {T<:BlasFloat} =
Expand Down Expand Up @@ -534,8 +547,8 @@ rank(C::CholeskyPivoted) = C.rank
"""
lowrankupdate!(C::Cholesky, v::StridedVector) -> CC::Cholesky

Update a Cholesky factorization `C` with the vector `v`. If `A = C[:U]'C[:U]` then
`CC = cholfact(C[:U]'C[:U] + v*v')` but the computation of `CC` only uses `O(n^2)`
Update a Cholesky factorization `C` with the vector `v`. If `A = C.U'C.U` then
`CC = cholfact(C.U'C.U + v*v')` but the computation of `CC` only uses `O(n^2)`
operations. The input factorization `C` is updated in place such that on exit `C == CC`.
The vector `v` is destroyed during the computation.
"""
Expand Down Expand Up @@ -580,8 +593,8 @@ end
"""
lowrankdowndate!(C::Cholesky, v::StridedVector) -> CC::Cholesky

Downdate a Cholesky factorization `C` with the vector `v`. If `A = C[:U]'C[:U]` then
`CC = cholfact(C[:U]'C[:U] - v*v')` but the computation of `CC` only uses `O(n^2)`
Downdate a Cholesky factorization `C` with the vector `v`. If `A = C.U'C.U` then
`CC = cholfact(C.U'C.U - v*v')` but the computation of `CC` only uses `O(n^2)`
operations. The input factorization `C` is updated in place such that on exit `C == CC`.
The vector `v` is destroyed during the computation.
"""
Expand Down Expand Up @@ -633,17 +646,17 @@ end
"""
lowrankupdate(C::Cholesky, v::StridedVector) -> CC::Cholesky

Update a Cholesky factorization `C` with the vector `v`. If `A = C[:U]'C[:U]`
then `CC = cholfact(C[:U]'C[:U] + v*v')` but the computation of `CC` only uses
Update a Cholesky factorization `C` with the vector `v`. If `A = C.U'C.U`
then `CC = cholfact(C.U'C.U + v*v')` but the computation of `CC` only uses
`O(n^2)` operations.
"""
lowrankupdate(C::Cholesky, v::StridedVector) = lowrankupdate!(copy(C), copy(v))

"""
lowrankdowndate(C::Cholesky, v::StridedVector) -> CC::Cholesky

Downdate a Cholesky factorization `C` with the vector `v`. If `A = C[:U]'C[:U]`
then `CC = cholfact(C[:U]'C[:U] - v*v')` but the computation of `CC` only uses
Downdate a Cholesky factorization `C` with the vector `v`. If `A = C.U'C.U`
then `CC = cholfact(C.U'C.U - v*v')` but the computation of `CC` only uses
`O(n^2)` operations.
"""
lowrankdowndate(C::Cholesky, v::StridedVector) = lowrankdowndate!(copy(C), copy(v))
8 changes: 4 additions & 4 deletions base/linalg/dense.jl
Original file line number Diff line number Diff line change
Expand Up @@ -709,8 +709,8 @@ function sqrt(A::StridedMatrix{<:Real})
return triu!(parent(sqrt(UpperTriangular(A))))
else
SchurF = schurfact(complex(A))
R = triu!(parent(sqrt(UpperTriangular(SchurF[:T])))) # unwrapping unnecessary?
return SchurF[:vectors] * R * SchurF[:vectors]'
R = triu!(parent(sqrt(UpperTriangular(SchurF.T)))) # unwrapping unnecessary?
return SchurF.vectors * R * SchurF.vectors'
end
end
function sqrt(A::StridedMatrix{<:Complex})
Expand All @@ -723,8 +723,8 @@ function sqrt(A::StridedMatrix{<:Complex})
return triu!(parent(sqrt(UpperTriangular(A))))
else
SchurF = schurfact(A)
R = triu!(parent(sqrt(UpperTriangular(SchurF[:T])))) # unwrapping unnecessary?
return SchurF[:vectors] * R * SchurF[:vectors]'
R = triu!(parent(sqrt(UpperTriangular(SchurF.T)))) # unwrapping unnecessary?
return SchurF.vectors * R * SchurF.vectors'
end
end

Expand Down
Loading