Skip to content

Add comprehension fast path for A[ints..., cols] indexing - #673

Draft
ChrisRackauckas-Claude wants to merge 4 commits into
SciML:masterfrom
ChrisRackauckas-Claude:voa-colindex-fastpath
Draft

ChrisRackauckas-Claude wants to merge 4 commits into
SciML:masterfrom
ChrisRackauckas-Claude:voa-colindex-fastpath

Conversation

@ChrisRackauckas-Claude

@ChrisRackauckas-Claude ChrisRackauckas-Claude commented Oct 8, 2026 •

Copy link
Copy Markdown
Member

What changed and why

_getindex's general multi-index method (src/vector_of_array.jl, used by both AbstractVectorOfArray and AbstractDiffEqArray) handled A[i, cols] — a scalar prefix index selecting many columns, the common sol[2, :] pattern — via stack(getindex.(A.u[col_idxs], tuple.(front)...)). For a scalar-Int prefix this is doing far more work than the result needs: it copies the selected columns into a new Vector (A.u[col_idxs]), wraps each prefix index in a 1-tuple and broadcasts getindex over it, then calls stack — to produce what is just a plain Vector of scalars.

Added a fast path to both the AbstractVectorOfArray and AbstractDiffEqArray _getindex methods: when every prefix index is a scalar Int, build the result as a direct comprehension over A.u (or view(A.u, col_idxs) when col_idxs isn't Colon) — [u[front...] for u in cols]. This skips both the outer-vector copy and the tuple-broadcast machinery. Falls back to the existing stack-based path for anything else. For AbstractDiffEqArray, the pre-existing all-Colon-prefix branch (which must return a DiffEqArray, not a plain Vector) is checked first, so it still takes priority.

Before/after

n = 1000, 5-element inner Vector{Float64}:

                         before                          after
VoA  A[2, :]   4101 ns / 9 allocs / 24248 B     2113 ns / 3 allocs / 8072 B
DEA  D[2, :]   4733 ns / 9 allocs                2480 ns / 3 allocs

(the 8072-byte "after" figure is essentially just the 1000-element Float64 output vector — 8000 bytes of data plus a small header)

n = 25, A[1, 1:10] / D[1, 1:10]: 82 ns / 5 allocs -> 33 ns / 2 allocs for both VectorOfArray and DiffEqArray.

Correctness, including a real ODE solution (per the task's request to check this through OrdinaryDiffEqTsit5):

using OrdinaryDiffEqTsit5
f(du, u, p, t) = (du .= -u)
prob = ODEProblem(f, [1.0, 2.0, 3.0], (0.0, 1.0))
sol = solve(prob, Tsit5())
sol[2, :] == [u[2] for u in sol.u]      # true, identical on master and this branch
sol[1, 1:5] == [u[1] for u in sol.u[1:5]]  # true, identical on master and this branch

Ragged semantics are unaffected (verified against VectorOfArray([[1.0, 2.0], [3.0], [4.0, 5.0, 6.0]])):

  • R[2, :] -> BoundsError, same before and after (unchanged — this candidate only touches the all-scalar-prefix case, and R[i, :] has a Colon prefix)
  • R[1, 1:3] == [1.0, 3.0, 4.0], same before and after (the fast path applies here — front = (1,) is all-Int — and gives the identical result, since index 1 is in-bounds for every selected inner array)
  • R[1, 1] == 1.0, R[3, 2] == 0.0 (zero-padded getindex, a different method entirely — untouched)

Mixed Int/Colon prefixes (outside the fast path, e.g. B[1, :, 2:3], B[:, 2, 2:3]) still take the unmodified stack-based fallback — verified identical before/after.

Failing-before / passing-after

Added test/Core/basic_indexing.jl::"A[i, cols]/A[i, :] scalar-row indexing is O(1) per column, ragged unaffected", which checks both correctness and an allocation bound (n Float64s is 8000 bytes; the old path allocated ~24 KB for the same n=1000, so < 10_000 cleanly separates them without depending on timing):

Before (reverted src/vector_of_array.jl to unmodified master, kept the new test):

Test Summary:                                                                | Pass  Fail  Total  Time
A[i, cols]/A[i, :] scalar-row indexing is O(1) per column, ragged unaffected |   10     2    12   3.5s

(both failures are the @allocated get_row(A, 2) < 10_000 / @allocated get_row(D, 2) < 10_000 assertions — the VectorOfArray and DiffEqArray cases)

After (this branch): 12/12 pass, on both Julia 1.10.12 and 1.12.7.

Full GROUP=Core run (both Julia versions): all testsets green, including the existing A[i, cols]/ragged/mixed-prefix coverage already in basic_indexing.jl and interface_tests.jl. GROUP=QA on 1.12.7: 20/20 pass. GROUP=QA on 1.10.12 hits the same pre-existing, unrelated Aqua ambiguity failure documented in #667 (confirmed present on unmodified master).

What was not verified

  • Downstream, GPU, NoPre and AD groups were not run as a full suite (only the one-off OrdinaryDiffEqTsit5 check above, done manually in a scratch environment since this repo's Downstream test env wasn't set up for this check). GPU-backed inner arrays were not benchmarked.

Anything a reviewer should push back on

  • Nothing behavior-changing: every fast-path output was checked equal to the pre-existing fallback's result (plain correctness, ragged, mixed-prefix, and a real ODE solution), not just benchmarked. (Superseded: see "Review findings addressed (second review)" below.)

Please ignore until reviewed by @ChrisRackauckas.

Risk assessment

  • Risk: low
  • Blast radius: src/vector_of_array.jl, the shared _getindex methods for AbstractVectorOfArray and AbstractDiffEqArray — a hot, widely-used indexing path (sol[i, :] is extremely common in SciML code), but the change is a pure fast-path addition with no behavior change for any input, verified directly. (Superseded: see "Review findings addressed (second review)" below.)
  • Evidence: failing-before/passing-after shown above; before/after timing and allocation counts; a real OrdinaryDiffEqTsit5 solution's sol[2, :]/sol[1, 1:5] verified identical to master; GROUP=Core full pass on Julia 1.10 and 1.12.
  • Independent review: pending
  • Merge: needs human review — touches a very hot, widely-used indexing path across the SciML ecosystem (sol[i, :]), so even with no observed behavior change (Superseded: see "Review findings addressed (second review)" below.) it warrants a closer look before merge rather than auto-merging.

🤖 Generated with Claude Code

https://claude.ai/code/session_01LPHREnnonfLg1VcE1EJovv

Review findings addressed

Two reviews are covered here: the Devin review (fusion-claude-opus-5-5-high-sidekick-swe-2-medium) posted as a comment on this PR, and a Grok Build grok-4.7 review of 2ca7e98.

The fast path now also requires a concrete Number element type in the inner arrays (eltype(eltype(A.u))). For those element types, collecting u[front...] gives the same type, shape and values as the stack fallback. The check is resolved at compile time, and the speedup is unchanged (n = 1000, 5-element inner vectors, Julia 1.13, BenchmarkTools median): A[2, :] takes 4355 ns / 8 allocs on master, 1847 ns / 3 allocs on 2ca7e98, and 1831 ns / 3 allocs with this commit. D[2, :], B[1, 2, :] and A[1, 1:10] show the same pattern.

  • Undisclosed result changes for array-valued, abstract and non-numeric element types (Devin, blocking 1). Fixed by the element-type check, so nothing needed disclosing. Inner arrays of SVectors or Vectors (both VectorOfArray and DiffEqArray), of Real or Any, and of String, Symbol or Union{Missing, Float64} go through stack again and match master exactly. Master returns a 3×3 Matrix for S[2, :] on a (2, 3) array of SVectors, which does not match size(S, 2). Fixing that changes the result type of sol[i, :], so it needs its own, breaking PR and is left alone here. New tests compare X[2, :] and X[2, 1:2] (type and value) against stack(getindex.(X.u, 2)) for four of those element types. They fail on 2ca7e98 (8 failures) and pass here.
  • Empty column selections (Grok, finding 1; also listed by Devin). The new result is kept. With a numeric element type, A[i, 1:0], A[i, Int[]], A[i, falses(n)], the same three on a DiffEqArray, and B[i, j, 1:0] return an empty Vector{T}. On master they throw ArgumentError: `stack` on an empty collection is not allowed. This is the one input class whose result differs from master, so the claims above of "no behavior change for any input" and "Nothing behavior-changing" are wrong for it. Empty selections with a non-numeric element type still throw, as on master. New tests cover it; on master they give 7 errors.
  • Source comments said the result is "a plain Vector of scalars" (Devin, non-blocking 1). Both comments are removed. One comment on the shared _is_numeric_row predicate now says when the fast path matches stack.
  • Comment density and inaccurate test comments (Devin, non-blocking 2). The block comments are removed, and the warm-up comment now reads # compile before measuring. Of the 85 lines added against master, 12 are full-line comments, and 6 of those are pre-existing DiffEqArray comments that moved with the restructured method.
  • Test name (Devin, non-blocking 3). Renamed to "A[i, cols] with scalar prefix indices".
  • Overlap with Add O(1) fast path to AbstractVectorOfArray getindex #668 and Add fast path to stack(::AbstractVectorOfArray) for numeric inner arrays #671 (Devin, non-blocking 4; Grok, finding 3). No code change. The two PRs edit different methods. Add O(1) fast path to AbstractVectorOfArray getindex #668 also appends to the end of test/Core/basic_indexing.jl, so whichever merges second gets a small text conflict there.
  • OffsetArray inner arrays (Devin, non-blocking 5). This behavior predates the PR: OI[0, :] gives the same result on master and here. Not changed.
  • No linked issue (Devin, non-blocking 6). There is no issue for this PR. No change.
  • CI pending (Grok, finding 2). A code change can't address this. CI runs on the new head.

Verification for this commit

  • Behavior probe: 51 indexing cases, run on master, on 2ca7e98 and on this commit (Julia 1.13). The cases cover numeric VectorOfArray/DiffEqArray with range, vector, mask, matrix, empty and out-of-bounds column selections; matrix, nested-VectorOfArray, ArrayPartition, StructArray, OffsetArray and ragged inner arrays; mixed-prefix slices; and SVector, Vector, Any, Real, String, Symbol, Char, Missing, Int, Bool, Complex, BigFloat and Rational elements. Compared with master, this commit differs only on the 7 empty-selection cases with a numeric element type, where it returns an empty vector instead of the stack ArgumentError. 2ca7e98 additionally differed on S[2, :], S[2, 1:2], S[2, 1:0], SD[2, :], VV[2, :], Any/Real elements, String, Symbol and Missing.
  • New testset, run on its own:
    • master: Pass 18, Fail 2, Error 7. The failures are @allocated get_row(A, 2) < 10_000 evaluating to 24184 < 10000, and the errors are ArgumentError: `stack` on an empty collection is not allowed.
    • 2ca7e98: Pass 19, Fail 8, for example same(SVector{3, Float64}[[5.0, 6.0, 7.0], …], [5.0 6.0 7.0; …]).
    • this commit: Pass 27, Total 27.
    • The same 27/27 on Julia 1.10.12.
  • GROUP=Core, Julia 1.13.0 and 1.10.12: Testing RecursiveArrayTools tests passed. VecOfArr Indexing Tests shows 207 Pass, 2 Broken; both broken tests are pre-existing @test_broken that are also on master.
  • Real ODE solutions (OrdinaryDiffEqTsit5 v2.1.5, SciMLBase v3.58.0) on master and on this commit, compared by type and isequal. sol[2, :], sol[1, 1:5] and sol[3, [1, 2]] (vector state), sols[2, :] (SVector state) and solm[1, 2, :] and solm[2, 1:3] (matrix state) are identical. sol[1, 1:0] returns Float64[] here, where master throws. A Vector{SVector} state cannot be solved at all (Non-Number element type inside of an Array detected), so ODE solutions never take the array-valued path.
  • GROUP=QA, Julia 1.13.0: QA | 20 20.
  • Runic (--check) and typos report nothing on both changed files.

Not verified

Downstream, NoPre, AD, GPU, Core 32-bit and Julia pre were not run locally.


🤖 Posted by an AI agent — harness: Claude Code 2.1.301 · model: claude-opus-5-5
Conversation: local session ID 62cad038-fd69-43d9-bdf5-5d147b550253 (headless, no URL)

Review findings addressed (second review)

Codex CLI (gpt-6.1-sol) re-reviewed eccc962 and rated it medium.

  • Matrix-shaped outer storage returned a matrix for A[i, :] (blocking). Fixed. VectorOfArray accepts any linearly indexed outer array (VectorOfArray(reshape(us, 2, 2))), and A.u[:] in the stack path flattens it. The fast path iterated A.u directly, so M[2, :] returned a 2×2 Matrix and M3[1, 2, :] returned a 2×2×2 Array. The Colon branch now iterates vec(A.u), which on a Vector returns the same object. Range, vector, mask and matrix column indices already went through a linear view and were unaffected. New tests: M[2, :], M[2, 2:3], and M3[1, 2, :] on a 3-D outer array of matrices. Two of them fail on eccc962 (same([12.0 32.0; 22.0 42.0], [12.0, 22.0, 32.0, 42.0])), and all three pass on master and here.
  • Contradictory PR description (non-blocking). The three earlier sentences claiming no behavior change are struck through above and point here; no text was removed. With this commit, empty column selections over numeric element types are again the only input whose result differs from master.

Verification for this commit

  • An outer-storage probe ran on master and here (Julia 1.13). It covered Matrix, 3-D, Int-element, SVector-element, reshaped-view, permutedims, OffsetVector, view and StructArray outer storage. Each case indexed with Colon, a range, a vector, a mask, a matrix index, an empty range and offset-axis indices, and also tried out-of-bounds columns and rows. Here the results differ from master only on the empty selections. Separately, a mask of the wrong length on StructArray outer storage throws BoundsError in both, with different message text.
  • The element-type probe from the first round (51 cases) still differs from master only on the 7 numeric empty selections.
  • New testset, run on its own:
    • eccc962: Pass 28, Fail 2.
    • this commit: Pass 30, Total 30 on Julia 1.13.0 and 1.10.12.
    • master: Pass 21, Fail 2, Error 7. These are the allocation-bound failures and the empty-selection errors.
  • Speed is unchanged: A[2, :] takes ~1.9–2.1 µs and 3 allocs. On matrix outer storage (20×50 of 5-vectors), M[2, :] takes 4394 ns / 8 allocs / 24184 B on master and 1871 ns / 3 allocs / 8072 B here.
  • GROUP=Core, Julia 1.13.0 and 1.10.12: Testing RecursiveArrayTools tests passed. VecOfArr Indexing Tests shows 210 Pass, 2 Broken; both broken tests are pre-existing @test_broken that are also on master. GROUP=QA, Julia 1.13.0: QA | 20 20. Runic (--check) and typos report nothing.

Breaking change

None. With a numeric element type, an empty column selection now returns an empty vector where master threw ArgumentError: `stack` on an empty collection is not allowed. All three reviewers judged that a fix that does not need a version bump. No other result differs from master in any probe or test run here.


🤖 Posted by an AI agent — harness: Claude Code 2.1.301 · model: claude-opus-5-5
Conversation: local session ID 62cad038-fd69-43d9-bdf5-5d147b550253 (headless, no URL)

Risk assessment (independent review, codex-cli gpt-6.1-sol, reviewed head 450e84a)

  • Risk: low
  • Blast radius: Scalar-Int-prefix slicing of AbstractVectorOfArray and AbstractDiffEqArray with concrete numeric inner element types, including common sol[i, :] calls. Non-numeric/abstract element types and mixed prefixes retain the stack fallback. No public name/signature removal, dependency addition, license change, or unflagged breaking change. Returning an empty numeric vector instead of an incidental stack error is a disclosed edge-case fix; no major bump is warranted.
  • Evidence: Reviewed head 450e84a, unchanged at the final check. Ran JULIA_NUM_THREADS=4 JULIA_NUM_PRECOMPILE_TASKS=4 GROUP=Core julia +release --project=repo -e 'using Pkg; Pkg.test()' on Julia 1.13.2: tests passed; indexing 210 pass, 2 pre-existing broken. Logs: core.log (also independently completed a duplicate invocation using a separate local depot, core-local.log). Extracted the 22 numeric regression assertions verbatim from the added testset: current head 22/22; prior eccc962 20 pass/2 fail, specifically matrix and 3-D outer-storage Colon shapes; master 76bec79 13 pass/2 allocation failures/7 empty-selection errors. See probe.log, probe-prior.log, probe-master.log. All 22 PR checks pending at both snapshots, no reported failures: https://github.com/SciML/RecursiveArrayTools.jl/actions/runs/38922149619 and https://github.com/SciML/RecursiveArrayTools.jl/actions/runs/38922149148 . Latest master runs include successful checks and a downstream failure; read its failed-job log: SciMLSensitivity Core4 threaded ensemble errors with Enzyme AssertionError: mi2 !== nothing (master-failure.log), https://github.com/SciML/RecursiveArrayTools.jl/actions/runs/38819764786 . No failing PR job exists to compare yet. Did not run local LTS, QA, downstream, GPU, AD, docs, or independent timing benchmarks.
  • Independent review: codex-cli 0.162.1 gpt-6.1-sol rated it low, high confidence: narrow numeric guard preserves fallback semantics and local tests reproduce the shape regression before the fix and pass afterward.
  • Merge: auto-merge candidate — no blocking findings or observed regressions; normal required CI must finish before merging.

🤖 Posted by an AI agent — harness: codex-cli · model: gpt-6.1-sol (independent reviewer; appended by Claude Code 2.1.301 · claude-opus-5-5)
Conversation: https://claude.ai/code/session_01VZjwf7Ux9YtTzwHGHMqnsq

…rray/DiffEqArray

_getindex's general multi-index method handled A[i, cols] (a scalar
prefix index selecting many columns, e.g. sol[2, :]) via
stack(getindex.(A.u[col_idxs], tuple.(front)...)): this copies the
selected columns into a new Vector (A.u[col_idxs]) and runs the result
through tuple-broadcast machinery before stacking, even though the
output is just a plain Vector of scalars.

Add a fast path for both AbstractVectorOfArray and AbstractDiffEqArray
_getindex: when every prefix index is a scalar Int, build the result
as a direct comprehension over A.u (or a view of it, when col_idxs is
not Colon), skipping both the outer-vector copy and the tuple
broadcast. Falls back to the existing stack-based path whenever any
prefix index is not a scalar Int (including the DiffEqArray
all-Colon-prefix case, checked first since it must keep producing a
DiffEqArray rather than a plain Vector).

n=1000, 5-element inner Float64 vectors, A[2, :]: 4101ns/9 allocs/
24248 B -> 2113ns/3 allocs/8072 B (VectorOfArray); 4733ns/9 allocs
-> 2480ns/3 allocs (DiffEqArray). Verified identical to master on a
real OrdinaryDiffEqTsit5 solution's sol[2, :] and sol[1, 1:5]. Ragged
semantics (R[i, :] throws BoundsError, R[i, j] zero-pads) and the
mixed Int/Colon fallback path are unaffected -- both checked on both
Julia 1.10 and 1.12.

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
@ChrisRackauckas

Copy link
Copy Markdown
Member

🤖 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): CHANGES, risk medium.

