Skip to content

Commit e37c0e0

Browse files
mbaumanmbauman
committed
Remove kludgy VectorStyle and MatrixStyle
These broadcast styles were introduced in response to #23939 (comment) as a way to limit the "greediness" of Sparse's broadcasting implementation -- sparse only wanted to allow known combinations of array types (including Array but not any AbstractArray). The idea was to allow us to gradually improve the sparse broadcast implementation over 1.x in a non-breaking manner. Unfortunately, these special styles for Array make defining new styles in the heirarchy a bit of a pain (ref. #23939 (comment)), and it was making my life harder in getting the 1.0 breaking changes in. This commit removes these special broadcast styles in favor of just having Sparse identify the cases itself and re-dispatch back into the default implementation in the cases it doesn't know how to handle.
1 parent 983f0f9 commit e37c0e0

File tree

3 files changed

+26
-60
lines changed

3 files changed

+26
-60
lines changed

‎base/broadcast.jl

Lines changed: 1 addition & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -160,32 +160,6 @@ BroadcastStyle(a::AbstractArrayStyle{N}, ::DefaultArrayStyle{N}) where N = a
160160
BroadcastStyle(a::AbstractArrayStyle{M}, ::DefaultArrayStyle{N}) where {M,N} =
161161
typeof(a)(_max(Val(M),Val(N)))
162162

