replace broadcast with map in sparse binary op definitions

[Sacha0]

A few elementwise operations over pairs of sparse matrices (e.g. +, min) are implemented as short children of broadcast preceded by a (same-)shape check. This pull request reimplements those operations in terms of map. Best!

Note: This pull request should not pass CI prior to rebasing on #19518.

[Sacha0]

Rebased with #19518 in. Should be in shape if CI approves. Best!

[Sacha0]

Absent objections or requests for time, I plan to merge this PR Sunday morning PST. Best!

[tkelman]

are mismatched-shape cases covered by existing tests?

[Sacha0]

are mismatched-shape cases covered by existing tests?

The checks/throws in map and broadcast are. But from a brief look at test/sparse/sparse.jl, those checks/throws are not exercised via the operations touched in this pull request. Updating with tests. Thanks!

[Sacha0]

Now with tests of shape checks. Thanks!

[tkelman]

lgtm. the sparse broadcast tests have some loops that go through the special cases for 1-by-n and n-by-1, right? those might also be good places to throw in some test_throws to make sure we don't unintentionally change anything unexpected

[Sacha0]

the sparse broadcast tests have some loops that go through the special cases for 1-by-n and n-by-1, right? those might also be good places to throw in some test_throws to make sure we don't unintentionally change anything unexpected

Do you mean adding tests along the lines of these tests for the generic code to e.g. these tests for the binary-specialized code? If so, absolutely, though in a separate PR? Thanks!

[tkelman]

sure, as long as it doesn't get forgotten. if these operations mistakenly went through the broadcast code path I don't think mismatched size square inputs would be enough to tell the difference between broadcast and map, since both would throw an error.

[Sacha0]

Ah, I see your concern now. Will update with sharper tests. Thanks!

[Sacha0]

The revised tests should differentiate between implementing these operations as map versus broadcast. (map should throw and pass, broadcast should not.)

Realization re. the more general sparse broadcast shape check testing discussion above: I condensed the entry points for sparse broadcast into two methods (in which lie the shape checks) in #19518. The tests for the generic code linked above exercise those methods' shape checks, thereby covering the shape check code for all sparse broadcast cases. Sufficient, or should we do more? Thanks!

[tkelman]

mac travis freeze unrelated, the added tests passed there

[Sacha0]

Thanks for reviewing / merging!