Instantiate per-partition Broadcasted in NamedArrayPartition copyto!

[ChrisRackauckas-Claude]

Conflicts with #447

#447 rewrites
src/named_array_partition.jl substantially (adds an AbstractNamedArrayPartition
type and generalizes the constructors/accessors), and its diff touches this
exact Base.copyto! method, changing dest::NamedArrayPartition to
dest::AbstractNamedArrayPartition and getfield(dest, :array_partition).x[i]
to ArrayPartition(dest).x[i]. This PR will textually conflict with #447 if
both are open; whichever lands second will need the Broadcast.instantiate(...)
wrapping re-applied to the other's version of the method. Flagging this per
the audit; not attempting to resolve it here since #447 is a larger,
independent change.

What and why

Same root cause as the ArrayPartition fix
(#670): unpack(bc, i)
drops a Broadcasted's axes, and NamedArrayPartition's copyto!
(src/named_array_partition.jl:222) never recomputed them before handing the
per-partition slice to that partition's own copyto!. On Julia 1.12 that
forces a slower, non-SIMD path. Wrapped the per-partition unpack in
Broadcast.instantiate.

Before/after (reused from the audit's scratch/a1_misc.jl)

Julia 1.12.4, @benchmark, NamedArrayPartition(a = rand(1000), b = rand(2000))
(3000 elements), in-place @. x = x * 2.0 + y:

	before	after
min	5353.3 ns	1873.0 ns
med	5876.7 ns	1880.0 ns

30-element case (a = rand(10), b = rand(20)), same op:

	before	after
min	87.3 ns	56.8 ns

Julia 1.10.12, 3000-element case:

	before	after
min	677.6 ns	632.3 ns

1.10 is close to neutral, matching the audit's note that this is a Julia
1.12-specific regression.

Results are bitwise identical. Seeded script comparing
(collect(nap.a), collect(nap.b)) after @. nap = nap * 2.0 + nap2 on a
3000-element NamedArrayPartition, serialized from the unpatched tree and
compared against this branch: identical: true.

Failing-before / passing-after

Same technique as PR #670 (neither @allocated nor JET.@report_opt
discriminates this fix for the same reason — it's a lost-SIMD timing effect,
not an allocation or dispatch effect). Added
test/Core/named_array_partition_tests.jl::"NamedArrayPartition broadcast instantiates partition Broadcasted", a deterministic structural test using a
custom partition array type that records whether the Broadcasted it
receives in its own copyto! was instantiated (axes !== nothing):

Before this fix (git stash the src/named_array_partition.jl change):

false

After:

true

Test results

GROUP=Core: all 12 Core testsets pass (NamedArrayPartition Tests: 41 Pass, 0 Fail).
GROUP=QA: QA: 20 Pass, 0 Fail — required the same :instantiate addition
to test/QA/qa.jl's ignore list as #670 (this PR branches off master
independently, so it needed its own copy of that one-line change).

Both re-run on Julia 1.12.4 and 1.10.12.

What was not verified

• Downstream, GPU, AD, and NoPre (JET) test groups were not run.
• Did not re-run the broader scratch/a1_misc.jl sublibrary benchmarks
  (RecursiveArrayToolsArrayPartitionAnyAll, RecursiveArrayToolsRaggedArrays)
  for a regression from this change specifically — they're unrelated to
  NamedArrayPartition's copyto! and were unaffected in both runs (numbers
  match within noise before/after), included above only as a byproduct of
  reusing the audit's script.

What a reviewer should push back on

As noted above, this conflicts with #447 and the two should be sequenced
(whichever merges second re-applies this one-line change to the other's
version of the method). Chris should decide the merge order.

Please ignore until reviewed by @ChrisRackauckas.

Risk assessment

• Risk: low
• Blast radius: src/named_array_partition.jl, one copyto! method; no
  signature or public API change. Affects every NamedArrayPartition
  broadcast.