Full review

VERDICT: CHANGES
RISK: medium

The fast path is correct for the numeric case, and I measured the speedup myself. On a real Tsit5 ODE solution the output is bit-identical to master. But the PR body says "no behavior change for any input, verified directly", and that is false. On several input classes A[ints..., cols] now returns a different type and shape than master, or no longer throws where master threw. Most of these changes are improvements, but none are disclosed or covered by a test, and they sit on the sol[i, :] hot path. Disclose them and pin them with tests, then this can merge.

Blocking findings

  1. Undisclosed public behavior change for array-valued elements. The PR body claims the opposite. (confirmed by running)
    • Location: src/vector_of_array.jl:676-679 (VoA fast path) and :702-706 (DEA fast path).
    • What changed: master built the result as stack(...) over whatever u[i] returns. If the elements are themselves arrays (VectorOfArray / DiffEqArray of Vector{SVector} or Vector{Vector}, i.e. eltype(A) is an array), master returned those elements stacked into a Matrix. The PR returns a Vector of the elements instead.
    • Command: scratch/probe.jl, run with julia +1.12 against scratch/envM (master tree from git archive origin/master) and scratch/envP (PR checkout). Results in scratch/probeM.txt and scratch/probeP.txt.
      S = VectorOfArray([[SVector(1.0,2,3), SVector(4.0,5,6)] for _ in 1:3])   # size (2,3), eltype SVector{3,Float64}
      S[2, :]     master: Matrix{Float64} 3×3        PR: Vector{SVector{3,Float64}} (3 elements)
      S[2, 1:2]   master: Matrix{Float64} 3×2        PR: Vector{SVector{3,Float64}}
      SD = DiffEqArray(same u, [0,1,2]); SD[2, :]   master: Matrix 3×3   PR: Vector{SVector}
      VV = VectorOfArray([[[1.0,2],[3.0,4]] for _ in 1:3]); VV[2, :]   master: Matrix 2×3   PR: Vector{Vector{Float64}}
      
    • The PR's result is the AbstractArray-consistent one. With it, size(S[2,:]) == (3,) == (size(S,2),) and S[2,:] == collect(S)[2,:] (checked in scratch/probe2.jl). It also matches the documented meaning "A[i, :] gives the timeseries for the ith component" (src/RecursiveArrayTools.jl:89-93). Master returned a 3×3 result for a dimension of length 3. So this is a semantics fix, not a regression, but it is still a public-behavior change.
    • Other input classes that also changed:
      • Empty column selection (A[2, Int[]], A[2, 1:0], VoA and DEA): master throws ArgumentError: stack on an empty collection; the PR returns Float64[].
      • Abstract/Union element types: VectorOfArray(Any[Any[1, 2.0], Any[3.0, 4]])[1, :] was Vector{Float64} [1.0, 3.0] on master; the PR returns Vector{Real} Real[1, 3.0], so the element values are no longer converted.
      • String/Symbol elements: master gave Matrix{Char} for ["ab","cd"] and a MethodError for Symbols; the PR returns Vector{String} and Vector{Symbol}.
    • Required:
      • Correct the PR body to list these changes, and frame them as a fix to match AbstractArray/doc semantics. A maintainer should decide whether the array-valued-eltype change needs more than a patch bump.
      • Add tests that pin the new results, so they can't silently flip back: at minimum S[2, :] for a Vector{SVector} VoA and DEA (Vector of SVectors, size == (size(S,2),)), and A[2, 1:0] == Float64[].
      • The ragged/mixed-prefix tests the PR added pass on master too (see "What I ran"). They pin unchanged behavior and do not cover any of the inputs that actually changed.