163-
# FIXME
164-
# The following definitions are necessary to limit SparseArray broadcasting to "plain Arrays"
165-
# (see https://github.com/JuliaLang/julia/pull/23939#pullrequestreview-72075382).
166-
# They should be deleted once the sparse broadcast infrastucture is capable of handling
167-
# arbitrary AbstractArrays.
168-
struct VectorStyle <: AbstractArrayStyle{1} end
169-
struct MatrixStyle <: AbstractArrayStyle{2} end
170-
const VMStyle = Union{VectorStyle,MatrixStyle}
171-
# These lose to DefaultArrayStyle
172-
VectorStyle(::Val{N}) where N = DefaultArrayStyle{N}()
173-
MatrixStyle(::Val{N}) where N = DefaultArrayStyle{N}()
174-
175-
BroadcastStyle(::Type{<:Vector}) = VectorStyle()
176-
BroadcastStyle(::Type{<:Matrix}) = MatrixStyle()
177-
178-
BroadcastStyle(::MatrixStyle, ::VectorStyle) = MatrixStyle()
179-
BroadcastStyle(a::AbstractArrayStyle{Any}, ::VectorStyle) = a
180-
BroadcastStyle(a::AbstractArrayStyle{Any}, ::MatrixStyle) = a
181-
BroadcastStyle(a::AbstractArrayStyle{N}, ::VectorStyle) where N = typeof(a)(_max(Val(N), Val(1)))
182-
BroadcastStyle(a::AbstractArrayStyle{N}, ::MatrixStyle) where N = typeof(a)(_max(Val(N), Val(2)))
183-
BroadcastStyle(::VectorStyle, ::DefaultArrayStyle{N}) where N = DefaultArrayStyle(_max(Val(N), Val(1)))
184-
BroadcastStyle(::MatrixStyle, ::DefaultArrayStyle{N}) where N = DefaultArrayStyle(_max(Val(N), Val(2)))
185-
# to avoid the VectorStyle(::Val) constructor we also need the following
186-
BroadcastStyle(::VectorStyle, ::MatrixStyle) = MatrixStyle()
187-
# end FIXME
188-
189163
## Allocating the output container
190164
"""
191165
broadcast_similar(f, ::BroadcastStyle, ::Type{ElType}, inds, As...)
@@ -205,17 +179,6 @@ broadcast_similar(f, ::ArrayConflict, ::Type{ElType}, inds::Indices, As...) wher
205179
broadcast_similar(f, ::ArrayConflict, ::Type{Bool}, inds::Indices, As...) =
206180
similar(BitArray, inds)
207181

208-
# FIXME: delete when we get rid of VectorStyle and MatrixStyle
209-
broadcast_similar(f, ::VectorStyle, ::Type{ElType}, inds::Indices{1}, As...) where ElType =
210-
similar(Vector{ElType}, inds)
211-
broadcast_similar(f, ::MatrixStyle, ::Type{ElType}, inds::Indices{2}, As...) where ElType =
212-
similar(Matrix{ElType}, inds)
213-
broadcast_similar(f, ::VectorStyle, ::Type{Bool}, inds::Indices{1}, As...) =
214-
similar(BitArray, inds)
215-
broadcast_similar(f, ::MatrixStyle, ::Type{Bool}, inds::Indices{2}, As...) =
216-
similar(BitArray, inds)
217-
# end FIXME
218-
219182
## Computing the result's indices. Most types probably won't need to specialize this.
220183
broadcast_indices() = ()
221184
broadcast_indices(::Type{T}) where T = ()
@@ -628,7 +591,7 @@ julia> string.(("one","two","three","four"), ": ", 1:4)
628591
broadcast(f, s, combine_eltypes(f, A, Bs...), combine_indices(A, Bs...),
629592
A, Bs...)
630593

631-
const NonleafHandlingTypes = Union{DefaultArrayStyle,ArrayConflict,VectorStyle,MatrixStyle}
594+
const NonleafHandlingTypes = Union{DefaultArrayStyle,ArrayConflict}
632595

633596
@inline function broadcast(f, s::NonleafHandlingTypes, ::Type{ElType}, inds::Indices, As...) where ElType
634597
if !Base.isconcretetype(ElType)

‎stdlib/Pkg3/src/precompile.jl

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -683,7 +683,6 @@ precompile(Tuple{typeof(Core.Compiler.getindex), Tuple{String, typeof(Base.info)
683683
precompile(Tuple{typeof(Core.Compiler.getindex), Tuple{UInt128, UInt128}, Int64})
684684
precompile(Tuple{typeof(Core.Compiler.getindex), Tuple{UInt32}, Int64})
685685
precompile(Tuple{typeof(Core.Compiler.getindex), Tuple{typeof(Pkg3.BinaryProvider.parse_tar_list)}, Int64})
686-
precompile(Tuple{typeof(Core.Compiler.getindex), Type{Any}, Core.Compiler.Const, Core.Compiler.Const, Type{Base.Broadcast.VectorStyle}, Core.Compiler.Const, Type{Tuple{Base.OneTo{Int64}}}, Type{Tuple{Array{Base.SubString{String}, 1}}}})
687686
precompile(Tuple{typeof(Core.Compiler.getindex), Type{Any}, GlobalRef, Bool, typeof(Pkg3.Types.parse_toml), Expr})
688687
precompile(Tuple{typeof(Core.Compiler.getindex), Type{Any}, GlobalRef, Core.SSAValue, Bool, Expr})
689688
precompile(Tuple{typeof(Core.Compiler.getindex), Type{Any}, GlobalRef, Core.SlotNumber, QuoteNode, Expr})

‎stdlib/SparseArrays/src/higherorderfns.jl

Lines changed: 25 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -973,35 +973,39 @@ broadcast(f::Tf, A::SparseMatrixCSC, ::Type{T}) where {Tf,T} = broadcast(x -> f(
973973
# and rebroadcast. otherwise, divert to generic AbstractArray broadcast code.
974974

975975
struct PromoteToSparse <: Broadcast.AbstractArrayStyle{2} end
976-
const StructuredMatrix = Union{Diagonal,Bidiagonal,Tridiagonal,SymTridiagonal}
977-
Broadcast.BroadcastStyle(::Type{<:StructuredMatrix}) = PromoteToSparse()
978-
979976
PromoteToSparse(::Val{0}) = PromoteToSparse()
980977
PromoteToSparse(::Val{1}) = PromoteToSparse()
981978
PromoteToSparse(::Val{2}) = PromoteToSparse()
982979
PromoteToSparse(::Val{N}) where N = Broadcast.DefaultArrayStyle{N}()
983980

984-
Broadcast.BroadcastStyle(::PromoteToSparse, ::SPVM) = PromoteToSparse()
985-
Broadcast.BroadcastStyle(::PromoteToSparse, ::Broadcast.Style{Tuple}) = Broadcast.DefaultArrayStyle{2}()
986-
987-
# FIXME: switch to DefaultArrayStyle once we can delete VectorStyle and MatrixStyle
988-
# Broadcast.BroadcastStyle(::SPVM, ::Broadcast.DefaultArrayStyle{0}) = PromoteToSparse()
989-
# Broadcast.BroadcastStyle(::SPVM, ::Broadcast.DefaultArrayStyle{1}) = PromoteToSparse()
990-
# Broadcast.BroadcastStyle(::SPVM, ::Broadcast.DefaultArrayStyle{2}) = PromoteToSparse()
991-
Broadcast.BroadcastStyle(::Type{<:Adjoint{T,<:Vector} where T}) = Broadcast.MatrixStyle() # Adjoint not yet defined when broadcast.jl loaded
992-
Broadcast.BroadcastStyle(::Type{<:Transpose{T,<:Vector} where T}) = Broadcast.MatrixStyle() # Transpose not yet defined when broadcast.jl loaded
981+
const StructuredMatrix = Union{Diagonal,Bidiagonal,Tridiagonal,SymTridiagonal}
982+
Broadcast.BroadcastStyle(::Type{<:StructuredMatrix}) = PromoteToSparse()
993983
Broadcast.BroadcastStyle(::Type{<:Adjoint{T,<:Union{SparseVector,SparseMatrixCSC}} where T}) = PromoteToSparse()
994984
Broadcast.BroadcastStyle(::Type{<:Transpose{T,<:Union{SparseVector,SparseMatrixCSC}} where T}) = PromoteToSparse()
995-
Broadcast.BroadcastStyle(::SPVM, ::Broadcast.VectorStyle) = PromoteToSparse()
996-
Broadcast.BroadcastStyle(::SPVM, ::Broadcast.MatrixStyle) = PromoteToSparse()
997-
Broadcast.BroadcastStyle(::SparseVecStyle, ::Broadcast.DefaultArrayStyle{N}) where N =
998-
Broadcast.DefaultArrayStyle(Broadcast._max(Val(N), Val(1)))
999-
Broadcast.BroadcastStyle(::SparseMatStyle, ::Broadcast.DefaultArrayStyle{N}) where N =
1000-
Broadcast.DefaultArrayStyle(Broadcast._max(Val(N), Val(2)))
1001-
# end FIXME
1002985

1003-
broadcast(f, ::PromoteToSparse, ::Nothing, ::Nothing, As::Vararg{Any,N}) where {N} =
1004-
broadcast(f, map(_sparsifystructured, As)...)
986+
Broadcast.BroadcastStyle(::SPVM, ::Broadcast.DefaultArrayStyle{0}) = PromoteToSparse()
987+
Broadcast.BroadcastStyle(::SPVM, ::Broadcast.DefaultArrayStyle{1}) = PromoteToSparse()
988+
Broadcast.BroadcastStyle(::SPVM, ::Broadcast.DefaultArrayStyle{2}) = PromoteToSparse()
989+
Broadcast.BroadcastStyle(::PromoteToSparse, ::SPVM) = PromoteToSparse()
990+
Broadcast.BroadcastStyle(::PromoteToSparse, ::Broadcast.Style{Tuple}) = Broadcast.DefaultArrayStyle{2}()
991+
992+
# FIXME: currently sparse broadcasts are only well-tested on known array types, while any AbstractArray
993+
# could report itself as a DefaultArrayStyle().
994+
# See https://github.com/JuliaLang/julia/pull/23939#pullrequestreview-72075382 for more details
995+
is_supported_sparse_broadcast() = true
996+
is_supported_sparse_broadcast(::AbstractArray, rest...) = false
997+
is_supported_sparse_broadcast(::AbstractSparseArray, rest...) = is_supported_sparse_broadcast(rest...)
998+
is_supported_sparse_broadcast(::StructuredMatrix, rest...) = is_supported_sparse_broadcast(rest...)
999+
is_supported_sparse_broadcast(::Array, rest...) = is_supported_sparse_broadcast(rest...)
1000+
is_supported_sparse_broadcast(t::Union{Transpose, Adjoint}, rest...) = is_supported_sparse_broadcast(t.parent, rest...)
1001+
is_supported_sparse_broadcast(x, rest...) = BroadcastStyle(typeof(x)) === Broadcast.Scalar() && is_supported_sparse_broadcast(rest...)
1002+
function broadcast(f, s::PromoteToSparse, ::Nothing, ::Nothing, As::Vararg{Any,N}) where {N}
1003+
if is_supported_sparse_broadcast(As...)
1004+
return broadcast(f, map(_sparsifystructured, As)...)
1005+
else
1006+
return broadcast(f, Broadcast.ArrayConflict(), nothing, nothing, As...)
1007+
end
1008+
end
10051009

10061010
# For broadcast! with ::Any inputs, we need a layer of indirection to determine whether
10071011
# the inputs can be promoted to SparseVecOrMat. If it's just SparseVecOrMat and scalars,

0 commit comments

Comments
 (0)