Repository navigation
Add @inline to unpack_args(i, ::Tuple{Any}) in ArrayPartition broadcast - #666
Conversation
The tuple-recursion base case was missing @inline, forcing a runtime dispatch whenever an ArrayPartition broadcast mixes partition eltypes (e.g. Float32 and Float64 partitions). Homogeneous partitions were unaffected since the compiler could already fold the single-method recursion. Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com> Co-Authored-By: Claude <noreply@anthropic.com> Agent-Harness: Claude Code 2.1.294 Agent-Model: claude-sonnet-5 Agent-Session: https://claude.ai/code/session_01LPHREnnonfLg1VcE1EJovv Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LPHREnnonfLg1VcE1EJovv
Independent review (Devin CLI 3000.11.3, model fusion-claude-opus-5-5-high-sidekick-swe-2-medium): MERGE, risk low. Full reviewVERDICT: MERGE PR: #666. The diff is one line, adding Blocking findingsNone. Non-blocking findings
What I ranAll runs used copies of
What I did not verify
🤖 Posted by an AI agent — harness: Devin CLI 3000.11.3 (review), Claude Code 2.1.285 (posting) · model: fusion-claude-opus-5-5-high-sidekick-swe-2-medium |
What and why
unpack_args(i, args::Tuple{Any})is the base case of the tuple-recursionthat unpacks each
ArrayPartition'si-th partition for broadcasting(
src/array_partition.jl:575). It was missing@inline. For a homogeneouspartition the compiler can still fold the one-method recursion away, but as
soon as any two partitions in the broadcast have different element types
(e.g.
Float32vsFloat64), the call is not inlined and becomes a runtimedispatch on every broadcast.
This PR adds
@inlineto that one method. No behavior change.Before/after (reused from the audit's
scratch/a1_bcfix.jl, re-measured on this branch)AP3mixed/AP5mixedhave mixedFloat32/Float64partitions;AP2ishomogeneous (unaffected, included as a control).
Julia 1.12.4 (
@benchmark, 3000 samples),a .= b + c .* dstyle in-place broadcasts:b+c*d4argnestedb+c*dnestedJulia 1.10.12, same workload:
b+c*db+c*dResults are bitwise identical before/after (same arithmetic, only inlining changes).
JET
@report_opton the mixed-eltype in-place broadcast (target_modules=(RecursiveArrayTools,)):runtime-dispatch reports go from 3 to 2 on this branch alone. The 2 remaining
reports are at the
Broadcast.Broadcasted(...)constructor calls insideunpack, not atunpack_args; they are addressed by the separateBroadcast.instantiatePR (candidate 2/3 in the audit), not by this change.Failing-before / passing-after
Added
test/Core/partitions_test.jl::"Mixed-eltype in-place broadcast does not allocate",which asserts
@allocated f!(a,b,c,d) == 0for a 3-partition mixedFloat32/Float64ArrayPartition.Before this fix (
git stashthesrc/array_partition.jlchange):After:
Test results
GROUP=Core: all 12 Core testsets pass, including the new test (Partitions Tests: 129 Pass, 0 Fail).GROUP=QA:QA: 20 Pass, 0 Fail.Both re-run on Julia 1.12.4 and 1.10.12.
What was not verified
a one-line
@inlinechange with no API surface).scratch/a1_ode.jlwas not runfor this PR specifically (no mixed-eltype RHS in that script); it is included in
PR 2, which is the change that affects that workload.
What a reviewer should push back on
Whether a one-line
@inlinewarrants its own PR versus being folded into theBroadcast.instantiatePR (candidate 2/3). They were kept separate because theyfix independent problems (dispatch at the tuple base case vs. lost SIMD from an
uninstantiated
Broadcasted) and@inlinealone does not require re-measuringthe homogeneity-check path.
Please ignore until reviewed by @ChrisRackauckas.
Risk assessment
src/array_partition.jl, one method, adds@inlineonly; nosignature, semantics, or public API change. Affects any
ArrayPartitionbroadcastwith mixed partition eltypes.
1.12.4 and 1.10.12; Core and QA test groups pass.
🤖 Generated with Claude Code
https://claude.ai/code/session_01LPHREnnonfLg1VcE1EJovv
Independent review: Devin Fusion (fusion-claude-opus-5-5-high-sidekick-swe-2-medium) rated it low, verdict MERGE: #666 (comment)