Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions changes/4352.bugfix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
Validate each codec in a chain against the chunk geometry produced by the codecs before it, including the inner codecs of a sharding codec and the actual fill value. For example, a sharding codec placed after a transpose must now divide the transposed chunks; previously it was checked against the untransposed chunk grid, which could accept chains that read back wrong data and reject valid ones.

Geometry is threaded through the chain as a whole chunk grid, so validation cost no longer depends on the number of distinct chunk shapes. Codecs describe how they map the grid with the new `BaseCodec.resolve_chunk_grid` method; the built-in dtype and fill-value codecs (`cast_value`, `scale_offset`, and the numcodecs `delta`, `fixedscaleoffset` and `astype` filters) declare the identity and `transpose` declares a permutation.

Trade-off: on a rectilinear chunk grid, a codec that overrides `resolve_metadata` without implementing `resolve_chunk_grid` makes the rest of its chain "chunk-local". Codecs after it are validated against one representative chunk (the largest edge along each axis), so a chain that is invalid only for some other chunk shape is accepted when the array is created and fails when such a chunk is first written or read. `ShardingCodec` now checks divisibility at encode and decode time, so this case raises an error instead of corrupting data. Regular chunk grids are always validated exactly. See "Chunk geometry and validation" in the extending guide.

The codec pipeline of a rectilinear-chunked array is now evolved against that same representative chunk shape, instead of an all-ones placeholder that broke shape-dependent codecs at array creation even when the metadata validated.
8 changes: 8 additions & 0 deletions docs/user-guide/arrays.md
Original file line number Diff line number Diff line change
Expand Up @@ -749,6 +749,14 @@ print(z[50:70, 40:60])
Note that rectilinear inner chunks with sharding are not supported — only the
shard boundaries can be rectilinear.

