parametrize Bidiagonal on wrapped vector type (#22925)

deprecate Bidiagonal constructor that automatically convert the vectors
JuliaLang · Aug 3, 2017 · 8832208 · 8832208
1 parent 607806e
commit 8832208
Show file tree

Hide file tree

Showing 6 changed files with 78 additions and 54 deletions.
diff --git a/NEWS.md b/NEWS.md
@@ -83,8 +83,9 @@ This section lists changes that do not have deprecation warnings.
     longer present. Use `first(R)` and `last(R)` to obtain
     start/stop. ([#20974])
 
-  * The `Diagonal` type definition has changed from `Diagonal{T}` to
-    `Diagonal{T,V<:AbstractVector{T}}` ([#22718]).
+  * The `Diagonal` and `Bidiagonal` type definitions have changed from `Diagonal{T}` and
+    `Bidiagonal{T}` to `Diagonal{T,V<:AbstractVector{T}}` and
+    `Bidiagonal{T,V<:AbstractVector{T}}` respectively ([#22718], [#22925]).
 
   * Spaces are no longer allowed between `@` and the name of a macro in a macro call ([#22868]).
 
@@ -152,8 +153,9 @@ Library improvements
 
   * `Char`s can now be concatenated with `String`s and/or other `Char`s using `*` ([#22532]).
 
-  * `Diagonal` is now parameterized on the type of the wrapped vector. This allows
-    for `Diagonal` matrices with arbitrary `AbstractVector`s ([#22718]).
+  * `Diagonal` and `Bidiagonal` are now parameterized on the type of the wrapped vectors,
+    allowing `Diagonal` and `Bidiagonal` matrices with arbitrary
+    `AbstractVector`s ([#22718], [#22925]).
 
   * Mutating versions of `randperm` and `randcycle` have been added:
     `randperm!` and `randcycle!` ([#22723]).
@@ -216,6 +218,9 @@ Deprecated or removed
   * `Bidiagonal` constructors now use a `Symbol` (`:U` or `:L`) for the upper/lower
     argument, instead of a `Bool` or a `Char` ([#22703]).
 
+  * `Bidiagonal` constructors that automatically converted the input vectors to
+    the same type are deprecated in favor of explicit conversion ([#22925]).
+
   * Calling `nfields` on a type to find out how many fields its instances have is deprecated.
     Use `fieldcount` instead. Use `nfields` only to get the number of fields in a specific object ([#22350]).
 

diff --git a/base/deprecated.jl b/base/deprecated.jl
@@ -1595,6 +1595,15 @@ end
 # issue #6466
 # `write` on non-isbits arrays is deprecated in io.jl.
 
+# PR #22925
+# also uncomment constructor tests in test/linalg/bidiag.jl
+function Bidiagonal(dv::AbstractVector{T}, ev::AbstractVector{S}, uplo::Symbol) where {T,S}
+    depwarn(string("Bidiagonal(dv::AbstractVector{T}, ev::AbstractVector{S}, uplo::Symbol) where {T,S}",
+        " is deprecated; manually convert both vectors to the same type instead."), :Bidiagonal)
+    R = promote_type(T, S)
+    Bidiagonal(convert(Vector{R}, dv), convert(Vector{R}, ev), uplo)
+end
+
 # END 0.7 deprecations
 
 # BEGIN 1.0 deprecations

diff --git a/base/linalg/bidiag.jl b/base/linalg/bidiag.jl
@@ -1,23 +1,23 @@
 # This file is a part of Julia. License is MIT: https://julialang.org/license
 
 # Bidiagonal matrices
-struct Bidiagonal{T} <: AbstractMatrix{T}
-    dv::Vector{T} # diagonal
-    ev::Vector{T} # sub/super diagonal
-    uplo::Char    # upper bidiagonal ('U') or lower ('L')
-    function Bidiagonal{T}(dv::Vector{T}, ev::Vector{T}, uplo::Char) where T
+struct Bidiagonal{T,V<:AbstractVector{T}} <: AbstractMatrix{T}
+    dv::V      # diagonal
+    ev::V      # sub/super diagonal
+    uplo::Char # upper bidiagonal ('U') or lower ('L')
+    function Bidiagonal{T}(dv::V, ev::V, uplo::Char) where {T,V<:AbstractVector{T}}
         if length(ev) != length(dv)-1
             throw(DimensionMismatch("length of diagonal vector is $(length(dv)), length of off-diagonal vector is $(length(ev))"))
         end
-        new(dv, ev, uplo)
+        new{T,V}(dv, ev, uplo)
     end
-    function Bidiagonal(dv::Vector{T}, ev::Vector{T}, uplo::Char) where T
+    function Bidiagonal(dv::V, ev::V, uplo::Char) where {T,V<:AbstractVector{T}}
         Bidiagonal{T}(dv, ev, uplo)
     end
 end
 
 """
-    Bidiagonal(dv, ev, uplo::Symbol)
+    Bidiagonal(dv::V, ev::V, uplo::Symbol) where V <: AbstractVector
 
 Constructs an upper (`uplo=:U`) or lower (`uplo=:L`) bidiagonal matrix using the
 given diagonal (`dv`) and off-diagonal (`ev`) vectors. The result is of type `Bidiagonal`
@@ -41,27 +41,22 @@ julia> ev = [7, 8, 9]
  9
 
 julia> Bu = Bidiagonal(dv, ev, :U) # ev is on the first superdiagonal
-4×4 Bidiagonal{Int64}:
+4×4 Bidiagonal{Int64,Array{Int64,1}}:
  1  7  ⋅  ⋅
  ⋅  2  8  ⋅
  ⋅  ⋅  3  9
  ⋅  ⋅  ⋅  4
 
 julia> Bl = Bidiagonal(dv, ev, :L) # ev is on the first subdiagonal
-4×4 Bidiagonal{Int64}:
+4×4 Bidiagonal{Int64,Array{Int64,1}}:
  1  ⋅  ⋅  ⋅
  7  2  ⋅  ⋅
  ⋅  8  3  ⋅
  ⋅  ⋅  9  4
 ```
 """
-function Bidiagonal(dv::AbstractVector{T}, ev::AbstractVector{T}, uplo::Symbol) where T
-    Bidiagonal{T}(collect(dv), collect(ev), char_uplo(uplo))
-end
-
-function Bidiagonal(dv::AbstractVector{Td}, ev::AbstractVector{Te}, uplo::Symbol) where {Td,Te}
-    T = promote_type(Td,Te)
-    Bidiagonal(convert(Vector{T}, dv), convert(Vector{T}, ev), uplo)
+function Bidiagonal(dv::V, ev::V, uplo::Symbol) where {T,V<:AbstractVector{T}}
+    Bidiagonal{T}(dv, ev, char_uplo(uplo))
 end
 
 """
@@ -79,22 +74,24 @@ julia> A = [1 1 1 1; 2 2 2 2; 3 3 3 3; 4 4 4 4]
  3  3  3  3
  4  4  4  4
 
-julia> Bidiagonal(A, :U) #contains the main diagonal and first superdiagonal of A
-4×4 Bidiagonal{Int64}:
+julia> Bidiagonal(A, :U) # contains the main diagonal and first superdiagonal of A
+4×4 Bidiagonal{Int64,Array{Int64,1}}:
  1  1  ⋅  ⋅
  ⋅  2  2  ⋅
  ⋅  ⋅  3  3
  ⋅  ⋅  ⋅  4
 
-julia> Bidiagonal(A, :L) #contains the main diagonal and first subdiagonal of A
-4×4 Bidiagonal{Int64}:
+julia> Bidiagonal(A, :L) # contains the main diagonal and first subdiagonal of A
+4×4 Bidiagonal{Int64,Array{Int64,1}}:
  1  ⋅  ⋅  ⋅
  2  2  ⋅  ⋅
  ⋅  3  3  ⋅
  ⋅  ⋅  4  4
 ```
 """
-Bidiagonal(A::AbstractMatrix, uplo::Symbol) = Bidiagonal(diag(A), diag(A, uplo == :U ? 1 : -1), uplo)
+function Bidiagonal(A::AbstractMatrix, uplo::Symbol)
+    Bidiagonal(diag(A, 0), diag(A, uplo == :U ? 1 : -1), uplo)
+end
 
 function getindex(A::Bidiagonal{T}, i::Integer, j::Integer) where T
     if !((1 <= i <= size(A,2)) && (1 <= j <= size(A,2)))
@@ -164,7 +161,8 @@ promote_rule(::Type{Tridiagonal{T}}, ::Type{Bidiagonal{S}}) where {T,S} = Tridia
 # No-op for trivial conversion Bidiagonal{T} -> Bidiagonal{T}
 convert(::Type{Bidiagonal{T}}, A::Bidiagonal{T}) where {T} = A
 # Convert Bidiagonal to Bidiagonal{T} by constructing a new instance with converted elements
-convert(::Type{Bidiagonal{T}}, A::Bidiagonal) where {T} = Bidiagonal(convert(Vector{T}, A.dv), convert(Vector{T}, A.ev), A.uplo)
+convert(::Type{Bidiagonal{T}}, A::Bidiagonal) where {T} =
+    Bidiagonal(convert(AbstractVector{T}, A.dv), convert(AbstractVector{T}, A.ev), A.uplo)
 # When asked to convert Bidiagonal to AbstractMatrix{T}, preserve structure by converting to Bidiagonal{T} <: AbstractMatrix{T}
 convert(::Type{AbstractMatrix{T}}, A::Bidiagonal) where {T} = convert(Bidiagonal{T}, A)
 

diff --git a/test/linalg/bidiag.jl b/test/linalg/bidiag.jl
@@ -24,10 +24,23 @@ srand(1)
     end
 
     @testset "Constructors" begin
-        @test Bidiagonal(dv,ev,:U) != Bidiagonal(dv,ev,:L)
-        @test_throws ArgumentError Bidiagonal(dv,ev,:R)
-        @test_throws DimensionMismatch Bidiagonal(dv,ones(elty,n),:U)
-        @test_throws MethodError Bidiagonal(dv,ev)
+        for (x, y) in ((dv, ev), (GenericArray(dv), GenericArray(ev)))
+            # from vectors
+            ubd = Bidiagonal(x, y, :U)
+            lbd = Bidiagonal(x, y, :L)
+            @test ubd != lbd
+            @test ubd.dv === x
+            @test lbd.ev === y
+            @test_throws ArgumentError Bidiagonal(x, y, :R)
+            @test_throws DimensionMismatch Bidiagonal(x, x, :U)
+            @test_throws MethodError Bidiagonal(x, y)
+            # from matrix
+            @test Bidiagonal(ubd, :U) == Bidiagonal(Matrix(ubd), :U) == ubd
+            @test Bidiagonal(lbd, :L) == Bidiagonal(Matrix(lbd), :L) == lbd
+        end
+        # enable when deprecations for 0.7 are dropped
+        # @test_throws MethodError Bidiagonal(dv, GenericArray(ev), :U)
+        # @test_throws MethodError Bidiagonal(GenericArray(dv), ev, :U)
     end
 
     @testset "getindex, setindex!, size, and similar" begin
@@ -97,29 +110,30 @@ srand(1)
         end
 
         @testset "triu and tril" begin
+            bidiagcopy(dv, ev, uplo) = Bidiagonal(copy(dv), copy(ev), uplo)
             @test istril(Bidiagonal(dv,ev,:L))
             @test !istril(Bidiagonal(dv,ev,:U))
-            @test tril!(Bidiagonal(dv,ev,:U),-1) == Bidiagonal(zeros(dv),zeros(ev),:U)
-            @test tril!(Bidiagonal(dv,ev,:L),-1) == Bidiagonal(zeros(dv),ev,:L)
-            @test tril!(Bidiagonal(dv,ev,:U),-2) == Bidiagonal(zeros(dv),zeros(ev),:U)
-            @test tril!(Bidiagonal(dv,ev,:L),-2) == Bidiagonal(zeros(dv),zeros(ev),:L)
-            @test tril!(Bidiagonal(dv,ev,:U),1)  == Bidiagonal(dv,ev,:U)
-            @test tril!(Bidiagonal(dv,ev,:L),1)  == Bidiagonal(dv,ev,:L)
-            @test tril!(Bidiagonal(dv,ev,:U))    == Bidiagonal(dv,zeros(ev),:U)
-            @test tril!(Bidiagonal(dv,ev,:L))    == Bidiagonal(dv,ev,:L)
-            @test_throws ArgumentError tril!(Bidiagonal(dv,ev,:U),n+1)
+            @test tril!(bidiagcopy(dv,ev,:U),-1) == Bidiagonal(zeros(dv),zeros(ev),:U)
+            @test tril!(bidiagcopy(dv,ev,:L),-1) == Bidiagonal(zeros(dv),ev,:L)
+            @test tril!(bidiagcopy(dv,ev,:U),-2) == Bidiagonal(zeros(dv),zeros(ev),:U)
+            @test tril!(bidiagcopy(dv,ev,:L),-2) == Bidiagonal(zeros(dv),zeros(ev),:L)
+            @test tril!(bidiagcopy(dv,ev,:U),1)  == Bidiagonal(dv,ev,:U)
+            @test tril!(bidiagcopy(dv,ev,:L),1)  == Bidiagonal(dv,ev,:L)
+            @test tril!(bidiagcopy(dv,ev,:U))    == Bidiagonal(dv,zeros(ev),:U)
+            @test tril!(bidiagcopy(dv,ev,:L))    == Bidiagonal(dv,ev,:L)
+            @test_throws ArgumentError tril!(bidiagcopy(dv,ev,:U),n+1)
 
             @test istriu(Bidiagonal(dv,ev,:U))
             @test !istriu(Bidiagonal(dv,ev,:L))
-            @test triu!(Bidiagonal(dv,ev,:L),1)  == Bidiagonal(zeros(dv),zeros(ev),:L)
-            @test triu!(Bidiagonal(dv,ev,:U),1)  == Bidiagonal(zeros(dv),ev,:U)
-            @test triu!(Bidiagonal(dv,ev,:U),2)  == Bidiagonal(zeros(dv),zeros(ev),:U)
-            @test triu!(Bidiagonal(dv,ev,:L),2)  == Bidiagonal(zeros(dv),zeros(ev),:L)
-            @test triu!(Bidiagonal(dv,ev,:U),-1) == Bidiagonal(dv,ev,:U)
-            @test triu!(Bidiagonal(dv,ev,:L),-1) == Bidiagonal(dv,ev,:L)
-            @test triu!(Bidiagonal(dv,ev,:L))    == Bidiagonal(dv,zeros(ev),:L)
-            @test triu!(Bidiagonal(dv,ev,:U))    == Bidiagonal(dv,ev,:U)
-            @test_throws ArgumentError triu!(Bidiagonal(dv,ev,:U),n+1)
+            @test triu!(bidiagcopy(dv,ev,:L),1)  == Bidiagonal(zeros(dv),zeros(ev),:L)
+            @test triu!(bidiagcopy(dv,ev,:U),1)  == Bidiagonal(zeros(dv),ev,:U)
+            @test triu!(bidiagcopy(dv,ev,:U),2)  == Bidiagonal(zeros(dv),zeros(ev),:U)
+            @test triu!(bidiagcopy(dv,ev,:L),2)  == Bidiagonal(zeros(dv),zeros(ev),:L)
+            @test triu!(bidiagcopy(dv,ev,:U),-1) == Bidiagonal(dv,ev,:U)
+            @test triu!(bidiagcopy(dv,ev,:L),-1) == Bidiagonal(dv,ev,:L)
+            @test triu!(bidiagcopy(dv,ev,:L))    == Bidiagonal(dv,zeros(ev),:L)
+            @test triu!(bidiagcopy(dv,ev,:U))    == Bidiagonal(dv,ev,:U)
+            @test_throws ArgumentError triu!(bidiagcopy(dv,ev,:U),n+1)
         end
 
         Tfull = Array(T)
@@ -255,7 +269,6 @@ srand(1)
     end
     BD = Bidiagonal(dv, ev, :U)
     @test Matrix{Complex{Float64}}(BD) == BD
-
 end
 
 # Issue 10742 and similar

diff --git a/test/linalg/cholesky.jl b/test/linalg/cholesky.jl
@@ -101,8 +101,7 @@ using Base.LinAlg: BlasComplex, BlasFloat, BlasReal, QRPivoted, PosDefException
 
         # test chol of 2x2 Strang matrix
         S = convert(AbstractMatrix{eltya},full(SymTridiagonal([2,2],[-1])))
-        U = Bidiagonal([2,sqrt(eltya(3))],[-1],:U) / sqrt(eltya(2))
-        @test full(chol(S)) ≈ full(U)
+        @test full(chol(S)) ≈ [2 -1; 0 sqrt(eltya(3))] / sqrt(eltya(2))
 
         # test extraction of factor and re-creating original matrix
         if eltya <: Real

diff --git a/test/show.jl b/test/show.jl
@@ -556,8 +556,8 @@ end
 # test structured zero matrix printing for select structured types
 A = reshape(1:16,4,4)
 @test replstr(Diagonal(A)) == "4×4 Diagonal{$(Int),Array{$(Int),1}}:\n 1  ⋅   ⋅   ⋅\n ⋅  6   ⋅   ⋅\n ⋅  ⋅  11   ⋅\n ⋅  ⋅   ⋅  16"
-@test replstr(Bidiagonal(A,:U)) == "4×4 Bidiagonal{$Int}:\n 1  5   ⋅   ⋅\n ⋅  6  10   ⋅\n ⋅  ⋅  11  15\n ⋅  ⋅   ⋅  16"
-@test replstr(Bidiagonal(A,:L)) == "4×4 Bidiagonal{$Int}:\n 1  ⋅   ⋅   ⋅\n 2  6   ⋅   ⋅\n ⋅  7  11   ⋅\n ⋅  ⋅  12  16"
+@test replstr(Bidiagonal(A,:U)) == "4×4 Bidiagonal{$(Int),Array{$(Int),1}}:\n 1  5   ⋅   ⋅\n ⋅  6  10   ⋅\n ⋅  ⋅  11  15\n ⋅  ⋅   ⋅  16"
+@test replstr(Bidiagonal(A,:L)) == "4×4 Bidiagonal{$(Int),Array{$(Int),1}}:\n 1  ⋅   ⋅   ⋅\n 2  6   ⋅   ⋅\n ⋅  7  11   ⋅\n ⋅  ⋅  12  16"
 @test replstr(SymTridiagonal(A+A')) == "4×4 SymTridiagonal{$Int}:\n 2   7   ⋅   ⋅\n 7  12  17   ⋅\n ⋅  17  22  27\n ⋅   ⋅  27  32"
 @test replstr(Tridiagonal(diag(A,-1),diag(A),diag(A,+1))) == "4×4 Tridiagonal{$Int}:\n 1  5   ⋅   ⋅\n 2  6  10   ⋅\n ⋅  7  11  15\n ⋅  ⋅  12  16"
 @test replstr(UpperTriangular(copy(A))) == "4×4 UpperTriangular{$Int,Array{$Int,2}}:\n 1  5   9  13\n ⋅  6  10  14\n ⋅  ⋅  11  15\n ⋅  ⋅   ⋅  16"