Skip to content

Commit 542ceba

Browse files
d-v-bCAOShurong
andauthored
fix(chunk-grids): make array creation O(1) in per-dimension chunk count (#4218)
* fix(chunk-grids): make array creation O(1) in per-dimension chunk count Chunk normalization now returns a ChunkGrid whose uniform dimensions are stored as FixedDimension (size + extent) instead of one array entry per chunk, so create_array(shape=(2**62,), chunks=(1,)) succeeds instantly instead of raising "array is too big", and chunks=(1, 1) on a (2**31, 2**31) array no longer allocates ~17 GB per dimension. Explicit per-chunk lists collapse to FixedDimension when they describe a regular grid; genuinely irregular lists become VaryingDimension. Both variants bind chunk sizes to their extent, so the two forms carry the same invariants. The intermediate ChunksTuple type and as_regular_shape helper are removed; create_chunk_grid_metadata consumes the grid directly and serializes uniform dimensions of mixed rectilinear grids as the spec's bare-int step-size shorthand. Creation-time counterpart of the gh-4174 indexing fix. Assisted-by: ClaudeCode:claude-fable-5 * refactor(chunk-grids): drop dead ndarray branch from is_regular_1d Since normalize_chunks_nd returns ChunkGrid dimensions instead of int64 arrays, is_regular_1d only ever receives plain Python sequences; the vectorized numpy path was unreachable in production code. Remove it and narrow the signatures of is_regular_1d / is_regular_nd to Sequence[int]. Assisted-by: ClaudeCode:claude-fable-5 * test(metadata): cover the unknown-dimension-type guard in create_chunk_grid_metadata The defensive TypeError branch is unreachable through the public API, so exercise it directly with a stub dimension object. This was the only genuinely uncovered patch line in #4218; the other lines codecov flagged came from a partial coverage upload. Assisted-by: ClaudeCode:claude-fable-5 * fix(testing): pass bare-int rectilinear dims through the chunks= conversion The stateful hypothesis tests convert generated chunk grid metadata back into a create_array chunks= argument. Bare-int dimensions — the spec's step-size shorthand, now produced when a uniform dimension of a mixed grid collapses — were wrapped as single-element lists, turning "repeat to cover the axis" into "exactly one chunk" and failing the sum-to-span check (e.g. chunks=[1] for span 3). Extract the conversion into chunks_param_from_rectilinear, pass bare ints through unchanged, and widen ChunksLike to admit mixed int | sequence per-dimension specs, which the normalizer already accepted. Assisted-by: ClaudeCode:claude-fable-5 * fix(chunk-grids): keep explicit per-chunk lists rectilinear Drop the regular-grid collapse for explicit list input in normalize_chunks_1d: a per-chunk size list now always produces VaryingDimension, even when the sizes are uniform or uniform plus a short tail. Scalar specs (ints, numpy integers, and the -1 sentinel) still produce FixedDimension, which is the path that makes array creation O(1) in per-dimension chunk count; explicit lists are already O(n) in the input, so nothing is lost. The grid kind now follows the input syntax, matching 3.2.x behavior: previously an explicitly rectilinear spec whose edges happened to look regular was silently stored as RegularChunkGridMetadata, which changes resize semantics — a regular grid grows by extending the uniform pattern while a rectilinear grid appends an edge chunk, breaking append-oriented layouts like (168,) * 13 + (24,). This is resolution option 1 from gh-4272. Uniform dimensions of mixed scalar/list specs still serialize as the spec's bare-int step-size shorthand; explicit lists serialize as edge lists. The touched-chunk-keys assertions in the resize regression test follow the approach from gh-4290, which fixes the same issue on main via a requested_rectilinear flag. Fixes #4272 Co-authored-by: Shurong Cao <CAOShurong@users.noreply.github.com> Assisted-by: ClaudeCode:claude-fable-5 * fix(array): guard chunks/shards access by stored grid kind, not runtime grid The stateful hypothesis tests in CI failed in from_array on a rectilinear source whose edges happen to be uniform (e.g. chunk_shapes=((3,),)): _parse_keep_array_attr and _info guarded data.chunks / data.shards with the runtime ChunkGrid.is_regular, but the runtime grid collapses uniform rectilinear dimensions to FixedDimension as an optimization, while metadata.chunks raises for any array whose stored grid is rectilinear. Before explicit per-chunk lists stopped collapsing at creation, the two notions of regularity always agreed for created arrays, so the mismatch was unreachable. Add _stored_chunk_grid_is_regular, which dispatches on the metadata kind, and use it at the three sites that read .chunks/.shards. A uniform-edged rectilinear source now round-trips through from_array with its grid kind intact. Assisted-by: ClaudeCode:claude-fable-5 * fix(array): make from_array 'keep' faithful for every stored chunk grid Fixes the three failures maxrjones identified in review of #4218, whose shared root cause was that from_array is really create_array plus an inverse mapping from stored grid metadata back into the chunks=/shards= parameter space, and that inverse mapping only handled regular grids: - The write_data=True copy path iterated shard regions via the .chunks/.shards accessors, which raise for rectilinear grids. _iter_shard_regions (and _shard_grid_shape) now iterate the stored chunk grid directly, which describes the write regions for every grid kind. - chunks='keep' expanded uniform dimensions of a rectilinear grid to one entry per chunk via write_chunk_sizes, resurrecting the O(nchunks) path and changing resize semantics. The stored chunk_shapes now pass through as-is, preserving the bare-int shorthand in O(ndim). - shards='keep' silently dropped sharding when the shard grid was rectilinear, and .info raised on such arrays through the .shards accessor. The shard grid's chunk_shapes now round-trip through the shards= parameter, and _info guards both accessors by stored grid kind. metadata.chunks now returns the inner chunk shape for any sharded array (inner chunks are always regular, whatever the shard grid), and the repeated sharding-codec isinstance dance is extracted into ArrayV3Metadata.sharding_codec per the existing TODO. ShardsLike admits mixed bare-int/sequence specs, matching what the normalizer accepts. Tests: the ad-hoc from_array regression test is replaced by a spec matrix (FROM_ARRAY_KEEP_CASES) covering every kind of chunk/shard spec create_array accepts, round-tripped through from_array at both write_data settings with grid, codec, and data equality asserted, plus an O(1) pin at shape=(2**62, 30) and error tests for the two accessors that still raise. The stateful hypothesis machine drops its write_data=False workaround, restoring property coverage of the default copy path. Assisted-by: ClaudeCode:claude-fable-5 * fix(array): coherent introspection for rectilinear-sharded arrays Follow-up to the from_array 'keep' fixes, addressing the remaining inconsistencies in rectilinear-sharded introspection: - .info / info_complete rendered such arrays as unsharded: no shard line, and the stored-shard count labeled 'Chunks Initialized'. ArrayInfo._shard_shape now admits a '<variable>' sentinel, so the shard shape renders as <variable> and the count is labeled 'Shards Initialized'. - nchunks_initialized raised through the .shards accessor. It now sums the chunk count of each initialized shard individually (shard sizes vary under a rectilinear shard grid; the per-shard sum reduces to the old uniform multiplier for regular shard grids). - The user guide still said .chunks is only available for regular grids; it now documents that sharded arrays always have .chunks, and the rectilinear-shard section shows .chunks/.write_chunk_sizes/.info for such arrays. The round-trip matrix gains an initialized-count assertion, plus tests for the info rendering and the per-shard count. Assisted-by: ClaudeCode:claude-fable-5 * fix(array): preserve stored grids verbatim through from_array 'keep', restore iteration bounds checks Addresses maxrjones's second review round on #4218: - from_array with chunks='keep' / shards='keep' failed on sources shrunk by resize: the stored grid legally keeps trailing edges beyond the extent, but the keep path re-validated it through the strict user-spec parser. ChunksLike/ShardsLike now admit a stored ChunkGridMetadata, which normalize_chunks_nd accepts under the tolerant from_sizes rules and init_array stores verbatim — the keep path passes the grid object itself, so the copy preserves the stored grid exactly. This also fixes the last fidelity gap: an externally-written rectilinear grid using bare-int shorthand on every dimension no longer degrades to a regular grid. - _iter_shard_regions silently truncated out-of-bounds selections after the switch to ChunkGrid.iter_chunk_regions, while its sibling _iter_shard_keys raises. all_chunk_coords now applies the same bounds check as _iter_grid, so both APIs fail loudly and consistently. - _is_rectilinear_chunks only sampled the first element, so mixed per-dimension specs with a bare int first bypassed the v2 and chunks+shards gates and died later on an internal error. It now scans all dimensions (and recognizes rectilinear grid metadata). - _nchunks_initialized walked every shard coordinate even for regular shard grids; the O(1) multiply is restored there, and rectilinear shard grids now decode only the initialized keys instead of scanning the full grid. - The #4174 changelog entry no longer claims #4172 as the full indexing counterpart; the coordinate-selection allocation stays tracked in #4174. FROM_ARRAY_KEEP_CASES gains shrunk rectilinear and shrunk rectilinear-sharded rows, plus tests for the bounds check, the order-independent gates, the regular-sharded multiply agreeing with the per-shard sum, and the all-bare-int keep round-trip. Assisted-by: ClaudeCode:claude-fable-5 * refactor+docs: apply ilan-gold's review suggestions - Contrast read_chunk_sizes and write_chunk_sizes in their docstring examples with a sharded array where the two differ (the previous examples were identical, which is what prompted the question), and route read_chunk_sizes through the _sharding_codec helper. - Validate scalar chunk sizes before handling the -1 sentinel, with an error message that names -1 as valid. - Rename _stored_rectilinear_grid to _stored_rectilinear_grid_or_none. - User guide: state why inner chunks are always regular (the sharding codec requires a uniform inner chunk shape). - Changelog: make explicit that everything in the entry sits behind the experimental rectilinear feature flag. Assisted-by: ClaudeCode:claude-fable-5 --------- Co-authored-by: Shurong Cao <CAOShurong@users.noreply.github.com>
1 parent ef6cfe2 commit 542ceba

16 files changed

Lines changed: 844 additions & 276 deletions

File tree

‎changes/4174.bugfix.md‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
Array creation is now O(1) in the number of chunks per dimension. Chunk
2+
normalization returns a `ChunkGrid` whose uniform dimensions are stored as a
3+
size + extent pair (`FixedDimension`) instead of being expanded to one entry
4+
per chunk, so creating arrays like
5+
`zarr.create_array(store, shape=(2**62,), chunks=(1,), dtype='int32')` succeeds
6+
instantly instead of raising `ValueError` or allocating gigabytes of memory.
7+
The intermediate `ChunksTuple` representation was removed in the process, and
8+
`ChunksLike` now admits per-dimension specs that mix a bare int (uniform chunk
9+
size) with explicit edge-length sequences, matching what the normalizer and
10+
the rectilinear grid spec already accepted.
11+
This fixes the array-creation half of #4174; the coordinate-selection
12+
allocation reported there is still tracked in that issue (#4172 fixed the
13+
related case of sorted 1-D coordinate selections).

‎changes/4272.bugfix.md‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
Explicit per-chunk size lists now always produce a rectilinear chunk grid,
2+
even when the sizes happen to describe a regular grid (all equal, or all equal
3+
with a smaller trailing chunk). Previously such input was silently collapsed to
4+
a regular grid, which changed resize semantics: a regular grid grows by
5+
extending the uniform pattern, while a rectilinear grid appends a new edge
6+
chunk — the behavior an append-oriented layout like `(168,) * 13 + (24,)`
7+
relies on. The grid kind now follows the input syntax, matching 3.2.x:
8+
scalar chunk sizes (including numpy integers and the `-1` sentinel) produce a
9+
regular grid, nested sequences produce a rectilinear grid. Rectilinear grids
10+
remain gated behind `zarr.config.set({"array.rectilinear_chunks": True})`.
11+
See #4174 for the accompanying O(1) chunk normalization change.
12+
13+
`zarr.from_array` with the default `chunks="keep"` / `shards="keep"` now
14+
reproduces the source's stored grid exactly: a rectilinear grid is passed
15+
through in O(number of dimensions), with uniform dimensions keeping their
16+
bare-int shorthand; sharding under a rectilinear shard grid is preserved
17+
instead of being silently dropped; and the default `write_data=True` copy
18+
works for every grid kind. `Array.chunks` is now defined for any sharded array
19+
(the inner chunks of a shard are always regular), and for sharded arrays with
20+
a rectilinear shard grid `Array.info` no longer raises — it reports the shard
21+
shape as `<variable>` — while `Array.nchunks_initialized` counts the chunks of
22+
each initialized shard individually instead of raising.
23+
24+
Everything described here concerns rectilinear chunk grids, which remain an
25+
experimental feature gated behind
26+
`zarr.config.set({"array.rectilinear_chunks": True})`; arrays with regular
27+
chunk grids are unaffected.

‎docs/user-guide/arrays.md‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -674,8 +674,12 @@ z_regular = zarr.create_array(
674674
print(z_regular.write_chunk_sizes)
675675
```
676676

677-
Note that the `.chunks` property is only available for regular chunk grids. For
678-
rectilinear arrays, use `.write_chunk_sizes` (or `.read_chunk_sizes`) instead.
677+
Note that the `.chunks` property is not available for non-sharded rectilinear
678+
arrays, since there is no single uniform chunk shape — use `.write_chunk_sizes`
679+
(or `.read_chunk_sizes`) instead. Sharded arrays always have `.chunks`: it
680+
returns the inner chunk shape, which is always regular — the sharding codec
681+
requires a single uniform inner chunk shape, so only the shard boundaries can
682+
be rectilinear (see [Rectilinear shard boundaries](#rectilinear-shard-boundaries)).
679683

680684
### Resizing and appending
681685

@@ -746,6 +750,17 @@ print(z[50:70, 40:60])
746750
Note that rectilinear inner chunks with sharding are not supported — only the
747751
shard boundaries can be rectilinear.
748752

753+
For such arrays, `.chunks` returns the (regular) inner chunk shape, while
754+
`.shards` raises `NotImplementedError` since there is no single uniform shard
755+
shape — use `.write_chunk_sizes` for the per-dimension shard sizes. `.info`
756+
reports the shard shape as `<variable>`:
757+
758+
```python exec="true" session="arrays" source="above" result="ansi"
759+
print(f"chunks={z.chunks}")
760+
print(f"shard sizes={z.write_chunk_sizes}")
761+
print(z.info)
762+
```
763+
749764
### Metadata format
750765

751766
Rectilinear chunk grid metadata uses run-length encoding (RLE) for compact

‎src/zarr/core/_info.py‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,9 @@ class ArrayInfo:
8282
_data_type: ZDType[TBaseDType, TBaseScalar]
8383
_fill_value: object
8484
_shape: tuple[int, ...]
85-
_shard_shape: tuple[int, ...] | None = None
85+
# "<variable>" marks a sharded array whose shard grid is rectilinear, so
86+
# there is no uniform shard shape; None means the array is not sharded.
87+
_shard_shape: tuple[int, ...] | Literal["<variable>"] | None = None
8688
_chunk_shape: tuple[int, ...] | None = None
8789
_order: Literal["C", "F"]
8890
_read_only: bool

0 commit comments

Comments
 (0)