Non-blocking findings

  1. The source comments at src/vector_of_array.jl:672-675 and :703-704 say the fast path returns "a plain Vector of scalars". That is wrong for exactly the array-valued-element case above, and should be reworded (e.g. "a Vector of elements, one per selected column"). (from reading, confirmed by probe)
  2. Comment density is high. About 20 of 73 added lines are comments (22 full-line comment lines, of which about 5 are pre-existing lines re-indented in the DEA method, plus 4 inline comments). The guideline is about 10%.
    • The test comments # compile away type instability before measuring are inaccurate: those calls are compilation warm-up for @allocated, and the call is type-stable (Base.return_types gives Vector{Float64} on both master and PR).
    • The 4-line block comment above the test mostly restates its name.
  3. Test name: "O(1) per column" says nothing new, since every implementation is O(1) per column. What the test actually checks is that the outer container isn't copied (a bound on allocations). Cosmetic.
  4. Sibling conflicts:
  5. Behavior that is unchanged, and so pre-existing rather than caused by this PR: with an inner OffsetArray, OI[0, :] follows the inner array's offset axes instead of A's 1-based contract (identical output on master and PR). The concern Add O(1) fast path to AbstractVectorOfArray getindex #668 raises for scalar indexing therefore also applies to this method, both before and after this PR.
  6. No linked issue or tracking issue. The PR has no closingIssuesReferences.

