Repository navigation
Do not densify the sparse arguments of a broadcast + or - whose result is empty - #1007
Merged
Merged
Conversation
…esult is empty Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1007 +/- ##
=======================================
Coverage 95.59% 95.59%
=======================================
Files 16 16
Lines 9304 9304
=======================================
Hits 8894 8894
Misses 410 410 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes a regression from #1004, found in review: a broadcast
+or-of a sparse array with a dense array converted the sparse arguments toArrays before looking at the shape of the result, so an empty result cost memory proportional to the size of the sparse argument, and could run out of it.Before
After
_densebroadcastnow hands the broadcast to the dense kernel as it is when one of its axes is empty: that kernel reads no entry, so the reason to densify (generic broadcast indexing the sparse arrays entry by entry) does not apply. The result type is the same on both paths and stays inferable.Two assertions that #1004 and #1005 added are removed as duplicates of ones in the older loops of
test/higherorderfns.jl:broadcast(+, S, d)(the sums of a sparse matrix with a dense vector in the mixed-argument loop) andcos.(view(S, :, [2, 1]))(the loop over views of sparse arrays, which has a view of columns picked by a vector).Not a backport candidate: #1004 is on no release branch.
Written by Claude Code.
🤖 Generated with Claude Code