Skip to content

LinAlg.fillslots! -> LinAlg.fillstored! #25030

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

Merged
fredrikekre merged 4 commits into from
Dec 14, 2017
Merged

LinAlg.fillslots! -> LinAlg.fillstored! #25030

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bc28fbf
rename LinAlg.fillslots! LinAlg.fillstored!
fredrikekre Dec 11, 2017
197ada2
rewrite fillstored!(::SpecialMatrix, x) without generated function
fredrikekre Dec 11, 2017
893281d
implement fillband! and call it from fillstored!(::AbstractTriangular)
fredrikekre Dec 13, 2017
69369a7
review fixes
fredrikekre Dec 14, 2017
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: 3 additions & 1 deletion NEWS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -465,8 +465,10 @@ Deprecated or removed
`Matrix{Int}(uninitialized, (2, 4))`, and `Array{Float32,3}(11, 13, 17)` is now
`Array{Float32,3}(uninitialized, 11, 13, 17)` ([#24781]).

* `LinAlg.fillslots!` has been renamed `LinAlg.fillstored!` ([#25030]).

* `fill!(A::Diagonal, x)` and `fill!(A::AbstractTriangular, x)` have been deprecated
in favor of `Base.LinAlg.fillslots!(A, x)` ([#24413]).
in favor of `Base.LinAlg.fillstored!(A, x)` ([#24413]).

* `eye` has been deprecated in favor of `I` and `Matrix` constructors. Please see the
deprecation warnings for replacement details ([#24438]).
Expand Down
7 changes: 5 additions & 2 deletions base/deprecated.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1855,8 +1855,11 @@ end
# Also un-comment the new definition in base/indices.jl

# deprecate odd fill! methods
@deprecate fill!(D::Diagonal, x) LinAlg.fillslots!(D, x)
@deprecate fill!(A::Base.LinAlg.AbstractTriangular, x) LinAlg.fillslots!(A, x)
@deprecate fill!(D::Diagonal, x) LinAlg.fillstored!(D, x)
@deprecate fill!(A::Base.LinAlg.AbstractTriangular, x) LinAlg.fillstored!(A, x)

# PR #25030
@eval LinAlg @deprecate fillslots! fillstored! false

function diagm(v::BitVector)
depwarn(string("diagm(v::BitVector) is deprecated, use diagm(0 => v) or ",
Expand Down
34 changes: 0 additions & 34 deletions base/linalg/bidiag.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -636,37 +636,3 @@ function eigvecs(M::Bidiagonal{T}) where T
Q #Actually Triangular
end
eigfact(M::Bidiagonal) = Eigen(eigvals(M), eigvecs(M))

# fill! methods
_valuefields(::Type{<:Diagonal}) = [:diag]
_valuefields(::Type{<:Bidiagonal}) = [:dv, :ev]
_valuefields(::Type{<:Tridiagonal}) = [:dl, :d, :du]
_valuefields(::Type{<:SymTridiagonal}) = [:dv, :ev]
_valuefields(::Type{<:AbstractTriangular}) = [:data]

const SpecialArrays = Union{Diagonal,Bidiagonal,Tridiagonal,SymTridiagonal,AbstractTriangular}

function fillslots!(A::SpecialArrays, x)
xT = convert(eltype(A), x)
if @generated
quote
$([ :(fill!(A.$field, xT)) for field in _valuefields(A) ]...)
end
else
for field in _valuefields(A)
fill!(getfield(A, field), xT)
end
end
return A
end

_small_enough(A::Bidiagonal) = size(A, 1) <= 1
_small_enough(A::Tridiagonal) = size(A, 1) <= 2
_small_enough(A::SymTridiagonal) = size(A, 1) <= 2

function fill!(A::Union{Bidiagonal,Tridiagonal,SymTridiagonal}, x)
xT = convert(eltype(A), x)
(xT == zero(eltype(A)) || _small_enough(A)) && return fillslots!(A, xT)
throw(ArgumentError("array A of type $(typeof(A)) and size $(size(A)) can
not be filled with x=$x, since some of its entries are constrained."))
end
16 changes: 16 additions & 0 deletions base/linalg/dense.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,22 @@ function tril!(M::AbstractMatrix, k::Integer)
end
tril(M::Matrix, k::Integer) = tril!(copy(M), k)

"""
fillband!(A::AbstractMatrix, x, l, u)

Fill the band between diagonals `l` and `u` with the value `x`.
"""
function fillband!(A::AbstractMatrix{T}, x, l, u) where T
m, n = size(A)
xT = convert(T, x)
for j in 1:n
for i in max(1,j-u):min(m,j-l)
@inbounds A[i, j] = xT
end
end
return A
end

function diagind(m::Integer, n::Integer, k::Integer=0)
if !(-m <= k <= n)
throw(ArgumentError(string("requested diagonal, $k, must be at least $(-m) and ",
Expand Down
18 changes: 18 additions & 0 deletions base/linalg/special.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,3 +126,21 @@ mul!(A::AbstractTriangular, adjB::Adjoint{<:Any,<:Union{QRCompactWYQ,QRPackedQ}}
(B = adjB.parent; mul!(full!(A), Adjoint(B)))
*(A::AbstractTriangular, adjB::Adjoint{<:Any,<:Union{QRCompactWYQ,QRPackedQ}}) =
(B = adjB.parent; *(copy!(similar(parent(A)), A), Adjoint(B)))

# fill[stored]! methods
fillstored!(A::Diagonal, x) = (fill!(A.diag, x); A)
fillstored!(A::Bidiagonal, x) = (fill!(A.dv, x); fill!(A.ev, x); A)
fillstored!(A::Tridiagonal, x) = (fill!(A.dl, x); fill!(A.d, x); fill!(A.du, x); A)
fillstored!(A::SymTridiagonal, x) = (fill!(A.dv, x); fill!(A.ev, x); A)

_small_enough(A::Bidiagonal) = size(A, 1) <= 1
_small_enough(A::Tridiagonal) = size(A, 1) <= 2
_small_enough(A::SymTridiagonal) = size(A, 1) <= 2

# TODO: Add Diagonal to this method when 0.7 deprecations are removed
function fill!(A::Union{Bidiagonal,Tridiagonal,SymTridiagonal}, x)
xT = convert(eltype(A), x)
(iszero(xT) || _small_enough(A)) && return fillstored!(A, xT)
throw(ArgumentError("array of type $(typeof(A)) and size $(size(A)) can
not be filled with x=$x, since some of its entries are constrained."))
Copy link
Member

@Sacha0 Sacha0 Dec 14, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Perhaps nix the x=, given that what x refers to is not user-visible?

end
5 changes: 5 additions & 0 deletions base/linalg/triangular.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -418,6 +418,11 @@ end
scale!(A::Union{UpperTriangular,LowerTriangular}, c::Number) = scale!(A,A,c)
scale!(c::Number, A::Union{UpperTriangular,LowerTriangular}) = scale!(A,c)

fillstored!(A::LowerTriangular, x) = (fillband!(A.data, x, 1-size(A,1), 0); A)
fillstored!(A::UnitLowerTriangular, x) = (fillband!(A.data, x, 1-size(A,1), -1); A)
fillstored!(A::UpperTriangular, x) = (fillband!(A.data, x, 0, size(A,2)-1); A)
fillstored!(A::UnitUpperTriangular, x) = (fillband!(A.data, x, 1, size(A,2)-1); A)

# Binary operations
+(A::UpperTriangular, B::UpperTriangular) = UpperTriangular(A.data + B.data)
+(A::LowerTriangular, B::LowerTriangular) = LowerTriangular(A.data + B.data)
Expand Down
98 changes: 48 additions & 50 deletions test/linalg/bidiag.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,58 +310,56 @@ end
@test promote(C,A) isa Tuple{Tridiagonal, Tridiagonal}
end

import Base.LinAlg: fillslots!, UnitLowerTriangular
@testset "fill! and fillslots!" begin
let #fill!
let # fillslots!
A = Tridiagonal(randn(2), randn(3), randn(2))
@test fillslots!(A, 3) == Tridiagonal([3, 3.], [3, 3, 3.], [3, 3.])
B = Bidiagonal(randn(3), randn(2), :U)
@test fillslots!(B, 2) == Bidiagonal([2.,2,2], [2,2.], :U)
S = SymTridiagonal(randn(3), randn(2))
@test fillslots!(S, 1) == SymTridiagonal([1,1,1.], [1,1.])
Ult = UnitLowerTriangular(randn(3,3))
@test fillslots!(Ult, 3) == UnitLowerTriangular([1 0 0; 3 1 0; 3 3 1])
end
let # fill!(exotic, 0)
exotic_arrays = Any[Tridiagonal(randn(3), randn(4), randn(3)),
Bidiagonal(randn(3), randn(2), rand([:U,:L])),
SymTridiagonal(randn(3), randn(2)),
sparse(randn(3,4)),
# Diagonal(randn(5)), # Diagonal fill! deprecated, see below
sparse(rand(3)),
# LowerTriangular(randn(3,3)), # AbstractTriangular fill! deprecated, see below
# UpperTriangular(randn(3,3)) # AbstractTriangular fill! deprecated, see below
]
for A in exotic_arrays
fill!(A, 0)
for a in A
@test a == 0
end
using Base.LinAlg: fillstored!, UnitLowerTriangular
@testset "fill! and fillstored!" begin
let # fillstored!
A = Tridiagonal(randn(2), randn(3), randn(2))
@test fillstored!(A, 3) == Tridiagonal([3, 3.], [3, 3, 3.], [3, 3.])
Copy link
Member

@Sacha0 Sacha0 Dec 14, 2017

Choose a reason for hiding this comment

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

No need for the trailing .s given that == does not check type equality?

B = Bidiagonal(randn(3), randn(2), :U)
@test fillstored!(B, 2) == Bidiagonal([2.,2,2], [2,2.], :U)
S = SymTridiagonal(randn(3), randn(2))
@test fillstored!(S, 1) == SymTridiagonal([1,1,1.], [1,1.])
Ult = UnitLowerTriangular(randn(3,3))
@test fillstored!(Ult, 3) == UnitLowerTriangular([1 0 0; 3 1 0; 3 3 1])
Copy link
Member

@Sacha0 Sacha0 Dec 14, 2017

Choose a reason for hiding this comment

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

Perhaps the other triangular types are worth testing as well?

end
let # fill!(exotic, 0)
exotic_arrays = Any[Tridiagonal(randn(3), randn(4), randn(3)),
Bidiagonal(randn(3), randn(2), rand([:U,:L])),
SymTridiagonal(randn(3), randn(2)),
sparse(randn(3,4)),
# Diagonal(randn(5)), # Diagonal fill! deprecated, see below
sparse(rand(3)),
# LowerTriangular(randn(3,3)), # AbstractTriangular fill! deprecated, see below
# UpperTriangular(randn(3,3)) # AbstractTriangular fill! deprecated, see below
]
for A in exotic_arrays
fill!(A, 0)
for a in A
Copy link
Member

@Sacha0 Sacha0 Dec 14, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Perhaps @test iszero(fill!(A, 0))?

@test a == 0
end
# Diagonal and AbstractTriangular fill! were defined as fillslots!,
# not matching the general behavior of fill!, and so have been deprecated.
# In a future dev cycle, these fill! methods should probably be reintroduced
# with behavior matching that of fill! for other structured matrix types.
# In the interm, equivalently test fillslots! below
@test iszero(fillslots!(Diagonal(fill(1, 3)), 0))
@test iszero(fillslots!(LowerTriangular(fill(1, 3, 3)), 0))
@test iszero(fillslots!(UpperTriangular(fill(1, 3, 3)), 0))
end
let # fill!(small, x)
val = randn()
b = Bidiagonal(randn(1,1), :U)
st = SymTridiagonal(randn(1,1))
for x in (b, st)
@test Array(fill!(x, val)) == fill!(Array(x), val)
end
b = Bidiagonal(randn(2,2), :U)
st = SymTridiagonal(randn(3), randn(2))
t = Tridiagonal(randn(3,3))
for x in (b, t, st)
@test_throws ArgumentError fill!(x, val)
@test Array(fill!(x, 0)) == fill!(Array(x), 0)
end
# Diagonal and AbstractTriangular fill! were defined as fillstored!,
# not matching the general behavior of fill!, and so have been deprecated.
# In a future dev cycle, these fill! methods should probably be reintroduced
# with behavior matching that of fill! for other structured matrix types.
# In the interm, equivalently test fillstored! below
@test iszero(fillstored!(Diagonal(fill(1, 3)), 0))
@test iszero(fillstored!(LowerTriangular(fill(1, 3, 3)), 0))
@test iszero(fillstored!(UpperTriangular(fill(1, 3, 3)), 0))
end
let # fill!(small, x)
val = randn()
b = Bidiagonal(randn(1,1), :U)
st = SymTridiagonal(randn(1,1))
for x in (b, st)
@test Array(fill!(x, val)) == fill!(Array(x), val)
end
Copy link
Member

@Sacha0 Sacha0 Dec 14, 2017

Choose a reason for hiding this comment

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

Perhaps simplify to the following?

@test fill!(Bidiagonal(fill(0, 1, 1), :U), 2) == fill(2, 1, 1)
@test fill!(SymTridiagonal(fill(0, 1, 1)), 2) == fill(2, 1, 1)

b = Bidiagonal(randn(2,2), :U)
st = SymTridiagonal(randn(3), randn(2))
t = Tridiagonal(randn(3,3))
for x in (b, t, st)
@test_throws ArgumentError fill!(x, val)
@test Array(fill!(x, 0)) == fill!(Array(x), 0)
Copy link
Member

@Sacha0 Sacha0 Dec 14, 2017

Choose a reason for hiding this comment

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

Perhaps simplify to @test fill!(x, 0) == ...?

end
end
end
Expand Down