What I ran

All runs used TMPDIR set to the review tmp/. "Master" means the tree from git archive origin/master in scratch/master.

  • Behavior diff master vs PR (scratch/probe.jl, scratch/probe2.jl, Julia 1.12.7). About 70 cases, including:

    • VoA/DEA with Int/range/vector/Bool-mask/empty column selections
    • out-of-bounds row/column, @inbounds
    • single-index A[2:3], A[:]
    • inferred return type
    • nested VoA, matrix inner arrays, ragged, SVector/StructArray/ArrayPartition inner arrays
    • array-valued elements, abstract eltypes, Strings/Symbols
    • offset outer/inner arrays, aliasing of the result

    Only the cases listed in Blocking flatten & reshape #1 differ. Everything else, including every numeric-element case, ragged R[2,:] BoundsError, R[1,1:3], the mixed-prefix fallback, and type inference (Vector{Float64} on both), is identical.

  • Benchmarks (scratch/bench.jl, BenchmarkTools, Julia 1.12, n=1000, 5-element inner vectors). Median times:

    Case master PR
    VoA A[2,:] 2491 ns / 9 allocs / 24768 B 961 ns / 3 allocs / 8256 B
    DEA D[2,:] 2532 ns / 9 allocs 881 ns / 3 allocs
    A[2,1:10] 49 ns / 5 allocs 20 ns / 2 allocs
    B[1,2,:] (matrix inner arrays) 3042 ns / 9 allocs 1646 ns / 3 allocs

    The direction and size of the gain match the PR body. The absolute numbers differ because this is a different machine.

  • Real ODE (scratch/ode.jl, OrdinaryDiffEqTsit5 v2.1.5 and SciMLBase v3.57.0, run in separate master/PR envs). I compared sol[2,:], sol[1,1:5], sol[3,[1,2]], SVector out-of-place sols[2,:], and matrix-state solm[1,2,:] / solm[2,1:3] against master using isequal on serialized results: all 10 equal. The PR's own equality checks are true.

  • New testset, before/after (extracted verbatim into scratch/newtest.jl):

    • Master: 10 pass / 2 fail on both Julia 1.12 and 1.10. The failures are the two @allocated < 10_000 checks (24768 B on 1.12, 24576 B on 1.10).
    • PR: 12/12 on both versions.
    • Logs: scratch/runM12.log, runP12.log, runM10.log, runP10.log.
    • The failing-before/passing-after claim reproduces. Only the allocation bound discriminates; the 10 correctness assertions pass on master too.
  • GROUP=Core on a copy of the PR tree, Julia 1.12: all testsets pass (2 pre-existing @test_broken in VecOfArr Indexing). Log: scratch/groupcore12.log.

  • Runic --check on both changed files: clean. typos on both changed files: clean.

