Skip to content

Commit 2466954

Browse files
Fix NamedArrayPartition Vector{Int} / UnitRange indexing (#583)
The invalidation-eliminating change in 05faa73 narrowed `getindex(::NamedArrayPartition, ...)` from a catch-all `args...` form to just `Int`. Anything else now falls through `AbstractArray`'s generic indexing, which routes via `similar(x, T, dims)`. That exposed a latent bug in the `similar` overloads: when `dims != size(A)`, `similar(::ArrayPartition, T, dims)` intentionally degrades to a plain `Vector` (a partition layout no longer fits the requested shape). The `NamedArrayPartition` overloads then tried to wrap that `Vector` via the inner constructor `NamedArrayPartition{T, A<:ArrayPartition{T}, NT}(::A, ::NT)`, which raised a MethodError. The catch-all `args...` getindex used to bypass `similar` entirely, so the bug stayed hidden until 05faa73. Make `similar(::NAP, dims)` and `similar(::NAP, T, dims)` mirror `ArrayPartition`'s own degradation: wrap when the inner result is an `ArrayPartition`, otherwise return the fallback array as-is. This restores `x[1:2]` and `x[[1, 4]]` for partial slices, keeps `x[1:end]` returning a `NamedArrayPartition` (the full-range slice still hits the ArrayPartition path), and reintroduces no method that catches a wider signature than necessary — `using RecursiveArrayTools` still produces 0 invalidation trees, verified with `SnoopCompile.invalidation_trees`. Closes #583. Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com> Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 45eb808 commit 2466954

2 files changed

Lines changed: 48 additions & 8 deletions

File tree

src/named_array_partition.jl

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -34,11 +34,16 @@ function Base.similar(A::NamedArrayPartition)
3434
)
3535
end
3636

37-
# return ArrayPartition when possible, otherwise next best thing of the correct size
37+
# return NamedArrayPartition when the requested dims still match the partition layout;
38+
# otherwise fall back to the plain backing array of the correct size. ArrayPartition's
39+
# own `similar(A, dims)` already does this degradation (it returns a Vector when
40+
# `dims != size(A)`), and we simply propagate that result instead of trying to
41+
# wrap a non-ArrayPartition in a NamedArrayPartition (which would hit the inner
42+
# constructor signature `NamedArrayPartition(::A<:ArrayPartition, ::NamedTuple)`).
3843
function Base.similar(A::NamedArrayPartition, dims::NTuple{N, Int}) where {N}
39-
return NamedArrayPartition(
40-
similar(getfield(A, :array_partition), dims), getfield(A, :names_to_indices)
41-
)
44+
inner = similar(getfield(A, :array_partition), dims)
45+
inner isa ArrayPartition || return inner
46+
return NamedArrayPartition(inner, getfield(A, :names_to_indices))
4247
end
4348

4449
# similar array partition of common type
@@ -48,11 +53,10 @@ end
4853
)
4954
end
5055

51-
# return ArrayPartition when possible, otherwise next best thing of the correct size
5256
function Base.similar(A::NamedArrayPartition, ::Type{T}, dims::NTuple{N, Int}) where {T, N}
53-
return NamedArrayPartition(
54-
similar(getfield(A, :array_partition), T, dims), getfield(A, :names_to_indices)
55-
)
57+
inner = similar(getfield(A, :array_partition), T, dims)
58+
inner isa ArrayPartition || return inner
59+
return NamedArrayPartition(inner, getfield(A, :names_to_indices))
5660
end
5761

5862
# similar array partition with different types

test/named_array_partition_tests.jl

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,3 +37,39 @@ using RecursiveArrayTools, ArrayInterface, Test
3737
@test typeof((x -> x[1]).(x)) <: NamedArrayPartition
3838
@test typeof(map(x -> x[1], x)) <: NamedArrayPartition
3939
end
40+
41+
# Regression test for https://github.com/SciML/RecursiveArrayTools.jl/issues/583:
42+
# indexing a NamedArrayPartition with a UnitRange / Vector{Int} smaller than
43+
# the whole array used to throw a MethodError because `similar(::NAP, T, dims)`
44+
# tried to wrap a plain Vector (returned by `similar(::ArrayPartition, T, dims)`
45+
# when `dims != size(A)`) in NamedArrayPartition's inner constructor, which
46+
# requires an ArrayPartition.
47+
@testset "NamedArrayPartition issue #583 indexing" begin
48+
x = NamedArrayPartition(a = ones(2), b = 2 * ones(3))
49+
50+
# UnitRange that doesn't span the whole array: returns a plain Vector
51+
@test x[1:2] == [1.0, 1.0]
52+
@test x[1:2] isa Vector{Float64}
53+
@test x[2:4] == [1.0, 2.0, 2.0]
54+
55+
# Vector{Int} indexing
56+
@test x[[1, 2]] == [1.0, 1.0]
57+
@test x[[1, 4]] == [1.0, 2.0]
58+
@test x[[1, 4]] isa Vector{Float64}
59+
60+
# Existing behavior: full-range slice still preserves NamedArrayPartition
61+
@test typeof(x[1:end]) <: NamedArrayPartition
62+
63+
# `similar` with a non-matching dims tuple gives the backing-array fallback
64+
@test similar(x, Float64, (2,)) isa Vector{Float64}
65+
@test similar(x, (2,)) isa Vector{Float64}
66+
# `similar` with matching dims preserves the NamedArrayPartition wrapper
67+
@test similar(x, Float64, size(x)) isa NamedArrayPartition
68+
@test similar(x, size(x)) isa NamedArrayPartition
69+
70+
# Scalar indexing untouched
71+
@test x[1] == 1.0
72+
@test x[3] == 2.0
73+
x[1] = 99.0
74+
@test x[1] == 99.0
75+
end

0 commit comments

Comments
 (0)