From 34f9a7738c8a2a0457c64e90d9e4936d54a26791 Mon Sep 17 00:00:00 2001 From: cgarling Date: Thu, 6 Aug 2026 17:44:04 -0400 Subject: [PATCH 1/7] Fix stride computation for dimensions with shape 0 in ndarray --- src/ASDF.jl | 26 ++++++++++++++++++++++---- test/test-ndarray.jl | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 4 deletions(-) diff --git a/src/ASDF.jl b/src/ASDF.jl index 26b820b..fd783da 100644 --- a/src/ASDF.jl +++ b/src/ASDF.jl @@ -785,9 +785,21 @@ function NDArray( offset = 0 end if strides isa Nothing - # Calculate byte strides in C order + # Calculate byte strides in C order. Any dimension of length zero is treated as + # length 1 within this product only (not in `shape` itself); this matches NumPy's + # convention for computing default C-contiguous strides (`PyArray_NewFromDescr`), + # relied on by the reference Python `asdf` package when constructing arrays via + # `np.ndarray(shape, dtype, data, offset, None, order)`. Without the clamp, any + # zero-length dimension collapses the strides of every outer dimension whose + # product includes it down to zero, which then fails the `strides` positivity + # check below even though no data is ever read from a zero-size array. Negative + # entries are left unclamped so the `shape` negativity check below still reports + # them with its own clear error message. STScI Roman L2 `.asdf` products contain + # zero-shape arrays (e.g. `chisq`, `dumo`) with no explicit `strides` key, + # triggering this exact failure prior to the fix. sz = sizeof(Type(datatype)) - strides = reverse(cumprod([sz; reverse(shape[(begin + 1):end])])) + clamped_shape = [s == 0 ? 1 : s for s in shape[(begin + 1):end]] + strides = reverse(cumprod([sz; reverse(clamped_shape)])) end return NDArray( lazy_block_headers, source, data, Vector{Int64}(shape), datatype, byteorder, Int64(offset), Vector{Int64}(strides) @@ -833,7 +845,7 @@ size(result) == Tuple(reverse(ndarray.shape)) eltype(result) == ASDF.materialized_eltype(ndarray.datatype) ``` -For the `ucs4` and `ascii` string datatypes, [`materialized_eltype`](@ref) is a thin `AbstractString` view over the characters ([`UCS4String`](@ref) / [`AsciiString`](@ref); see [`stringify_data`](@ref)). For all other datatypes, `eltype(result) == Type(ndarray.datatype)` and additionally `sizeof(eltype) .* strides(result) == Tuple(reverse(ndarray.strides))`. +For the `ucs4` and `ascii` string datatypes, [`materialized_eltype`](@ref) is a thin `AbstractString` view over the characters ([`UCS4String`](@ref) / [`AsciiString`](@ref); see [`stringify_data`](@ref)). For all other datatypes, `eltype(result) == Type(ndarray.datatype)` and additionally, for every dimension with more than one element, `sizeof(eltype) .* strides(result) == Tuple(reverse(ndarray.strides))` along that dimension. (Dimensions of length 0 or 1 have no well-defined stride -- there is no pair of adjacent elements to space apart -- so `reshape`/`reinterpret` are free to report any value there, and such dimensions are excluded from this check.) """ function Base.getindex(ndarray::NDArray) if ndarray.data !== nothing @@ -870,7 +882,13 @@ function Base.getindex(ndarray::NDArray) # Check array layout @assert size(data) == Tuple(reverse(ndarray.shape)) # `data` conforms to specified `ndarray.shape` @assert eltype(data) == Type(ndarray.datatype) # `data` matches type specified by `ndarray.datatype` - if sizeof(eltype(data)) .* Base.strides(data) != Tuple(reverse(ndarray.strides)) + # Dimensions of length 0 or 1 have no meaningful stride (there is no pair of adjacent + # elements to space apart along them), so `reshape`/`reinterpret` are free to report any + # value for them; only compare strides along dimensions with more than one element. + computed_strides = sizeof(eltype(data)) .* Base.strides(data) + expected_strides = Tuple(reverse(ndarray.strides)) + data_shape = size(data) + if any(data_shape[i] > 1 && computed_strides[i] != expected_strides[i] for i in eachindex(data_shape)) error("`data` has different stride from `ndarray.strides`") end diff --git a/test/test-ndarray.jl b/test/test-ndarray.jl index ee6fef1..47786fb 100644 --- a/test/test-ndarray.jl +++ b/test/test-ndarray.jl @@ -85,6 +85,44 @@ end ) end +@testset "implicit strides with zero-length dimensions" begin + # 2026/08/06: Regression test for STScI Roman L2 `.asdf` products produced by romanisim + # which contain `!core/ndarray` nodes (e.g. the `chisq`/`dumo` arrays) + # with a zero-length shape and no explicit `strides` + # key. The naive C-order stride formula (`stride[i] = itemsize * prod(shape[i+1:])`) + # collapses to zero for every dimension whose product includes a zero-length axis, which + # then fails the `strides` positivity check below even though no data is ever read from a + # zero-size array. NumPy avoids this by treating a zero-length axis as though it were + # length 1 when computing default C-contiguous strides (`PyArray_NewFromDescr`), which the + # reference Python `asdf` package relies on. Expected values below were verified against + # `np.ndarray(shape, dtype, buffer, offset, strides=None, order='C').strides`. + cases = [ + (Int64[0, 0], ASDF.Datatype_float16, Int64[2, 2]), + (Int64[0, 5], ASDF.Datatype_float32, Int64[20, 4]), + (Int64[5, 0], ASDF.Datatype_float32, Int64[4, 4]), + (Int64[0, 0, 3], ASDF.Datatype_float32, Int64[12, 12, 4]), + (Int64[2, 0, 3], ASDF.Datatype_float32, Int64[12, 12, 4]), + (Int64[0, 2, 3], ASDF.Datatype_float32, Int64[24, 12, 4]), + ] + for (shape, datatype, expected_strides) in cases + nd = ASDF.NDArray( + ASDF.LazyBlockHeaders(), Int64(0), nothing, shape, datatype, ASDF.host_byteorder, Int64(0), nothing, + ) + @test nd.strides == expected_strides + end + + @testset "materializes a zero-size block-backed array" begin + lbh = ASDF.LazyBlockHeaders() + push!(lbh.block_headers, make_block_header(UInt8[])) + nd = ASDF.NDArray( + lbh, Int64(0), nothing, Int64[0, 0], ASDF.Datatype_float16, ASDF.host_byteorder, Int64(0), nothing, + ) + arr = nd[] + @test size(arr) == (0, 0) + @test eltype(arr) == Float16 + end +end + @testset "getindex" begin opposite = ASDF.host_byteorder == ASDF.Byteorder_little ? ASDF.Byteorder_big : ASDF.Byteorder_little From 65d81f13d4a2ccc21e848126d2e508fee8fecab0 Mon Sep 17 00:00:00 2001 From: Chris Garling Date: Sat, 15 Aug 2026 10:58:49 -0400 Subject: [PATCH 2/7] update `getindex` docstring Co-authored-by: Ian Weaver --- src/ASDF.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ASDF.jl b/src/ASDF.jl index fd783da..e74fbb5 100644 --- a/src/ASDF.jl +++ b/src/ASDF.jl @@ -845,7 +845,7 @@ size(result) == Tuple(reverse(ndarray.shape)) eltype(result) == ASDF.materialized_eltype(ndarray.datatype) ``` -For the `ucs4` and `ascii` string datatypes, [`materialized_eltype`](@ref) is a thin `AbstractString` view over the characters ([`UCS4String`](@ref) / [`AsciiString`](@ref); see [`stringify_data`](@ref)). For all other datatypes, `eltype(result) == Type(ndarray.datatype)` and additionally, for every dimension with more than one element, `sizeof(eltype) .* strides(result) == Tuple(reverse(ndarray.strides))` along that dimension. (Dimensions of length 0 or 1 have no well-defined stride -- there is no pair of adjacent elements to space apart -- so `reshape`/`reinterpret` are free to report any value there, and such dimensions are excluded from this check.) +For the `ucs4` and `ascii` string datatypes, [`materialized_eltype`](@ref) is a thin `AbstractString` view over the characters ([`UCS4String`](@ref) / [`AsciiString`](@ref); see [`stringify_data`](@ref)). For all other datatypes, `eltype(result) == Type(ndarray.datatype)` and additionally, when the array has at least one element, `sizeof(eltype) .* strides(result) == Tuple(reverse(ndarray.strides))` along every dimension with more than one element. (A dimension of length 1 has no well-defined stride, there is no pair of adjacent elements to space apart along it, and in a zero-element array no dimension does, so `reshape`/`reinterpret` are free to report any value there; such strides are excluded from this check.) """ function Base.getindex(ndarray::NDArray) if ndarray.data !== nothing From 1c02d438755125d00676831d932e10f0e4350e9a Mon Sep 17 00:00:00 2001 From: Chris Garling Date: Sat, 15 Aug 2026 11:00:01 -0400 Subject: [PATCH 3/7] shorten test-ndarray.jl test comment Co-authored-by: Ian Weaver --- test/test-ndarray.jl | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/test/test-ndarray.jl b/test/test-ndarray.jl index 47786fb..718c20e 100644 --- a/test/test-ndarray.jl +++ b/test/test-ndarray.jl @@ -86,16 +86,11 @@ end end @testset "implicit strides with zero-length dimensions" begin - # 2026/08/06: Regression test for STScI Roman L2 `.asdf` products produced by romanisim - # which contain `!core/ndarray` nodes (e.g. the `chisq`/`dumo` arrays) - # with a zero-length shape and no explicit `strides` - # key. The naive C-order stride formula (`stride[i] = itemsize * prod(shape[i+1:])`) - # collapses to zero for every dimension whose product includes a zero-length axis, which - # then fails the `strides` positivity check below even though no data is ever read from a - # zero-size array. NumPy avoids this by treating a zero-length axis as though it were - # length 1 when computing default C-contiguous strides (`PyArray_NewFromDescr`), which the - # reference Python `asdf` package relies on. Expected values below were verified against - # `np.ndarray(shape, dtype, buffer, offset, strides=None, order='C').strides`. + # 2026/08/06: Regression test for STScI Roman L2 `.asdf` products produced by romanisim, + # which contain `!core/ndarray` nodes (e.g. `chisq`/`dumo`) with a zero-length shape and + # no explicit `strides` key; see the default-strides comment in the `NDArray` outer + # constructor for the NumPy convention involved. Expected values below were verified + # against `np.ndarray(shape, dtype, buffer, offset, strides=None, order='C').strides`. cases = [ (Int64[0, 0], ASDF.Datatype_float16, Int64[2, 2]), (Int64[0, 5], ASDF.Datatype_float32, Int64[20, 4]), From 96de41cb72887e3e7f1a19f1b4d3eb31c49ef1ea Mon Sep 17 00:00:00 2001 From: Chris Garling Date: Sat, 15 Aug 2026 11:04:24 -0400 Subject: [PATCH 4/7] add `!isempty(data)` check to `getindex` Co-authored-by: Ian Weaver --- src/ASDF.jl | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/ASDF.jl b/src/ASDF.jl index e74fbb5..1fd403f 100644 --- a/src/ASDF.jl +++ b/src/ASDF.jl @@ -882,13 +882,14 @@ function Base.getindex(ndarray::NDArray) # Check array layout @assert size(data) == Tuple(reverse(ndarray.shape)) # `data` conforms to specified `ndarray.shape` @assert eltype(data) == Type(ndarray.datatype) # `data` matches type specified by `ndarray.datatype` - # Dimensions of length 0 or 1 have no meaningful stride (there is no pair of adjacent - # elements to space apart along them), so `reshape`/`reinterpret` are free to report any - # value for them; only compare strides along dimensions with more than one element. + # A dimension of length 1 has no meaningful stride (there is no pair of adjacent elements + # to space apart along it), and in an empty array no dimension does — Julia reports + # stride 0 along any dimension whose faster-varying dimensions include a zero length. + # Only compare strides where they are meaningful. computed_strides = sizeof(eltype(data)) .* Base.strides(data) expected_strides = Tuple(reverse(ndarray.strides)) data_shape = size(data) - if any(data_shape[i] > 1 && computed_strides[i] != expected_strides[i] for i in eachindex(data_shape)) + if !isempty(data) && any(data_shape[i] > 1 && computed_strides[i] != expected_strides[i] for i in eachindex(data_shape)) error("`data` has different stride from `ndarray.strides`") end From 3bda1086d8fbec85d1f2940bfc1d351a47fd2556 Mon Sep 17 00:00:00 2001 From: Chris Garling Date: Sat, 15 Aug 2026 11:05:57 -0400 Subject: [PATCH 5/7] simplify zero-length dimension strides test Co-authored-by: Ian Weaver --- test/test-ndarray.jl | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/test/test-ndarray.jl b/test/test-ndarray.jl index 718c20e..1b0083b 100644 --- a/test/test-ndarray.jl +++ b/test/test-ndarray.jl @@ -100,21 +100,16 @@ end (Int64[0, 2, 3], ASDF.Datatype_float32, Int64[24, 12, 4]), ] for (shape, datatype, expected_strides) in cases - nd = ASDF.NDArray( - ASDF.LazyBlockHeaders(), Int64(0), nothing, shape, datatype, ASDF.host_byteorder, Int64(0), nothing, - ) - @test nd.strides == expected_strides - end - - @testset "materializes a zero-size block-backed array" begin lbh = ASDF.LazyBlockHeaders() push!(lbh.block_headers, make_block_header(UInt8[])) - nd = ASDF.NDArray( - lbh, Int64(0), nothing, Int64[0, 0], ASDF.Datatype_float16, ASDF.host_byteorder, Int64(0), nothing, + nd = make_ndarray(; + lazy_block_headers = lbh, source = Int64(0), data = nothing, shape, datatype, strides = nothing, ) + @test nd.strides == expected_strides + # Materialize from an empty block to catch stride-check false positives at read time. arr = nd[] - @test size(arr) == (0, 0) - @test eltype(arr) == Float16 + @test size(arr) == Tuple(reverse(shape)) + @test eltype(arr) == Type(datatype) end end From b10d858a31ea47e73d1aa0142a710135ca6367e3 Mon Sep 17 00:00:00 2001 From: cgarling Date: Mon, 24 Aug 2026 15:28:49 -0400 Subject: [PATCH 6/7] Implement Ian's suggestions --- src/ASDF.jl | 3 +-- test/test-ndarray.jl | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/src/ASDF.jl b/src/ASDF.jl index 1fd403f..abe3848 100644 --- a/src/ASDF.jl +++ b/src/ASDF.jl @@ -798,8 +798,7 @@ function NDArray( # zero-shape arrays (e.g. `chisq`, `dumo`) with no explicit `strides` key, # triggering this exact failure prior to the fix. sz = sizeof(Type(datatype)) - clamped_shape = [s == 0 ? 1 : s for s in shape[(begin + 1):end]] - strides = reverse(cumprod([sz; reverse(clamped_shape)])) + strides = reverse(cumprod([sz; reverse(max.(shape[(begin + 1):end], 1))])) end return NDArray( lazy_block_headers, source, data, Vector{Int64}(shape), datatype, byteorder, Int64(offset), Vector{Int64}(strides) diff --git a/test/test-ndarray.jl b/test/test-ndarray.jl index 1b0083b..34d0080 100644 --- a/test/test-ndarray.jl +++ b/test/test-ndarray.jl @@ -113,6 +113,26 @@ end end end +@testset "implicit strides with negative-shape elements" begin + # Regression test: the implicit-strides branch of + # the `NDArray` outer constructor clamps negative entries so the + # `strides` positivity check downstream can't misfire on them. That clamping only affects + # the temporary array used to compute `strides`; it must not suppress the `shape` + # negativity check in the inner constructor, which always receives the original, + # unclamped `shape`. Cover a negative element both outside (`shape[1]`) and inside + # (`shape[2:end]`) the slice passed to `max.(...)`. + for shape in (Int64[-1, 3], Int64[3, -1]) + test_ndarray( + ArgumentError, + "`shape` cannot have negative elements."; + source = Int64(0), + data = nothing, + shape, + strides = nothing, + ) + end +end + @testset "getindex" begin opposite = ASDF.host_byteorder == ASDF.Byteorder_little ? ASDF.Byteorder_big : ASDF.Byteorder_little From 624570e16e2563feec27d9cc71c2e9e5094050b9 Mon Sep 17 00:00:00 2001 From: Ian Weaver Date: Mon, 24 Aug 2026 16:10:21 -0700 Subject: [PATCH 7/7] Apply suggestion from @icweaver --- src/ASDF.jl | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/src/ASDF.jl b/src/ASDF.jl index abe3848..ce7d528 100644 --- a/src/ASDF.jl +++ b/src/ASDF.jl @@ -785,18 +785,15 @@ function NDArray( offset = 0 end if strides isa Nothing - # Calculate byte strides in C order. Any dimension of length zero is treated as - # length 1 within this product only (not in `shape` itself); this matches NumPy's - # convention for computing default C-contiguous strides (`PyArray_NewFromDescr`), - # relied on by the reference Python `asdf` package when constructing arrays via - # `np.ndarray(shape, dtype, data, offset, None, order)`. Without the clamp, any - # zero-length dimension collapses the strides of every outer dimension whose + # Calculate byte strides in C order, treating any zero-length dimension as + # length 1 within this product only (not in `shape` itself). This matches NumPy's + # convention for default C-contiguous strides (`PyArray_NewFromDescr`), + # relied on by the reference Python `asdf` package. Without the clamp, a + # zero-length dimension collapses the stride of every outer dimension whose # product includes it down to zero, which then fails the `strides` positivity # check below even though no data is ever read from a zero-size array. Negative - # entries are left unclamped so the `shape` negativity check below still reports - # them with its own clear error message. STScI Roman L2 `.asdf` products contain - # zero-shape arrays (e.g. `chisq`, `dumo`) with no explicit `strides` key, - # triggering this exact failure prior to the fix. + # entries are left unclamped so the shape negativity check below still reports them with its own clear error message. STScI Roman L2 `.asdf` products contain such + # zero-shape arrays (e.g. `chisq`, `dumo`) with no explicit `strides` key. sz = sizeof(Type(datatype)) strides = reverse(cumprod([sz; reverse(max.(shape[(begin + 1):end], 1))])) end