Skip to content

Add simplify_broadcasted as the supported downstream entry point - #444

Open
jishnub wants to merge 1 commit into
jishnub/broadcaststylefrom
jishnub/broadcaststyle-v2
Open

Add simplify_broadcasted as the supported downstream entry point#444
jishnub wants to merge 1 commit into
jishnub/broadcaststylefrom
jishnub/broadcaststyle-v2

Conversation

@jishnub

@jishnub jishnub commented Aug 10, 2026

Copy link
Copy Markdown
Member

Stacked on #385 — review that first; this PR's diff is the single commit on top.

The problem

A package with a BroadcastStyle of its own currently reaches the fill rules by forwarding through broadcast(DefaultArrayStyle{N}(), op, args...). That protocol is undocumented and leaks implementation details three ways:

  1. It names a style that has nothing to do with fills — it is merely where the rules used to be attached.
  2. It makes the caller enumerate the argument shapes that happen to have rules. Miss one and the simplification is silently lost; forward one we don't handle and the result materializes densely. LazyArrays documents this hazard in a comment.
  3. It requires knowing that x .^ k lowers to a three-argument Base.literal_pow with Ref arguments, and that our rule takes them unwrapped. Both LazyArrays and InfiniteArrays carry near-duplicate methods for this alone.

Packages also call internals directly: _broadcasted_zeros (BlockArrays, BandedMatrices), mult_zeros (ArrayLayouts, LazyArrays, QuasiArrays), elconvert, _range_convert, steprangelen.

The change

FillArrays.simplify_broadcasted(style, op, args...)

Returns the simplified array where a rule applies, and otherwise a Broadcasted carrying the caller's own style — so anything we cannot simplify comes back for the caller to keep lazy, infinite, or on-device. Ref-wrapped literal_pow arguments are unwrapped, so callers never mention it.

This is the existing _dispatch_on_fills with two changes: the fallback style is parameterized instead of hard-coded to DefaultArrayStyle{N}, and the Ref-unwrapping folds in. The dimension now comes from the arguments rather than the style, so an AbstractArrayStyle{Any} caller works too.

Resolution order is unchanged, and for the same reason as before: the Zeros-absorbing rules are styleless and would otherwise be preempted by the more specific FillStyle range rules.

  1. has_fill_rule → styleless rule
  2. broadcasted(FillStyle{N}(), …) → fill-style rule
  3. isfill → evaluate on the fill values
  4. Broadcasted{typeof(style)}

The DefaultArrayStyle shims now delegate here, leaving one implementation rather than two. They are kept for packages that predate this.

Verification

  • A 3592-probe sweep of broadcasted(DefaultArrayStyle{N}(), op, args...) is byte-identical before and after, so unmigrated packages are unaffected.
  • Full suite passes; Broadcast testset 1524 → 1533 with the new tests, covering all four resolution branches including that the fallback carries the caller's style.

Downstream, against this branch: LazyArrays 35 broadcast methods → 14, InfiniteArrays' 4 redundant literal_pow methods → 0, BlockArrays and BandedMatrices 4 → 3 each with _broadcasted_zeros dropped. All four suites pass, and a 16-probe old-vs-new diff against their real checkouts shows no behavioural change.

Versioning

This is a minor release (1.19.0): simplify_broadcasted is new, the DefaultArrayStyle
shims are untouched in behaviour, and the 3592-probe sweep over them is byte-identical. Every
published downstream keeps resolving — LazyArrays and InfiniteArrays pin FillArrays = "1.0",
BlockArrays "1.11", BandedMatrices "1.3", and all of those admit 1.19. Packages opt into the
new entry point by raising their lower bound to "1.19" when they are ready; nothing is forced.

Deleting the DefaultArrayStyle shims (src/fillbroadcast.jl:245-254) is the eventual endpoint of
this migration and is genuinely breaking. That is deliberately left for a future major release,
once downstream packages have moved over.

🤖 Generated with Claude Code

@codecov

codecov Bot commented Aug 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 88.88889% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 93.63%. Comparing base (b7eeafb) to head (5e01e70).

Files with missing lines Patch % Lines
src/fillbroadcast.jl 88.88% 1 Missing ⚠️
Additional details and impacted files
@@                    Coverage Diff                     @@
##           jishnub/broadcaststyle     #444      +/-   ##
==========================================================
+ Coverage                   93.60%   93.63%   +0.03%     
==========================================================
  Files                           9        9              
  Lines                        1298     1305       +7     
==========================================================
+ Hits                         1215     1222       +7     
  Misses                         83       83              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Packages with a `BroadcastStyle` of their own currently reach the fill rules by
forwarding through `broadcast(DefaultArrayStyle{N}(), op, args...)`. That protocol
is undocumented and leaks implementation details: it names a style unrelated to
fills, it makes the caller enumerate the argument shapes that happen to have rules,
and it requires knowing that `x .^ k` lowers to a three-argument `literal_pow` with
`Ref`s.

`simplify_broadcasted(style, op, args...)` replaces it. It returns the simplified
array where a rule applies, and otherwise a `Broadcasted` carrying the caller's own
style, so anything unsimplified stays lazy. The dimension comes from the arguments
rather than the style, so an `AbstractArrayStyle{Any}` caller works too.

The resolution order is unchanged, and for the same reason as before: the
`Zeros`-absorbing rules are styleless and would otherwise be preempted by the more
specific `FillStyle` range rules. The `DefaultArrayStyle` shims now delegate here,
leaving one implementation rather than two, and a 3592-probe sweep of that path is
byte-identical to before.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jishnub
jishnub force-pushed the jishnub/broadcaststyle-v2 branch from 8a50f77 to 5e01e70 Compare August 12, 2026 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant