Fix multiplication, division between sparse and scalar #14973
Conversation
for corner cases when the scalar is zero, Inf, or NaN Returning dense data for sparse input is potentially really bad for performance, but necessary to satisfy full(op(A, B)) == op(full(A), full(B))
|
This seems to have languished without review---we humans are woefully inadequate, and we need a bot that has the admirable attention to detail of, well, @tkelman to make sure that His Pull Requests Are Not Neglected. 👍 to this as a general strategy. Detailed comments coming. |
| if isfinite(B) | ||
| SparseMatrixCSC(A.m, A.n, copy(A.colptr), copy(A.rowval), A.nzval .* B) | ||
| else | ||
| densify_with_default(A, A.nzval .* B, zero(eltype(A)) .* B) |
There was a problem hiding this comment.
At least theoretically, now that we have fast anonymous functions you could pass B and (x,y) ->x .* y, and thus skip the intermediate storage for A.nzval .* B.
|
LGTM. |
|
Most of the discussion here happened in #14963, then I got distracted by other things and didn't finish my todo's. Will revisit and try your suggestion. |
|
@tkelman, is this still on your horizon? If not, would you appreciate someone else taking up the torch? Best! |
|
Yes, and yes. Haven't had time to revisit this but would be quite pleased if you want to do so. |
|
|
|
This works fine now. We return a fully filled sparse matrix with the right entries. |
for corner cases when the scalar is zero, Inf, or NaN
Returning dense data for sparse input is potentially really bad for
performance, but necessary to satisfy
full(op(A, B)) == op(full(A), full(B))todo:
scaleandscale!SparseVector, ref fix #14111 dividing sparse vector by constant should give sparse vector #14963