What I did not verify

  • GROUP=Core on Julia 1.10. Only the new testset was run on 1.10.
  • GROUP=QA, Downstream, NoPre, AD, GPU, SymbolicIndexingInterface groups.
  • 32-bit (the CI "Core 32-bit" lane). From reading, the < 10_000 bound should still hold there (8000 B of data and less pointer overhead), but I did not run it.
  • Whether any downstream package relies on master's stacked-Matrix result for array-valued elements, e.g. N-body or Vector{SVector} states indexed as sol[i, :]. I did not search downstream code.
  • ModelingToolkit symbolic indexing that lowers onto this method.

🤖 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)

ChrisRackauckas and others added 3 commits October 16, 2026 07:40
The comprehension fast path only matches the `stack` fallback when the
inner arrays have a concrete `Number` element type. For array-valued
elements (`Vector{SVector}`, `Vector{Vector}`) `stack` concatenates the
elements into a Matrix, and for abstract element types it promotes, so
those inputs return what they return on master again. Empty column
selections with a numeric element type return an empty vector instead
of the `stack` error.

Tests pin the empty-selection results for VectorOfArray, DiffEqArray and
matrix inner arrays, and check that SVector, nested-Vector and Real
element types still match `stack`. The testset is renamed and its
comments trimmed.

Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Harness: Claude Code 2.1.301
Agent-Model: claude-opus-5-5
Agent-Session: local session ID 62cad038-fd69-43d9-bdf5-5d147b550253 (headless claude -p, no URL)
`VectorOfArray` accepts any linearly indexed outer array, and the
`stack` fallback flattens it through `A.u[:]`. The Colon branch of the
fast path iterated `A.u` directly, so on a matrix of inner arrays
`A[i, :]` returned a matrix instead of a vector. Iterate `vec(A.u)`,
which on a `Vector` returns the same object.

Tests cover `A[i, :]` and `A[i, 2:3]` on matrix outer storage and
`A[i, j, :]` on a 3-D outer array of matrices.

Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Harness: Claude Code 2.1.301
Agent-Model: claude-opus-5-5
Agent-Session: local session ID 62cad038-fd69-43d9-bdf5-5d147b550253 (headless claude -p, no URL)

This branch has not been deployed

No deployments
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.

2 participants