Skip to content

Commit

Permalink
Extend sparse broadcast! to combinations of broadcast scalars and spa…
Browse files Browse the repository at this point in the history
…rse vectors/matrices.

Makes broadcast! dispatch on container type (as broadcast), and inject generic sparse broadcast! for the appropriate container type.
  • Loading branch information
Sacha0 committed Jan 9, 2017
1 parent b9cc65b commit 746dbb0
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 4 deletions.
4 changes: 3 additions & 1 deletion base/broadcast.jl
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,9 @@ Note that `dest` is only used to store the result, and does not supply
arguments to `f` unless it is also listed in the `As`,
as in `broadcast!(f, A, A, B)` to perform `A[:] = broadcast(f, A, B)`.
"""
@inline function broadcast!{N}(f, C::AbstractArray, A, Bs::Vararg{Any,N})
@inline broadcast!{N}(f, C::AbstractArray, A, Bs::Vararg{Any,N}) =
broadcast_c!(f, containertype(C, A, Bs...), C, A, Bs...)
@inline function broadcast_c!{N}(f, ::Type, C::AbstractArray, A, Bs::Vararg{Any,N})

This comment has been minimized.

Copy link
@andreasnoack

andreasnoack Jun 5, 2017

Member

@Sacha0 Why do you need the ::Type argument here? Why not just use the type of the destination array instead?

This comment has been minimized.

Copy link
@Sacha0

Sacha0 Jun 5, 2017

Author Member

Not certain I follow you, so let's iterate :). First iterate: [... not accurate, see below for clarification ...] Does that answer your question? :)

This comment has been minimized.

Copy link
@andreasnoack

andreasnoack Jun 5, 2017

Member

Why not just use the type of C for this? If AbstractArray is too restrictive then it could just be Any, right?

This comment has been minimized.

Copy link
@Sacha0

Sacha0 Jun 5, 2017

Author Member

Ah sorry, it has been too long since I thought about this code! :) You are of course correct, the third argument to broadcast_c! suffices for dispatch on destination array type alone. Rather, the second argument to broadcast_c! exists to facilitate dispatch not merely on the destination array type, but also on the combination of destination array type and source types (i.e. the type returned by containertype) .

Examples that hopefully illustrates why this mechanism exists:

On the one hand, knowing that your destination array is a SparseMatrixCSC does not suffice to decide between dispatching to sparse or AbstractArray broadcast; you must also know something about the sources. If the sources are strictly sparse and/or structured and/or scalars, then you might dispatch to sparse broadcast. But if the sources include some custom array type that sparse broadcast does not support, then you must instead dispatch to AbstractArray broadcast. So you need some type information about the destination-sources combination exposed, hence the second argument in the signature above.

On the other hand, the existing design of containertype does not differentiate the destination array from the sources. But cases exist in which that differentiation is necessary. For example, sparse broadcast doesn't know how to handle structured matrix destinations yet. So were your destination array structured in the above example, you would need to dispatch to AbstractArray broadcast. But dispatch solely on the return of containertype (called on the sources, or on the combination of sources and destination array) wouldn't facilitate that. So you need both the return of containertype (called strictly on the sources, as in later commits) and the destination array type exposed.

In the future we should probably replace the existing containertype mechanism and construction above / in later commits with a function that, rather than returning an ambiguous "container type", instead returns a type identifying the particular flavor of broadcast logic to dispatch to.

Does that clarify? :)

This comment has been minimized.

Copy link
@Sacha0

Sacha0 Jun 6, 2017

Author Member

Conversation continues f997d6f#commitcomment-22403451

shape = indices(C)
@boundscheck check_broadcast_indices(shape, A, Bs...)
keeps, Idefaults = map_newindexer(shape, A, Bs)
Expand Down
11 changes: 8 additions & 3 deletions base/sparse/higherorderfns.jl
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ module HigherOrderFns
# This module provides higher order functions specialized for sparse arrays,
# particularly map[!]/broadcast[!] for SparseVectors and SparseMatrixCSCs at present.
import Base: map, map!, broadcast, broadcast!
import Base.Broadcast: containertype, promote_containertype, broadcast_indices, broadcast_c
import Base.Broadcast: containertype, promote_containertype,
broadcast_indices, broadcast_c, broadcast_c!

using Base: front, tail, to_shape
using ..SparseArrays: SparseVector, SparseMatrixCSC, AbstractSparseArray, indtype
Expand Down Expand Up @@ -852,11 +853,15 @@ promote_containertype(::Type{Tuple}, ::Type{AbstractSparseArray}) = Array
promote_containertype(::Type{AbstractSparseArray}, ::Type{Array}) = Array
promote_containertype(::Type{AbstractSparseArray}, ::Type{Tuple}) = Array

# broadcast entry point for combinations of sparse arrays and other types
function broadcast_c(f, ::Type{AbstractSparseArray}, mixedargs...)
# broadcast[!] entry points for combinations of sparse arrays and other types
@inline function broadcast_c{N}(f, ::Type{AbstractSparseArray}, mixedargs::Vararg{Any,N})
parevalf, passedargstup = capturescalars(f, mixedargs)
return broadcast(parevalf, passedargstup...)
end
@inline function broadcast_c!{N}(f, ::Type{AbstractSparseArray}, dest::SparseVecOrMat, mixedsrcargs::Vararg{Any,N})
parevalf, passedsrcargstup = capturescalars(f, mixedsrcargs)
return broadcast!(parevalf, dest, passedsrcargstup...)
end
# capturescalars takes a function (f) and a tuple of mixed sparse vectors/matrices and
# broadcast scalar arguments (mixedargs), and returns a function (parevalf) and a reduced
# argument tuple (passedargstup) containing only the sparse vectors/matrices in mixedargs
Expand Down

0 comments on commit 746dbb0

Please sign in to comment.