Skip to content

feat(sharding): lift constraint on evenly divisible subchunk shapes - #370

Open
LDeakin wants to merge 1 commit into
zarr-developers:mainfrom
LDeakin:sharding_lift_constraint
Open

LDeakin wants to merge 1 commit into
zarr-developers:mainfrom
LDeakin:sharding_lift_constraint

Conversation

@LDeakin

@LDeakin LDeakin commented Dec 3, 2025

Copy link
Copy Markdown
Member

@normanrz

normanrz commented Dec 4, 2025

Copy link
Copy Markdown
Member

Thanks! I can see the rationale of this change for the rectilinear chunk grid. However, I would favor keeping the strict requirement for regular outer chunk grids. That way, implementations that don't implement the rectilinear grid won't need to change.

@LDeakin

LDeakin commented Dec 4, 2025

Copy link
Copy Markdown
Member Author

I would favor keeping the strict requirement for regular outer chunk grids. That way, implementations that don't implement the rectilinear grid won't need to change.

I'm interested to hear more perspectives here. Personally, I think this change is very useful with regularly gridded arrays, and irregular grids just make the problem far more apparent. For example, say a chunk shape is constrained by a sensor or data pipeline and must be [997, 997, 997]. Well, the inner chunk shape can only be [1, 1, 1] because 997 is prime.

Would it be sufficient in your view to recommend that implementations that add support for this issue a warning that it will not be supported by impls that only support version 1.0 of the sharding codec?

@d-v-b

d-v-b commented Dec 4, 2025

Copy link
Copy Markdown
Contributor

+1 to this change, it seems like a logical extension of how we currently handle the shape / chunk grid relationship for the regular chunk grid.

@srivarra srivarra left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@LDeakin +1 from me. I am in favor of this.

For context we run into these restrictions being rather annoying when dealing with acquisitions from microscopes. We pick chunk and shard shapes for read/write performance and reducing inode count on our HPCs

Since the shard shape has to be a multiple of the inner chunk shape, it rounds up past the array bound. Part of the shard then sits outside the array, so no write ever covers it and every write becomes a RMW. Usually that just costs performance, but it also means we can't repair a half-written shard by rewriting it, since the rewrite has to read the damaged shard first.

We can pick an inner chunk that divides the extent, but then the prime factorization of whatever came off the microscope picks our chunk shape instead of useful chunks for read/write performance (that 997 example). And those dimensions can change every experiment. The shapes that always divide are the ones I don't want, (1,1,1) or the whole array.

I'd like chunking and sharding to be invisible to our users and make "just work" across acquisition geometries, and I think this gets us closer. Thanks for flagging the PR!

Oh, also would this work well with rectilinear grids?

@normanrz

Copy link
Copy Markdown
Member

Thanks for sharing your perspectives. I want to state that I am still skeptical about the complexity that this would add to implementations that optimize around the current constraints in the sharding codec. I think we need more discussion in the ZFWG.

@d-v-b

d-v-b commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

it would definitely be good to see an impact assessment of this change for existing implementations.

@ieivanov

ieivanov commented Aug 7, 2026

Copy link
Copy Markdown

I can't speak to implementation cost, but I can offer a user perspective.

@normanrz, one thing worth weighing: the case @srivarra described is a regular outer chunk grid, so keeping the strict requirement for regular grids and relaxing it only for rectilinear would exclude exactly this workload. We're not reaching for clipped subchunks because we use an irregular grid; we're reaching for them because a regular grid plus the divisibility rule leaves no usable choice.

Concretely, on a light-sheet reconstruction with shape = [67, 3, 1068, 256, 1664]:

  • 1068 = 2²·3·89. The largest divisor ≤ 16 is 12 — and the only shard extents that are both multiples of 12 and divisors of 1068 are 12 and 1068. So the options are a 12-plane shard, or a whole-Z shard of 1.82 GB that has to be held in memory to write.
  • 1664 = 2⁷·13, so 128 works and 256 does not (1664/256 = 6.5).

Round the shard up instead and part of it sits outside the array, so no write ever covers a full shard. Two measured consequences:

  • Every write becomes a read-modify-write. For one 116 MB (t, c) volume: 85 ms when the file doesn't exist, 126 ms merging into an existing one.
  • More importantly it costs recoverability. A shard left truncated by a preempted job cannot be repaired by rewriting it, because the rewrite reads it back first and fails its checksum — deterministically, so retries never converge. We reproduced both sides: with the rounded-up grid, rewriting a torn shard raises the checksum is invalid; with a grid that tiles the array exactly, the identical rewrite succeeds.

We've worked around it by deleting the shards a write owns before writing (czbiohub-sf/iohub#455), which is fine but is a workaround every writer would have to reinvent.

To be fair about scope: that recoverability problem is fixable in implementations, by treating a write that covers a boundary shard's full in-bounds region as complete — no spec change needed, and zarr-developers/zarr-python#757 already did the equivalent for unsharded chunks. What that doesn't fix is the first problem: when an extent factors badly, the spec forbids the subchunk shape you actually want. That part needs #370.

@jbms

jbms commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

For recovering from failed partial writes, you should write to a temp filename and rename when complete.

If you want your shard shape to exactly divide your overall shape you will still be quite constrained.

Allowing dimensions to be marked non-resizable would be one possibly more elegant solution to avoiding extra overhead on boundary chunks.

Still the overhead you mention --- the 126ms is avoided by using a temporary file, and how does the 85ms compare to the case when you don't need to check for an existing file?

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.

6 participants