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.

Sign up for GitHub

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

Jump to bottom

parametrize Bidiagonal on wrapped vector type #22925

Merged
merged 1 commit into from
Aug 3, 2017
Merged

parametrize Bidiagonal on wrapped vector type #22925

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
13 changes: 9 additions & 4 deletions NEWS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,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]).

Expand Down Expand Up @@ -142,8 +143,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]).
Expand Down Expand Up @@ -206,6 +208,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]).

Expand Down
9 changes: 9 additions & 0 deletions base/deprecated.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
44 changes: 21 additions & 23 deletions base/linalg/bidiag.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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`
Expand All @@ -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

"""
Expand All @@ -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)
Copy link
Contributor

Choose a reason for hiding this comment

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

I didn't actually realize diag(A, k) would already give a SparseVector

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, as long as the two arg form is inferable this should be "more correct" since diag(A) in some cases give something else.

Copy link
Contributor

Choose a reason for hiding this comment

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

which isn't very consistent with it acting like a default argument, but that's a discussion for a different issue

end

function getindex(A::Bidiagonal{T}, i::Integer, j::Integer) where T
if !((1 <= i <= size(A,2)) && (1 <= j <= size(A,2)))
Expand Down Expand Up @@ -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)

Expand Down
59 changes: 36 additions & 23 deletions test/linalg/bidiag.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -97,29 +110,30 @@ srand(1)
end

@testset "triu and tril" begin
bidiagcopy(dv, ev, uplo) = Bidiagonal(copy(dv), copy(ev), uplo)
Copy link
Contributor

Choose a reason for hiding this comment

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

hm, so was this previously writing zeros through to these?

Copy link
Member Author

Choose a reason for hiding this comment

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

No but previously the constructor always made a copy of the vectors, ref e0d75d7#commitcomment-22968553

Copy link
Contributor

Choose a reason for hiding this comment

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

were Tridiagonal or SymTridiagonal behaving like that too, or was Bidiagonal the only case of it?

Copy link
Member Author

Choose a reason for hiding this comment

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

e0d75d7 only changed the behaviour of Bidiagonal. The other ones still have constructors that don't copy: Diagonal, Tridiagonal and SymTridiagonal.

@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)
Expand Down Expand Up @@ -255,7 +269,6 @@ srand(1)
end
BD = Bidiagonal(dv, ev, :U)
@test Matrix{Complex{Float64}}(BD) == BD

end

# Issue 10742 and similar
Expand Down
3 changes: 1 addition & 2 deletions test/linalg/cholesky.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions test/show.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down