!!! note "When the divisibility check happens"
Zarr checks that the inner chunk shape divides every shard when the array is
created, as long as each filter before the sharding codec describes how it maps
the chunk grid. All built-in filters that keep the chunk shape, and `transpose`, do. A third-party filter that changes chunk
metadata without describing its grid limits that check to the largest shard. A
smaller shard that is not divisible is then reported only when it is first written
or read. See [Chunk geometry and validation](extending.md#chunk-geometry-and-validation).

For such arrays, `.chunks` returns the (regular) inner chunk shape, while
`.shards` raises `NotImplementedError` since there is no single uniform shard
shape — use `.write_chunk_sizes` for the per-dimension shard sizes. `.info`
Expand Down
56 changes: 56 additions & 0 deletions docs/user-guide/extending.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,65 @@ Custom codecs should also implement the following methods:
array metadata. It should raise errors if not.
- `resolve_metadata` (optional), which is important for codecs that change the shape,
dtype or fill value of a chunk.
- `resolve_chunk_grid` (optional, but recommended for any codec that overrides
`resolve_metadata`), which describes how the codec changes the chunk grid as a whole.
See [Chunk geometry and validation](#chunk-geometry-and-validation).
- `evolve_from_array_spec` (optional), which can be useful for automatically filling in
codec configuration metadata from the array metadata.

### Chunk geometry and validation

When array metadata is created, Zarr validates every codec against the chunks it will
actually receive, which are the chunks produced by the codecs before it. For example, in
the chain `transpose` → `sharding_indexed`, the shard's inner chunk shape must divide the
*transposed* chunks.

The geometry is carried through the chain as a whole chunk grid rather than chunk by
chunk. A rectilinear grid with `n` distinct edge lengths along each of `d` axes has `n**d`
distinct chunk shapes, which quickly becomes too many to check one at a time (a 3-d grid
with 1000 distinct edges per axis has a billion). Each codec therefore reports how it
changes the grid through `resolve_chunk_grid(shape=..., chunk_grid=...)`:

- **Return `(shape, chunk_grid)` unchanged** if the codec never changes the chunk
shape, even if it changes the data type or fill value. This is the default for codecs
that do not override `resolve_metadata`. If your codec overrides `resolve_metadata`
only to change the data type or fill value, override `resolve_chunk_grid` too, as
`CastValue`, `ScaleOffset` and the numcodecs `Delta` do.
- **Return the mapped shape and grid** if the codec changes chunk shapes in a way a grid
can express. `TransposeCodec`, for example, permutes the axes of both.
- **Return `None`** if the chunks after the codec cannot be described by a single grid
computed from the input grid. This is the default for codecs that override
`resolve_metadata`.

Returning `None` has no cost on a regular chunk grid, because every chunk has the same
shape and `resolve_metadata` describes it exactly. On a rectilinear chunk grid it makes
the rest of the chain **chunk-local**. From then on, codecs are validated against a single
representative chunk: the one with the largest edge along each axis. This has the
following consequences:

- **Rejections are always correct.** The representative is a real chunk of the array, so
a codec that rejects it would fail on that chunk.
- **Acceptance is incomplete.** A chain that is invalid only for some *other* chunk
shape is accepted when the array is created, and the error surfaces when a chunk of
that shape is first encoded or decoded. Suppose a sharding codec follows an undeclared
filter on a grid with chunk edges `[4, 6]`. An inner chunk size of `3` divides the
representative edge `6`, so the array is created, but writing to a chunk with edge `4`
raises an error.
- **Size-sensitive codecs must check at run time.** Any codec whose correctness depends on
the chunk shape (for example, requiring it to divide evenly) must repeat that check
when encoding and decoding, because metadata validation may have seen only the
representative chunk. `ShardingCodec` does this. A codec that skips the check can
silently corrupt data on the chunks that validation never saw.

Zarr uses a declared grid only when it can trust it. A declaration is ignored, and the
codec treated as returning `None`, if a subclass overrides `resolve_metadata` without
also overriding `resolve_chunk_grid`. It is also ignored if the declared grid disagrees
with what `resolve_metadata` returns for the representative chunk.

This design follows [zarrs](https://github.com/zarrs/zarrs), whose array-to-array codecs
map chunk grids through `encoded_chunk_grid` and return a "chunk-local" result when no
whole-array grid exists.

To use custom codecs in Zarr, they need to be registered using the
[entrypoint mechanism](https://packaging.python.org/en/latest/specifications/entry-points/).
Commonly, entrypoints are declared in the `pyproject.toml` of your package under the
Expand Down
49 changes: 49 additions & 0 deletions src/zarr/abc/codec.py
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,55 @@ def resolve_metadata(self, chunk_spec: ArraySpec) -> ArraySpec:
"""
return chunk_spec

def resolve_chunk_grid(

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ilan-gold this is new API on the codec abc. we need the whole chunk grid to be in-scope for resolution because combining a rectilinear chunk grid with e.g. a transpose or reshape codec creates a large number of chunk shapes, and each chunk size needs to be consistent with the subchunk grid of e.g. a sharding codec. The meaning of "consistent" will in theory vary with the downstream codecs, but for sharding we just need to ensure that each incoming chunk shape tiles the sharding chunk grid.

this would be a lot easier if the sharding codec chunk grid was semi-regular.

self, *, shape: tuple[int, ...], chunk_grid: ChunkGridMetadata
) -> tuple[tuple[int, ...], ChunkGridMetadata] | None:
"""The array shape and chunk grid seen by the codecs after this one.

This is the whole-array counterpart of `resolve_metadata`: where
`resolve_metadata` maps the spec of one chunk, this maps the geometry
of every chunk at once. It is used to validate a codec chain when the
array metadata is created, so that a later size-sensitive codec (such
as `ShardingCodec`) is checked against the chunks it will actually
receive, without enumerating the chunks of the grid.

Return `None` when the chunks after this codec cannot be described by
a single chunk grid computed from `chunk_grid` alone. On a regular grid
that costs nothing, because all chunks have one shape and
`resolve_metadata` describes them exactly. On a rectilinear grid it
makes the remaining chain "chunk-local": later codecs are validated
against one representative chunk only, so a chain that is invalid for
some other chunk shape is accepted at creation and fails when such a
chunk is first encoded or decoded (see
`zarr.core.metadata.v3.evolve_and_validate_codecs`).

The default declares the identity when `resolve_metadata` is not
overridden, and returns `None` otherwise. Codecs that override
`resolve_metadata` without changing the chunk shape (for example to
change the data type or fill value) should override this method to
return `(shape, chunk_grid)` unchanged, and codecs that change the chunk
shape in a way expressible as a grid (for example a permutation of the
axes) should return the mapped geometry.

A declaration is ignored if a subclass overrides `resolve_metadata`
without also overriding this method, or if it disagrees with
`resolve_metadata` on the representative chunk.

Parameters
----------
shape : tuple[int, ...]
The array shape seen by this codec.
chunk_grid : ChunkGridMetadata
The chunk grid seen by this codec.

Returns
-------
tuple[tuple[int, ...], ChunkGridMetadata] | None
"""
if type(self).resolve_metadata is BaseCodec.resolve_metadata:
return shape, chunk_grid
return None

def evolve_from_array_spec(self, array_spec: ArraySpec) -> Self:
"""Fills in codec configuration parameters that can be automatically
inferred from the array metadata.
Expand Down
6 changes: 6 additions & 0 deletions src/zarr/codecs/cast_value.py
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,12 @@ def resolve_metadata(self, chunk_spec: ArraySpec) -> ArraySpec:

return replace(chunk_spec, dtype=target_zdtype, fill_value=new_fill)

def resolve_chunk_grid(
self, *, shape: tuple[int, ...], chunk_grid: ChunkGridMetadata
) -> tuple[tuple[int, ...], ChunkGridMetadata]:
"""Casting changes the data type and fill value, never the chunk shape."""
return shape, chunk_grid

def _encode_sync(
self,
chunk_array: NDBuffer,
Expand Down
19 changes: 19 additions & 0 deletions src/zarr/codecs/numcodecs/_codecs.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
from zarr.abc.numcodec import Numcodec
from zarr.core.array_spec import ArraySpec
from zarr.core.buffer import Buffer, NDBuffer
from zarr.core.metadata.v3 import ChunkGridMetadata

CODEC_PREFIX = "numcodecs."

Expand Down Expand Up @@ -241,6 +242,12 @@ def resolve_metadata(self, chunk_spec: ArraySpec) -> ArraySpec:
return replace(chunk_spec, dtype=dtype)
return chunk_spec

def resolve_chunk_grid(
self, *, shape: tuple[int, ...], chunk_grid: ChunkGridMetadata
) -> tuple[tuple[int, ...], ChunkGridMetadata]:
"""Delta encoding may change the data type, never the chunk shape."""
return shape, chunk_grid


class BitRound(_NumcodecsArrayArrayCodec, codec_name="bitround"):
pass
Expand All @@ -253,6 +260,12 @@ def resolve_metadata(self, chunk_spec: ArraySpec) -> ArraySpec:
return replace(chunk_spec, dtype=dtype)
return chunk_spec

def resolve_chunk_grid(
self, *, shape: tuple[int, ...], chunk_grid: ChunkGridMetadata
) -> tuple[tuple[int, ...], ChunkGridMetadata]:
"""Scaling may change the data type, never the chunk shape."""
return shape, chunk_grid

def evolve_from_array_spec(self, array_spec: ArraySpec) -> FixedScaleOffset:
if self.codec_config.get("dtype") is None:
dtype = array_spec.dtype.to_native_dtype()
Expand Down Expand Up @@ -293,6 +306,12 @@ def resolve_metadata(self, chunk_spec: ArraySpec) -> ArraySpec:
dtype = parse_dtype(np.dtype(self.codec_config["encode_dtype"]), zarr_format=3) # type: ignore[arg-type]
return replace(chunk_spec, dtype=dtype)

def resolve_chunk_grid(
self, *, shape: tuple[int, ...], chunk_grid: ChunkGridMetadata
) -> tuple[tuple[int, ...], ChunkGridMetadata]:
"""Casting changes the data type, never the chunk shape."""
return shape, chunk_grid

def evolve_from_array_spec(self, array_spec: ArraySpec) -> AsType:
if self.codec_config.get("decode_dtype") is None:
# TODO: remove these coverage exemptions the correct way, i.e. with tests
Expand Down
6 changes: 6 additions & 0 deletions src/zarr/codecs/scale_offset.py
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,12 @@ def resolve_metadata(self, chunk_spec: ArraySpec) -> ArraySpec:
new_fill = _encode(fill, offset, scale)
return replace(chunk_spec, fill_value=new_fill.reshape(()).item())

def resolve_chunk_grid(
self, *, shape: tuple[int, ...], chunk_grid: ChunkGridMetadata
) -> tuple[tuple[int, ...], ChunkGridMetadata]:
"""Scaling changes the fill value, never the chunk shape."""
return shape, chunk_grid

def _decode_sync(
self,
chunk_array: NDBuffer,
Expand Down
49 changes: 37 additions & 12 deletions src/zarr/codecs/sharding.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@
ChunkGridMetadata,
RectilinearChunkGridMetadata,
RegularChunkGridMetadata,
evolve_and_validate_codecs,
parse_codecs,
)
from zarr.registry import get_ndbuffer_class, get_pipeline_class
Expand Down Expand Up @@ -559,14 +560,21 @@ def to_dict(self) -> dict[str, JSON]:
}

def evolve_from_array_spec(self, array_spec: ArraySpec) -> Self:
"""Thread the spec through the inner chain.
"""Thread the spec through the inner chain, evolving and validating it.

Each codec is evolved against the spec produced by the previous one.
Evolving every codec against the same unthreaded spec is the bug shape that
strips `BytesCodec.endian` behind a dtype-changing codec — and this
method runs on the real array-creation path, baking the damaged chain
into the evolved instance before the transform builders ever run.

The inner chain is validated here rather than in `validate`, because
only the array spec carries the fill value some inner codecs need to
resolve their metadata (e.g. `scale_offset`); `validate` only has the
geometry and dtype. Inner codecs see chunks of `chunk_shape`, so that
is the shape and (regular) grid they are validated against, threaded
through any shape-changing inner codec as at the top level.

Parameters
----------
array_spec
Expand All @@ -576,10 +584,13 @@ def evolve_from_array_spec(self, array_spec: ArraySpec) -> Self:
-------
This codec with the evolved code chain.
"""
from zarr.core.chunk_utils import evolve_codecs

shard_spec = self._get_chunk_spec(array_spec)
evolved_codecs = evolve_codecs(self.codecs, shard_spec)
chunk_spec = self._get_chunk_spec(array_spec)
evolved_codecs = evolve_and_validate_codecs(
self.codecs,
shape=self.chunk_shape,
chunk_grid=RegularChunkGridMetadata(chunk_shape=self.chunk_shape),
chunk_spec=chunk_spec,
)
if evolved_codecs != self.codecs:
return replace(self, codecs=evolved_codecs)
return self
Expand Down Expand Up @@ -1586,14 +1597,28 @@ def _get_chunk_spec(self, shard_spec: ArraySpec) -> ArraySpec:
)

def _get_chunks_per_shard(self, shard_spec: ArraySpec) -> tuple[int, ...]:
return tuple(
s // c
for s, c in zip(
shard_spec.shape,
self.chunk_shape,
strict=False,
"""The number of inner chunks along each axis of a shard.

Every encode, decode and size computation goes through here, so this is
also the run-time divisibility check. Metadata validation cannot always
establish divisibility for every shard: after a codec that maps chunk
shapes without declaring a chunk grid on a rectilinear grid, only one
representative shard is checked (see
`zarr.core.metadata.v3.evolve_and_validate_codecs`). Without this check
a non-dividing shard would be floor-divided and read or written with the
wrong layout, silently corrupting data.
"""
if len(shard_spec.shape) != len(self.chunk_shape) or any(
s % c != 0 for s, c in zip(shard_spec.shape, self.chunk_shape, strict=True)
):
raise ValueError(
f"A shard of shape {shard_spec.shape} is not divisible by the shard's inner "
f"chunk shape {self.chunk_shape}. Metadata validation checks this for "
"every shard only when each codec before the sharding codec declares its "
"chunk grid (`resolve_chunk_grid`); otherwise it is detected here, when "
"such a shard is first encoded or decoded."
)
)
return tuple(s // c for s, c in zip(shard_spec.shape, self.chunk_shape, strict=True))

def _shard_index_byte_range(
self, chunks_per_shard: tuple[int, ...]
Expand Down
15 changes: 15 additions & 0 deletions src/zarr/codecs/transpose.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,21 @@ def resolve_metadata(self, chunk_spec: ArraySpec) -> ArraySpec:
prototype=chunk_spec.prototype,
)

def resolve_chunk_grid(
self, *, shape: tuple[int, ...], chunk_grid: ChunkGridMetadata
) -> tuple[tuple[int, ...], ChunkGridMetadata]:
"""Permute the array shape and the per-axis chunk edges by `order`."""
from zarr.core.metadata.v3 import RectilinearChunkGridMetadata, RegularChunkGridMetadata

permuted_shape = tuple(shape[d] for d in self.order)
if isinstance(chunk_grid, RegularChunkGridMetadata):
return permuted_shape, RegularChunkGridMetadata(
chunk_shape=tuple(chunk_grid.chunk_shape[d] for d in self.order)
)
return permuted_shape, RectilinearChunkGridMetadata(
chunk_shapes=tuple(chunk_grid.chunk_shapes[d] for d in self.order)
)

def _decode_sync(
self,
chunk_array: NDBuffer,
Expand Down
17 changes: 9 additions & 8 deletions src/zarr/core/array.py
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@
RegularChunkGridMetadata,
create_chunk_grid_metadata,
parse_node_type_array,
representative_chunk_shape,
)
from zarr.core.sync import sync
from zarr.errors import (
Expand Down Expand Up @@ -239,15 +240,15 @@ def create_codec_pipeline(metadata: ArrayMetadata, *, store: Store | None = None
# `codecs_from_list_unchecked`, so it does not re-emit them.
pipeline = get_pipeline_class().from_codecs(metadata.codecs)

# Use the regular chunk shape if available, otherwise use a
# placeholder. The ChunkTransform is shape-agnostic — the actual
# chunk shape is passed per-call at decode/encode time.
if isinstance(metadata.chunk_grid, RegularChunkGridMetadata):
chunk_shape = metadata.chunk_grid.chunk_shape
else:
chunk_shape = (1,) * len(metadata.shape)
# Evolve against the same representative chunk shape that
# `ArrayV3Metadata.__init__` used, so a codec whose `resolve_metadata`
# depends on the chunk shape (reshape, sharding's inner chain) sees a
# real chunk rather than a placeholder that metadata validation never
# saw. Only evolution is shape-sensitive: the resulting ChunkTransform
# is shape-agnostic — the actual chunk shape is passed per call at
# decode/encode time.
chunk_spec = ArraySpec(
shape=chunk_shape,
shape=representative_chunk_shape(metadata.chunk_grid),
dtype=metadata.data_type,
fill_value=metadata.fill_value,
config=ArrayConfig.from_dict({}),
Expand Down
Loading
Loading