Skip to content

Commit

Permalink
Extend sparse broadcast! to structured matrices by promoting structur…
Browse files Browse the repository at this point in the history 
…ed matrices to sparse.
  • Loading branch information
@Sacha0
Sacha0 committed Jan 10, 2017
1 parent a630a2f commit f997d6f
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 4 deletions.
4 changes: 2 additions & 2 deletions base/broadcast.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -203,8 +203,8 @@ 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 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})
broadcast_c!(f, containertype(C), containertype(A, Bs...), C, A, Bs...)
@inline function broadcast_c!{N}(f, ::Type, ::Type, C, A, Bs::Vararg{Any,N})

This comment has been minimized.

Sign in to view

Copy link
@andreasnoack

andreasnoack Jun 5, 2017

Member

@Sacha0 Sorry, I still don't get it but I should probably have asked here instead of in the older commit. I can see the you'd been joint type information for C, A, and Bs to dispatch to the right routine but it seems to me that you could just dispatch on C and a single of the ::Type arguments instead ignoring typeof(C) and dispatching on two ::Type arguments. Does it make sense or should I read you last comment again?

This comment has been minimized.

Sign in to view

Copy link
@Sacha0

Sacha0 Jun 5, 2017

Author Member

Cheers, I think I follow you now. To check: Formerly we dispatched on containertype(C, A, Bs...) (second arg) and typeof(C) (third arg), but we needed also containertype(A, Bs...) as discussed in my preceding response. So we switched to dispatching on containertype(C) (second arg) and containertype(A, Bs...) (third arg) (while already having typeof(C) (fourth arg) available). IIUC, you are asking why dispatch on containertype(C) and containertype(A, Bs...) (introducing an argument for containertype(C)) rather than containertype(A, Bs...) and typeof(C) (avoiding introducing an argument for containertype(C))?

This comment has been minimized.

Sign in to view

Copy link
@andreasnoack

andreasnoack Jun 5, 2017 •
edited
Loading

Member

Yes exactly, I'd think that containertype(A, Bs...) and typeof(C) would be sufficient.

This comment has been minimized.

Sign in to view

Copy link
@Sacha0

Sacha0 Jun 7, 2017

Author Member

In principle containertype(A, Bs...) and typeof(C) should be sufficient (containertype(C) being derived from typeof(C)). But containertype(C) is not strictly typeof(C), and (at least at the time) all relevant definitions were in containertype(C). That is to say, having containertype(C) directly exposed (at least) made life simpler. IIRC it was also necessary given the existing infrastructure, though I cannot recall precisely why. (I have some fleeting memory, for example, that destination array C wasn't consistently exposed as an argument separate from the source arguments, necessitating exposure of containertype(C) in upstream methods. But I may be misremembering, and of course the preceding isn't an issue in principle.)

This code absolutely can and should be much better architected :).

(Continuation of conversation from 746dbb0#commitcomment-22399775)
shape = indices(C)
@boundscheck check_broadcast_indices(shape, A, Bs...)
keeps, Idefaults = map_newindexer(shape, A, Bs)
Expand Down
9 changes: 7 additions & 2 deletions base/sparse/higherorderfns.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -854,7 +854,7 @@ promote_containertype(::Type{Tuple}, ::Type{AbstractSparseArray}) = Array
promote_containertype(::Type{AbstractSparseArray}, ::Type{Array}) = Array
promote_containertype(::Type{AbstractSparseArray}, ::Type{Tuple}) = Array

# broadcast[!] entry points for combinations of sparse arrays and other types
# broadcast[!] entry points for combinations of sparse arrays and other (scalar) types
@inline function broadcast_c{N}(f, ::Type{AbstractSparseArray}, mixedargs::Vararg{Any,N})
parevalf, passedargstup = capturescalars(f, mixedargs)
return broadcast(parevalf, passedargstup...)
Expand Down Expand Up @@ -914,7 +914,12 @@ promote_containertype(::Type{Tuple}, ::Type{StructuredArray}) = Array

# for combinations involving sparse/structured arrays and scalars only,
# promote all structured arguments to sparse and then rebroadcast
broadcast_c{N,Tf}(f::Tf, ::Type{StructuredArray}, As::Vararg{Any,N}) = broadcast(f, map(_sparsifystructured, As)...)
@inline broadcast_c{N}(f, ::Type{StructuredArray}, As::Vararg{Any,N}) =
broadcast(f, map(_sparsifystructured, As)...)
@inline broadcast_c!{N}(f, ::Type{AbstractSparseArray}, ::Type{StructuredArray}, C, B, As::Vararg{Any,N}) =
broadcast!(f, C, _sparsifystructured(B), map(_sparsifystructured, As)...)
@inline broadcast_c!{N}(f, CT::Type, ::Type{StructuredArray}, C, B, As::Vararg{Any,N}) =
broadcast_c!(f, CT, Array, C, B, As...)
@inline _sparsifystructured(S::SymTridiagonal) = SparseMatrixCSC(S)
@inline _sparsifystructured(T::Tridiagonal) = SparseMatrixCSC(T)
@inline _sparsifystructured(B::Bidiagonal) = SparseMatrixCSC(B)
Expand Down

0 comments on commit f997d6f

Please sign in to comment.