From 4aed581e5ca7ab2f749cc0b03b138bd3ef6366f4 Mon Sep 17 00:00:00 2001 From: Sacha Verweij Date: Wed, 16 May 2018 20:06:24 -0700 Subject: [PATCH] Deprecate lq(...) in favor of lqfact(...) and factorization destructuring. --- NEWS.md | 9 +++++ stdlib/LinearAlgebra/docs/src/index.md | 1 - stdlib/LinearAlgebra/src/LinearAlgebra.jl | 1 - stdlib/LinearAlgebra/src/deprecated.jl | 16 +++++++++ stdlib/LinearAlgebra/src/lq.jl | 40 ++++++----------------- stdlib/LinearAlgebra/test/lq.jl | 39 +--------------------- 6 files changed, 36 insertions(+), 70 deletions(-) diff --git a/NEWS.md b/NEWS.md index 8df8f33724273..503b9afd10a3c 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1080,6 +1080,15 @@ Deprecated or removed either destructured into its components (`U, V, Q, D1, D2, R0 = svdfact(x, y)`) or as a `GeneralizedSVD` object (`gsvdf = svdfact(x, y)`) ([#26997]). + * `lq(A; thin=true)` has been deprecated in favor of `lqfact(A)`. + Whereas `lq(A; thin=true)` returns a tuple of arrays, `lqfact` returns + an `LQ` object. So for a direct replacement of `lqfact(A; thin=true)`, + use `(F = lqfact(A); (F.L, Array(F.Q)))`, and for `lqfact(A; thin=false)` + use `(F = lqfact(A); k = size(F.Q.factors, 2); (F.L, lmul!(F.Q, Matrix{eltype(F.Q)}(I, k, k))))`. + But going forward, consider using the direct result of `lqfact(A)` instead, + either destructured into its components (`L, Q = lqfact(A)`) + or as an `LQ` object (`lqf = lqfact(A)`) ([#26997]). + * The timing functions `tic`, `toc`, and `toq` are deprecated in favor of `@time` and `@elapsed` ([#17046]). diff --git a/stdlib/LinearAlgebra/docs/src/index.md b/stdlib/LinearAlgebra/docs/src/index.md index 16f5423a188ce..0a8b1eea3e8c3 100644 --- a/stdlib/LinearAlgebra/docs/src/index.md +++ b/stdlib/LinearAlgebra/docs/src/index.md @@ -331,7 +331,6 @@ LinearAlgebra.QRCompactWY LinearAlgebra.QRPivoted LinearAlgebra.lqfact! LinearAlgebra.lqfact -LinearAlgebra.lq LinearAlgebra.bkfact LinearAlgebra.bkfact! LinearAlgebra.eigvals diff --git a/stdlib/LinearAlgebra/src/LinearAlgebra.jl b/stdlib/LinearAlgebra/src/LinearAlgebra.jl index 985103e6c6847..10ebed9742633 100644 --- a/stdlib/LinearAlgebra/src/LinearAlgebra.jl +++ b/stdlib/LinearAlgebra/src/LinearAlgebra.jl @@ -126,7 +126,6 @@ export qr, qrfact!, qrfact, - lq, lqfact!, lqfact, rank, diff --git a/stdlib/LinearAlgebra/src/deprecated.jl b/stdlib/LinearAlgebra/src/deprecated.jl index 540a140c4d9d0..8eadcf5496eb7 100644 --- a/stdlib/LinearAlgebra/src/deprecated.jl +++ b/stdlib/LinearAlgebra/src/deprecated.jl @@ -1418,3 +1418,19 @@ end svd(A::BitMatrix) = _simpledepsvd(float(A)) svd(D::Diagonal{<:Number}) = _simpledepsvd(D) svd(A::AbstractTriangular) = _simpledepsvd(copyto!(similar(parent(A)), A)) + +# deprecate lq(...) in favor of lqfact(...) and factorization destructuring +export lq +function lq(A::Union{Number,AbstractMatrix}; full::Bool = false, thin::Union{Bool,Nothing} = nothing) + depwarn(string("`lq(A; thin=true)` has been deprecated in favor of `lqfact(A)`. ", + "Whereas `lq(A; thin=true)` returns a tuple of arrays, `lqfact` returns ", + "an `LQ` object. So for a direct replacement of `lqfact(A; thin=true)`, ", + "use `(F = lqfact(A); (F.L, Array(F.Q)))`, and for `lqfact(A; thin=false)` ", + "use `(F = lqfact(A); k = size(F.Q.factors, 2); (F.L, lmul!(F.Q, Matrix{eltype(F.Q)}(I, k, k))))`. ", + "But going forward, consider using the direct result of `lqfact(A)` instead, ", + "either destructured into its components (`L, Q = lqfact(A)`) ", + "or as an `LQ` object (`lqf = lqfact(A)`)."), :lq) + F = lqfact(A) + retQ = !full ? Array(F.Q) : (k = size(F.Q.factors, 2); lmul!(F.Q, Matrix{eltype(F.Q)}(I, k, k))) + return F.L, retQ +end diff --git a/stdlib/LinearAlgebra/src/lq.jl b/stdlib/LinearAlgebra/src/lq.jl index 1e875454e07f1..c8fcefa605483 100644 --- a/stdlib/LinearAlgebra/src/lq.jl +++ b/stdlib/LinearAlgebra/src/lq.jl @@ -17,49 +17,29 @@ end LQ(factors::AbstractMatrix{T}, τ::Vector{T}) where {T} = LQ{T,typeof(factors)}(factors, τ) LQPackedQ(factors::AbstractMatrix{T}, τ::Vector{T}) where {T} = LQPackedQ{T,typeof(factors)}(factors, τ) +# iteration for destructuring into factors +Base.start(::LQ) = Val(:L) +Base.next(F::LQ, ::Val{:L}) = (F.L, Val(:Q)) +Base.next(F::LQ, ::Val{:Q}) = (F.Q, Val(:done)) +Base.done(F::LQ, ::Val{:done}) = true +Base.done(F::LQ, ::Any) = false + """ lqfact!(A) -> LQ Compute the LQ factorization of `A`, using the input -matrix as a workspace. See also [`lq`](@ref). +matrix as a workspace. See also [`lqfact`](@ref). """ lqfact!(A::StridedMatrix{<:BlasFloat}) = LQ(LAPACK.gelqf!(A)...) """ lqfact(A) -> LQ -Compute the LQ factorization of `A`. See also [`lq`](@ref). +Perform an LQ factorization of `A` such that `A = L*Q`. The LQ factorization is +the QR factorization of `transpose(A)`. """ lqfact(A::StridedMatrix{<:BlasFloat}) = lqfact!(copy(A)) lqfact(x::Number) = lqfact(fill(x,1,1)) -""" - lq(A; full = false) -> L, Q - -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 `transpose(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" 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}; full::Bool = false, thin::Union{Bool,Nothing} = 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, !full ? Array(Q) : lmul!(Q, Matrix{eltype(Q)}(I, size(Q.factors, 2), size(Q.factors, 2))) -end - copy(A::LQ) = LQ(copy(A.factors), copy(A.τ)) LQ{T}(A::LQ) where {T} = LQ(convert(AbstractMatrix{T}, A.factors), convert(Vector{T}, A.τ)) diff --git a/stdlib/LinearAlgebra/test/lq.jl b/stdlib/LinearAlgebra/test/lq.jl index cfa25a2ba87ab..01a12a445548b 100644 --- a/stdlib/LinearAlgebra/test/lq.jl +++ b/stdlib/LinearAlgebra/test/lq.jl @@ -39,7 +39,7 @@ rectangularQ(Q::LinearAlgebra.LQPackedQ) = convert(Array, Q) α = rand(eltya) aα = fill(α,1,1) @test lqfact(α).L*lqfact(α).Q ≈ lqfact(aα).L*lqfact(aα).Q - @test lq(α)[1]*lq(α)[2] ≈ lqfact(aα).L*lqfact(aα).Q + @test ((αL, αQ) = lqfact(α); αL*αQ ≈ lqfact(aα).L*lqfact(aα).Q) @test abs(lqfact(α).Q[1,1]) ≈ one(eltya) tab = promote_type(eltya,eltyb) @@ -105,43 +105,6 @@ rectangularQ(Q::LinearAlgebra.LQPackedQ) = convert(Array, Q) end 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 ("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 ("full", "reduced"/"thin"), and L and Q - # should have multiplication-compatible shapes. - local m, n = 4, 2 - A = randn(m, n) - 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 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 - 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 rectangular/"thin" factorization, - # so for L and Q to have multiplication-compatible shapes, L must be zero-padded - # to have the shape of A. - 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 local m, n function getqs(F::LinearAlgebra.LQ)