Fix, clean up, and generify broadcast[!] over pairs of sparse matrices #19371
Conversation
Sacha0
added
sparse
| ## Broadcast operations involving two sparse matrices | ||
| function broadcast{Tf}(f::Tf, A::SparseMatrixCSC, B::SparseMatrixCSC) | ||
| indextypeC = promote_type(eltype(A.rowval), eltype(B.rowval)) | ||
| entrytypeC = Base.Broadcast.promote_eltype_op(f, A, B) |
There was a problem hiding this comment.
The Base. prefix shouldn't be necessary, right?
There was a problem hiding this comment.
promote_eltype_op is not exported, necessitating the Base.Broadcast. qualification? Thanks!
There was a problem hiding this comment.
Right, you need Broadcast but since this is part of Base you shouldn't need Base, I think.
There was a problem hiding this comment.
This file isn't part of Base.
There was a problem hiding this comment.
Okay, my mistake. Sorry for the noise.
There was a problem hiding this comment.
promote_eltype_op is defined in abstractarray.jl in Base so you can drop the Broadcast
There was a problem hiding this comment.
Nixed the qualification's Broadcast part. Thanks!
|
Fixed / improved a few lines with benefit of what I learned from #19438. Absent objections or requests for time, I plan to merge this Thursday morning PST. Best! |
…here the broadcast function does not return zero when both arguments are zero.
|
Rebased. Planning to merge once CI clears. Best! |
This pull request fixes (#19110), cleans up, and slightly generifies
broadcast[!]over pairs of sparse matrices.This pull request does not fundamentally change existing semantics or implementation. Apart from the fix for #19110, changes that reduce cognitive load for wetware interpreters (removal of meta-fu, transformation of variable names and expressions to more enlightening alternatives, insertion of comments) constitute most of the diff. About half the additions derive from the #19110 fix, which unfortunately duplicates part of the existing implementation. (The bulk of that duplication could go away in exchange for some efficiency (both time and memory).)
This pull request represents a first step, not where the code should land. The next step requires settling
map!/broadcast!'s semantics for sparse matrices, and steps thereafter require determiningbroadcast_zpreserving!'s fate. I will open issues to hash those design questions out.Best!