Skip to content

Commit

Permalink
LinAlg.fillslots! -> LinAlg.fillstored! (#25030)
Browse files Browse the repository at this point in the history 
* rename LinAlg.fillslots! LinAlg.fillstored!

* rewrite fillstored!(::SpecialMatrix, x) without generated function
move implementations to more suitable places

* implement fillband! and call it from fillstored!(::AbstractTriangular)

* review fixes
  • Loading branch information
@fredrikekre
fredrikekre authored Dec 14, 2017
1 parent e09eca2 commit 1f33cdd
Show file tree
Hide file tree
Showing 7 changed files with 92 additions and 87 deletions.
4 changes: 3 additions & 1 deletion NEWS.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -471,8 +471,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
Original file line number Diff line number Diff line change
Expand Up @@ -1882,8 +1882,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
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
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
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, since some of its entries are constrained."))
end
5 changes: 5 additions & 0 deletions base/linalg/triangular.jl
View file
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
95 changes: 45 additions & 50 deletions test/linalg/bidiag.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -310,58 +310,53 @@ 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])
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])
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])
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
@test iszero(fill!(A, 0))
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
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))
# 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
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
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
end
end
Expand Down

1 comment on commit 1f33cdd

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

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

Executing the daily benchmark build, I will reply here when finished:

@nanosoldier runbenchmarks(ALL, isdaily = true)

Please sign in to comment.