Repository navigation
Invalidations 2/6: fuse nested broadcasts without broadcastable - #339
Draft
asinghvi17 wants to merge 2 commits into
Draft
asinghvi17 wants to merge 2 commits into
asinghvi17 wants to merge 2 commits into
Conversation
Remove Base.broadcastable(::BroadcastDiskArray). It replaced a lazy disk
broadcast by its Broadcasted wherever it was broadcast again, so flatten could
fuse the expression, but a method on broadcastable invalidates compiled
broadcast code that calls broadcastable on abstractly inferred arguments.
DiskArrays now does the same unwrapping inside its own ChunkStyle copy/copyto!
methods (new internal helper unwrap_broadcast), and before handing a
Broadcasted{Nothing} to the diskarrays_coptyo! backend hook. Values, chunk
reads and allocations of nested broadcasts are unchanged.
Backends that return their own style from diskarrays_broadcaststyle can call
unwrap_broadcast in their copy/copyto! methods; document this in backend.md.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019Dfq6aDr3GqA8w61TsvMsc
Check that s ./ a3, with s = a1 .+ a2 .* 2 lazy, has the leaf arrays as arguments, gives the same values, reads each chunk of each leaf once, and allocates no more than the same expression written at once. Also check unwrap_broadcast with a foreign style and the diskarrays_coptyo! hook. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019Dfq6aDr3GqA8w61TsvMsc
asinghvi17
added this pull request to stack #342
October 8, 2026 14:03
asinghvi17
changed the title
broadcastable method
asinghvi17
changed the title
broadcastable methodbroadcastable
asinghvi17
changed the title
broadcastablebroadcastable
mkitti
reviewed
Oct 9, 2026
mkitti
left a comment
mkitti
left a comment
Member
There was a problem hiding this comment.
Looks good to me. I checked out the branch: the full suite passes on Julia 1.13.
Verification. I ran 14 broadcast cases on #338 and on this PR and compared them: s .+ s, 3-deep chains, vector/row/Ref operands, ifelse. mixes, sum(s .* 2), indexing/view/adjoint of a lazy broadcast, and dest .= s .* b into an Array and into a disk array. Results are identical in every case, including the number of reads of the leaf array (2 for a shared subexpression, both before and after). Base.broadcastable(s) === s is now true, as intended.
Comments
Broadcast.flattenis a visible change. ForBroadcast.broadcasted(+, s, 1)with a lazys,flatten(bc).argshad 3 entries before and has 2 now (the lazy array is an opaque leaf). Nothing is wrong inside DiskArrays, andbackend.mddocumentsunwrap_broadcastfor backends. Still, any downstream package that callsflattenon user expressions (not only backends with their own style) loses fusion until it callsunwrap_broadcast. A CHANGELOG entry saying so would help (there is none in this PR).- Add a direct guard for the invalidation. The new tests pin the fusion behavior, and they would also pass with
Base.broadcastable(::BroadcastDiskArray)put back, so nothing stops the invalidating method from returning. Something like@test !any(m -> m.module === DiskArrays, methods(Base.broadcastable))would. I use the same pattern forBase.Generatorin #345. - Brittle assertions.
length(bc.args) == 4inisfuseddepends on the exact shapeflattenproduces, and@allocated nested <= @allocated directcan differ across Julia versions (nightly) or with inlining. Fine today; if CI flakes, those are the first suspects. - As the description says, on 1.13 this method forms no invalidation tree (my 1.13 numbers for #338 → #339 are unchanged), so the benefit is on 1.12. That is a reasonable justification given there is no behavior change.
Not blocking.
This branch has not been deployed
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
2 participants
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.
Part 2/6, on #338.
Removes
Base.broadcastable(::BroadcastDiskArray). Base callsbroadcastableon every broadcast argument, so this method invalidated compiled broadcast code. Nested lazy broadcasts still fuse: DiskArrays' owncopy/copyto!now unwrap them with a new internalDiskArrays.unwrap_broadcast. A backend with its own broadcast style should call it too (docs/src/backend.md).Values, reads and allocations match #316, and a new test guards the fusion.
uinvalidatedafterusing DiskArrays(#338 → this PR):On 1.13 this method forms no tree, so only the 1.12 column moves.
🤖 Generated with Claude Code
https://claude.ai/code/session_019Dfq6aDr3GqA8w61TsvMsc