Skip to content

Commit

Permalink
Deprecate keyword argument "thin" in favor of "square" in lq methods.
Browse files Browse the repository at this point in the history
  • Loading branch information
Sacha0 committed Oct 26, 2017
1 parent 3590010 commit 5d7931a
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 38 deletions.
4 changes: 4 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,10 @@ Deprecated or removed
* The `cholfact`/`cholfact!` methods that accepted an `uplo` symbol have been deprecated
in favor of using `Hermitian` (or `Symmetric`) views ([#22187], [#22188]).

* The `thin` keyword argument for orthogonal decomposition methods has
been deprecated in favor of `full`, which has the opposite meaning:
`thin == true` if and only if `full == false` ([#24279]).

* `isposdef(A::AbstractMatrix, UL::Symbol)` and `isposdef!(A::AbstractMatrix, UL::Symbol)`
have been deprecated in favor of `isposdef(Hermitian(A, UL))` and `isposdef!(Hermitian(A, UL))`
respectively ([#22245]).
Expand Down
1 change: 1 addition & 0 deletions base/deprecated.jl
Original file line number Diff line number Diff line change
Expand Up @@ -2034,6 +2034,7 @@ end
# TODO: after 0.7, remove thin keyword argument and associated logic from...
# (1) base/linalg/svd.jl
# (2) base/linalg/qr.jl
# (3) base/linalg/lq.jl

@deprecate find(x::Number) find(!iszero, x)
@deprecate findnext(A, v, i::Integer) findnext(equalto(v), A, i)
Expand Down
29 changes: 18 additions & 11 deletions base/linalg/lq.jl
Original file line number Diff line number Diff line change
Expand Up @@ -33,24 +33,31 @@ lqfact(A::StridedMatrix{<:BlasFloat}) = lqfact!(copy(A))
lqfact(x::Number) = lqfact(fill(x,1,1))

"""
lq(A; [thin=true]) -> L, Q
lq(A; full = false) -> L, Q
Perform an LQ factorization of `A` such that `A = L*Q`. The default is to compute
a "thin" factorization. The LQ factorization is the QR factorization of `A.'`.
`L` is not extendedwith zeros if the explicit, square form of `Q`
is requested via `thin = false`.
Perform an LQ factorization of `A` such that `A = L*Q`. The default (`full = false`)
computes a factorization with possibly-rectangular `L` and `Q`, commonly the "thin"
factorization. The LQ factorization is the QR factorization of `A.'`. If the explicit,
full/square form of `Q` is requested via `full = true`, `L` is not extended with zeros.
!!! note
While in QR factorization the "thin" factorization is so named due to yielding
either a square or "tall"/"thin" factor `Q`, in LQ factorization the "thin"
factorization somewhat confusingly produces either a square or "short"/"wide"
factor `Q`. "Thin" factorizations more broadly are also (more descriptively)
referred to as "truncated" or "reduced" factorizatons.
either a square or "tall"/"thin" rectangular factor `Q`, in LQ factorization the
"thin" factorization somewhat confusingly produces either a square or "short"/"wide"
rectangular factor `Q`. "Thin" factorizations more broadly are also
referred to as "reduced" factorizatons.
"""
function lq(A::Union{Number,AbstractMatrix}; thin::Bool = true)
function lq(A::Union{Number,AbstractMatrix}; full::Bool = false, thin::Union{Bool,Void} = nothing)
# DEPRECATION TODO: remove deprecated thin argument and associated logic after 0.7
if thin != nothing
Base.depwarn(string("the `thin` keyword argument in `lq(A; thin = $(thin))` has ",
"been deprecated in favor of `full`, which has the opposite meaning, ",
"e.g. `lq(A; full = $(!thin))`."), :lq)
full::Bool = !thin
end
F = lqfact(A)
L, Q = F[:L], F[:Q]
return L, thin ? Array(Q) : A_mul_B!(Q, eye(eltype(Q), size(Q.factors, 2)))
return L, !full ? Array(Q) : A_mul_B!(Q, eye(eltype(Q), size(Q.factors, 2)))
end

copy(A::LQ) = LQ(copy(A.factors), copy(A.τ))
Expand Down
54 changes: 27 additions & 27 deletions test/linalg/lq.jl
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ bimg = randn(n,2)/2

# helper functions to unambiguously recover explicit forms of an LQPackedQ
squareQ(Q::LinAlg.LQPackedQ) = A_mul_B!(Q, eye(eltype(Q), size(Q.factors, 2)))
truncatedQ(Q::LinAlg.LQPackedQ) = convert(Array, Q)
rectangularQ(Q::LinAlg.LQPackedQ) = convert(Array, Q)

@testset for eltya in (Float32, Float64, Complex64, Complex128)
a = eltya == Int ? rand(1:7, n, n) : convert(Matrix{eltya}, eltya <: Complex ? complex.(areal, aimg) : areal)
Expand Down Expand Up @@ -98,10 +98,10 @@ truncatedQ(Q::LinAlg.LQPackedQ) = convert(Array, Q)
@testset "Matmul with LQ factorizations" begin
lqa = lqfact(a[:,1:n1])
l,q = lqa[:L], lqa[:Q]
@test truncatedQ(q)*truncatedQ(q)' eye(eltya,n1)
@test rectangularQ(q)*rectangularQ(q)' eye(eltya,n1)
@test squareQ(q)'*squareQ(q) eye(eltya, n1)
@test_throws DimensionMismatch A_mul_B!(eye(eltya,n+1),q)
@test Ac_mul_B!(q, truncatedQ(q)) eye(eltya,n1)
@test Ac_mul_B!(q, rectangularQ(q)) eye(eltya,n1)
@test_throws DimensionMismatch A_mul_Bc!(eye(eltya,n+1),q)
@test_throws BoundsError size(q,-1)
end
Expand All @@ -111,38 +111,38 @@ end
@testset "correct form of Q from lq(...) (#23729)" begin
# where the original matrix (say A) is square or has more rows than columns,
# then A's factorization's triangular factor (say L) should have the same shape
# as A independent of factorization form (truncated, square), and A's factorization's
# as A independent of factorization form ("full", "reduced"/"thin"), and A's factorization's
# orthogonal factor (say Q) should be a square matrix of order of A's number of
# columns independent of factorization form (truncated, square), and L and Q
# columns independent of factorization form ("full", "reduced"/"thin"), and L and Q
# should have multiplication-compatible shapes.
local m, n = 4, 2
A = randn(m, n)
for thin in (true, false)
L, Q = lq(A, thin = thin)
for full in (false, true)
L, Q = lq(A, full = full)
@test size(L) == (m, n)
@test size(Q) == (n, n)
@test isapprox(A, L*Q)
end
# where the original matrix has strictly fewer rows than columns ...
m, n = 2, 4
A = randn(m, n)
# ... then, for a truncated factorization of A, L should be a square matrix
# ... then, for a rectangular/"thin" factorization of A, L should be a square matrix
# of order of A's number of rows, Q should have the same shape as A,
# and L and Q should have multiplication-compatible shapes
Lthin, Qthin = lq(A, thin = true)
@test size(Lthin) == (m, m)
@test size(Qthin) == (m, n)
@test isapprox(A, Lthin * Qthin)
# ... and, for a square/non-truncated factorization of A, L should have the
Lrect, Qrect = lq(A, full = false)
@test size(Lrect) == (m, m)
@test size(Qrect) == (m, n)
@test isapprox(A, Lrect * Qrect)
# ... and, for a full factorization of A, L should have the
# same shape as A, Q should be a square matrix of order of A's number of columns,
# and L and Q should have multiplication-compatible shape. but instead the L returned
# has no zero-padding on the right / is L for the truncated factorization,
# has no zero-padding on the right / is L for the rectangular/"thin" factorization,
# so for L and Q to have multiplication-compatible shapes, L must be zero-padded
# to have the shape of A.
Lsquare, Qsquare = lq(A, thin = false)
@test size(Lsquare) == (m, m)
@test size(Qsquare) == (n, n)
@test isapprox(A, [Lsquare zeros(m, n - m)] * Qsquare)
Lfull, Qfull = lq(A, full = true)
@test size(Lfull) == (m, m)
@test size(Qfull) == (n, n)
@test isapprox(A, [Lfull zeros(m, n - m)] * Qfull)
end

@testset "getindex on LQPackedQ (#23733)" begin
Expand All @@ -153,14 +153,14 @@ end
return implicitQ, explicitQ
end

m, n = 3, 3 # truncated Q 3-by-3, square Q 3-by-3
m, n = 3, 3 # reduced Q 3-by-3, full Q 3-by-3
implicitQ, explicitQ = getqs(lqfact(randn(m, n)))
@test implicitQ[1, 1] == explicitQ[1, 1]
@test implicitQ[m, 1] == explicitQ[m, 1]
@test implicitQ[1, n] == explicitQ[1, n]
@test implicitQ[m, n] == explicitQ[m, n]

m, n = 3, 4 # truncated Q 3-by-4, square Q 4-by-4
m, n = 3, 4 # reduced Q 3-by-4, full Q 4-by-4
implicitQ, explicitQ = getqs(lqfact(randn(m, n)))
@test implicitQ[1, 1] == explicitQ[1, 1]
@test implicitQ[m, 1] == explicitQ[m, 1]
Expand All @@ -169,7 +169,7 @@ end
@test implicitQ[m+1, 1] == explicitQ[m+1, 1]
@test implicitQ[m+1, n] == explicitQ[m+1, n]

m, n = 4, 3 # truncated Q 3-by-3, square Q 3-by-3
m, n = 4, 3 # reduced Q 3-by-3, full Q 3-by-3
implicitQ, explicitQ = getqs(lqfact(randn(m, n)))
@test implicitQ[1, 1] == explicitQ[1, 1]
@test implicitQ[n, 1] == explicitQ[n, 1]
Expand All @@ -178,11 +178,11 @@ end
end

@testset "size on LQPackedQ (#23780)" begin
# size(Q::LQPackedQ) yields the shape of Q's square form
# size(Q::LQPackedQ) yields the shape of Q's full/square form
for ((mA, nA), nQ) in (
((3, 3), 3), # A 3-by-3 => square Q 3-by-3
((3, 4), 4), # A 3-by-4 => square Q 4-by-4
((4, 3), 3) )# A 4-by-3 => square Q 3-by-3
((3, 3), 3), # A 3-by-3 => full/square Q 3-by-3
((3, 4), 4), # A 3-by-4 => full/square Q 4-by-4
((4, 3), 3) )# A 4-by-3 => full/square Q 3-by-3
@test size(lqfact(randn(mA, nA))[:Q]) == (nQ, nQ)
end
end
Expand All @@ -205,12 +205,12 @@ end
@test Ac_mul_Bc(C, implicitQ) Ac_mul_Bc(C, explicitQ)
end
# where the LQ-factored matrix has at least as many rows m as columns n,
# Q's square and truncated forms have the same shape (n-by-n). hence we expect
# Q's full/square and reduced/rectangular forms have the same shape (n-by-n). hence we expect
# _only_ *-by-n (n-by-*) C to work in A_mul_B*(C, Q) (Ac_mul_B*(C, Q)) ops.
# and hence the n-by-n C tests above suffice.
#
# where the LQ-factored matrix has more columns n than rows m,
# Q's square form is n-by-n whereas its truncated form is m-by-n.
# Q's full/square form is n-by-n whereas its reduced/rectangular form is m-by-n.
# hence we need also test *-by-m C with
# A*_mul_B(C, Q) ops, as below via m-by-m C.
mA, nA = 3, 4
Expand Down

0 comments on commit 5d7931a

Please sign in to comment.