Skip to content

Give each plate field of view its own pool slots - #234

Open
nclack wants to merge 4 commits into
acquire-project:mainfrom
nclack:hcs-field-slots
Open

nclack wants to merge 4 commits into
acquire-project:mainfrom
nclack:hcs-field-slots

Conversation

@nclack

@nclack nclack commented Aug 25, 2026

Copy link
Copy Markdown
Member

A plate shared one shard pool across every field of view but sized it for a
single field, so the slot indices aliased. Every field's multiscale started its
slot range at 0, and a shard picks its slot as the range base plus the shard
index, so two fields landed on the same slot. A slot holds the open file, so the
second field's open closed the first field's file and took its place, and the
first field's next write went into the second field's file at the first field's
offset.

The plate still shares one pool, which is what keeps it on one io queue. The
pool is now sized for every field, and each field gets the slots at its own
offset, computed from the same index the plate uses to store the field. Summing
a multiscale's slots moved into ngff_multiscale_slot_count, so the count that
spaces the ranges and the count that advances slots within a field are one
computation. The shard pool header now states that callers sharing a pool need
slot ranges that do not overlap.

A new case in test_hcs opens a shard in two fields of the same well at once,
writes different bytes through each, and reads both files back.

Closes #211

@codecov

codecov Bot commented Aug 25, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 77.35849% with 12 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/ngff/ngff_multiscale.c 73.52% 2 Missing and 7 partials ⚠️
src/hcs/hcs.c 84.21% 1 Missing and 2 partials ⚠️
Files with missing lines Coverage Δ
src/hcs/hcs.c 59.19% <84.21%> (-0.99%) ⬇️
src/ngff/ngff_multiscale.c 58.00% <73.52%> (-0.54%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@nclack

nclack commented Aug 25, 2026

Copy link
Copy Markdown
Member Author

Rationale, alternatives, and verification.

One pool, not one per field. A pool per field of view would give a 96-well plate
with four fields 384 worker pools and 384 queued-byte budgets, so widening the
one pool was the only option worth taking.

Ranges keyed on the field index. The plate already indexes its fields by
fov_index, so reusing that index as the range number makes disjointness plain:
distinct fields have distinct indices, and the ranges are slots_per_fov apart.
The alternative, a running counter over the wells the mask leaves active, sizes
the pool tighter but detaches the range from the array index and turns a fully
masked plate into a create failure, since shard_pool_fs_create wants at least
one slot. The cost of the looser sizing is slots reserved for masked-off wells: a
struct fs_slot is 80 bytes, so a 1536-well plate with four fields and seven
slots per field reserves 3.3 MiB.

One level walk. The first draft summed shard counts straight off the LOD plan and
took the number of append dimensions from the original dimensions, while the
create loop derives that per level from the level's own chunk sizes. Those can
disagree, and a disagreement would silently overlap two fields' ranges, which is
the same failure this PR fixes. Both paths now go through level_dimensions and
dims_slot_count.

Verification. The new case was mutation-checked three ways on a CPU node: with
slot_base forced back to 0, with the stride set to 1, and with the stride off
by one. All three make it fail at the writer-distinctness check, and backing the
fix out entirely also leaves the first field's shard file empty at zero bytes.

Not addressed here. Shard files no longer recycle across fields, so the files a
plate holds open at once now scale with the plate rather than with one field; a
driver keeping a shard open in every field of a 96-well four-field plate would
pass the usual descriptor limit and pool_fs_open would start failing. Bounding
the live handles needs a slot-recycling policy in the pool. pool_s3_flush and
pool_s3_wait_fence also walk every slot, which a plate-wide pool makes 25 us at
43k slots instead of nothing, on a path that runs once per aggregate batch; only
S3-backed plates are affected.

The deeper fix, worth doing next, is to make the pool vend a reserved range as a
value type instead of taking a slot_base integer on trust. With that, sizing
the pool for one field would have failed hcs_plate_create outright and turned
the existing plate tests red the day this bug landed.

This branch has not been deployed

No deployments
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.

Plate fields share pool slots and overwrite each other's shard files

1 participant