• Evidence: before/after timing on Julia 1.12.4 and 1.10.12;
  bitwise-identical correctness check; Core and QA test groups pass; a
  deterministic structural regression test (same technique as Instantiate per-partition Broadcasted in ArrayPartition copy/copyto!, use type-level homogeneity check #670).
• Independent review: pending
• Merge: needs human review — conflicts with the open Adds an abstract type to NamedArrayPartition #447 and the merge
  order should be a human decision.

🤖 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: #677 (comment)

[ChrisRackauckas]

🤖 Automated comment from an AI agent running as @ChrisRackauckas — not written or reviewed by Chris.

Independent review (Devin CLI 3000.11.3, model fusion-claude-opus-5-5-high-sidekick-swe-2-medium): MERGE, risk low.

Full review

VERDICT: MERGE
RISK: low

Blocking findings

None.

Non-blocking findings

1. The out-of-place path has the same un-instantiated unpack and is left unchanged (src/named_array_partition.jl:211, copy(unpack(bc, i))). I confirmed this by running it: in bench.jl, @. x * 2.0 + y on a 3000-element NAP takes 2258 ns (min) on Julia 1.12.7 both before and after this PR, compared with 837 ns on Julia 1.10.12. At n=300000 it is 226 µs on 1.12 vs 78 µs on 1.10. So the out-of-place NAP broadcast has a 1.12 regression of about 2.7×, and this PR does not touch it. The sibling ArrayPartition fix (Instantiate per-partition Broadcasted in ArrayPartition copy/copyto!, use type-level homogeneity check #670) wraps both copy and copyto!. To match it, this method would need copy(Broadcast.instantiate(unpack(bc, i))). I have not checked that this change would actually recover the time. The PR title scopes the change to copyto!, so this can go in a follow-up, but it should be done.
2. Narrating comment in the test (test/Core/named_array_partition_tests.jl:86-88). The comment "Mirrors the same fix in ArrayPartition broadcast…" points to another change instead of saying what the test checks. The global CLAUDE.md asks for present-tense descriptions. The 2 in RATRecordingArray2 is also unnecessary: each file runs in its own @safetestset module, so the name cannot clash with RATRecordingArray from Instantiate per-partition Broadcasted in ArrayPartition copy/copyto!, use type-level homogeneity check #670. Both are cosmetic.
3. Broadcast.instantiate is not public. Base.ispublic(Base.Broadcast, :instantiate) returns false on 1.12.7. The PR adds it to the ExplicitImports ignore list in test/QA/qa.jl. I confirmed by running QA that this entry is needed: without it, QA fails only with instantiate` is not public in `Base.Broadcast. The list already holds other non-public Base.Broadcast names (flatten, result_style, …), its header comment approves exactly this case, and instantiate is the hook Base's broadcasting interface expects. I think it is acceptable.
4. Branch is one commit behind master. origin/master has Add @inline to unpack_args(i, ::Tuple{Any}) in ArrayPartition broadcast #666 on top of the merge base 02c477c. A two-dot diff therefore looks like it reverts Add @inline to unpack_args(i, ::Tuple{Any}) in ArrayPartition broadcast #666's @inline and test. The three-dot diff and git merge-tree origin/master HEAD show the merge is clean and nothing is reverted.
5. Interaction with sibling PRs. git merge-tree pr670 pr677 is clean: both add the same :instantiate line to qa.jl, and git merges identical changes without conflict. The conflict the PR body predicts with Adds an abstract type to NamedArrayPartition #447 comes from reading the diff; I did not run a merge for it. Adds an abstract type to NamedArrayPartition #447 was last updated 2025-05-15.
6. No comparable baseline numbers. The PR body's timings (5353 → 1873 ns) do not match mine, which is expected on different hardware. What does match is the direction and the 1.12-only effect (see What I ran).

What I ran

All results below are confirmed by running. I used copies of the repo in scratch/ (head = PR, before = PR with src/named_array_partition.jl reset to master, noqa = PR with qa.jl reset). The checkout itself was not modified.

• New test fails before the fix and passes after, on both Julia 1.12.7 and 1.10.12, running test/Core/named_array_partition_tests.jl:
  • before: the new testset fails 1/1 on all(x -> x.instantiated, ArrayPartition(nap).x).
  • head: 21/21, 19/19 and 1/1 pass.
  • The test is structural (bc.axes !== nothing at the partition's copyto!). It can fail and does, so it really detects whether the fix is present.
• QA (GROUP=QA, 1.12): head passes 20/20. noqa has 19 passing and 1 erroring, and the only error is ExplicitImports flagging instantiate.
• Behavior probes (scratch/probe.jl): 20 broadcast cases on before and head, on both Julia versions, compared with isequal on the serialized results (scratch/compare.jl). All 20 are identical on both versions, including the error type and message where a case throws. Cases covered: in-place muladd, scalar fill, Ref, nested functions, identity copy, .+=, Int dest with Float source, matrix partitions, MVector partitions, SVector partitions (error, unchanged), nested ArrayPartition partition, uniform and ragged VectorOfArray partition, shape mismatch (DimensionMismatch, unchanged), length-1 partitions, NAP .+ ArrayPartition, out-of-place, empty partition. @allocated of the in-place op is 0 before and after on both versions. The mixed-eltype NAP case fails at construction on both sides because of an existing NAP assertion, so it is not a regression. This confirms the PR body's "bitwise identical" claim.
• Benchmarks (scratch/bench.jl, BenchmarkTools, run one at a time; the 1.12 pair was run twice). In-place @. x = x * 2.0 + y, min ns, before → after:
  • 1.12, n=30: 33.3 → 29.1 (run 2: 37.5 → 29.1).
  • 1.12, n=3000: 2258 → 1308 (run 2: 2275 → 1308).
  • 1.12, n=300000: 278.9 µs → 158.1 µs (run 2: 276.5 → 153.4).
  • 1.10: no change at any size (n=3000: 1300 → 1304).
  • On 1.12 after the fix, NAP in-place (1308 ns) matches a plain Vector (1271 ns) at n=3000.
• git merge-tree checks against master and against Instantiate per-partition Broadcasted in ArrayPartition copy/copyto!, use type-level homogeneity check #670 (results in non-blocking findings 4 and 5).

What I did not verify

• The rest of the Core group (only the NamedArrayPartition test file was run), the Downstream, GPU, AD and NoPre (JET) groups, and the sublibrary tests.
• Runic formatting and typos on the diff.
• Whether wrapping the copy path (non-blocking finding 1) recovers the out-of-place time; I believe it would only from reading the code.
• The actual merge conflict with Adds an abstract type to NamedArrayPartition #447 (predicted from reading the diff only).
• There is no AGENTS.md or CLAUDE.md at the repo root; I applied the global ~/.claude/CLAUDE.md rules instead.

---

🤖 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
Conversation: local Claude session 3c311569-dda6-4687-bc9f-65febb212c33 (fleet master)