Skip to content

Handle stacked MultipartRelatedConsolidator with fpp > 1 - #67

Merged
genematx merged 5 commits into
bluesky:mainfrom
thopkins32:fix-multipart-bug
Oct 14, 2026
Merged

genematx merged 5 commits into
bluesky:mainfrom
thopkins32:fix-multipart-bug

Conversation

@thopkins32

@thopkins32 thopkins32 commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Assisted-by: oh-my-pi:gpt-5.6-sol.

Closes #66

…reads

Collapse the two stacked-layout helpers into a single `_derive_frames_per_file`
that is the one source of truth for how many frames each file holds. Both
`files_per_datum` (= `datum_shape[0] // frames_per_file`) and the physical
`orig_chunks` layout now derive from it, so they can no longer disagree.

This fixes sub-array reads for the legacy file-per-frame `multiplier`
(`frame_per_point`) layout: previously that path set `files_per_datum` but not
`orig_chunks`, so `read(0)` on a stacked multi-file datum raised a 422 reshape
error. `orig_chunks` is now recorded for every stacked datum spanning more than
one file (single-frame, multi-page, and legacy multiplier alike).

Also rename `_recompute_files_per_datum` -> `_derive_files_per_datum`.

Extend the stacked-TIFF writer test with a `legacy-multiplier` case so full and
sub-array reads are value-checked for all three layouts.
@genematx

genematx commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

When the default join_method changed from concat to stack, MultipartRelatedConsolidator stopped deriving files_per_datum from chunk_shape and fell back to frame_per_point (default 1). Multi-frame image sequences (e.g. ophyd-async areaDetector TIFF with NumImages > 1) therefore registered a single file per datum, so the declared stacked shape (N, frames, H, W) could not be filled and reads raised a reshape error.

Fix: introduce a single _derive_frames_per_file() helper — the one source of truth for how many frames each file holds (chunk_shape[0] for concat and rank-aligned stacked chunks; 1 for the legacy file-per-frame multiplier/frame_per_point layout; the whole datum otherwise). files_per_datum is then datum_shape[0] // _derive_frames_per_file(), and the physical file layout is recorded in orig_chunks from the same helper. Because both derive from one place they can't disagree, which also fixes sub-array reads (read(0), read((0,1))): previously the legacy multiplier path set files_per_datum but not orig_chunks, so slicing a stacked multi-file datum raised a 422 reshape error. Concat, single- and multi-page rank-aligned stack, and the legacy file-per-frame multiplier cases are all correct for full and sliced reads.

TST: add a parametrized consolidator test (single/2/3-frame, non-divisor, inner-shape-mismatch, legacy multiplier) and a writer test that value-checks full and sub-array reads for single-frame, multi-page, and legacy-multiplier layouts; fix a vacuous assertion in test_tiff_and_jpeg_chunks that never checked the stack file count.

genematx and others added 2 commits October 7, 2026 05:55
An explicit `files_per_datum` StreamResource parameter is authoritative, but
`_derive_frames_per_file` was still inferring from `chunk_shape[0]`, so
`orig_chunks` could violate the invariant
`files_per_datum * frames_per_file == datum_shape[0]` (e.g. datum_shape (6, H, W),
chunk_shape (1, H, W), files_per_datum=2 wrongly yielded frames_per_file=1 instead
of 3).

Back-derive frames_per_file from an explicit files_per_datum, and skip orig_chunks
when files_per_datum does not divide datum_shape[0] (the previous guard, dropped in
the unification, used to protect this). Add a regression test for the explicit
files_per_datum override under stack.
Unify multipart frames-per-file derivation and fix legacy multiplier reads

@genematx genematx left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Awaiting Tiled v0.2.19 release before merging.

@codecov

codecov Bot commented Oct 14, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.15385% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 78.53%. Comparing base (2d1d76a) to head (76b39a3).

Files with missing lines Patch % Lines
src/bluesky_tiled_plugins/writing/consolidators.py 96.15% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #67      +/-   ##
==========================================
+ Coverage   78.38%   78.53%   +0.15%     
==========================================
  Files          16       16              
  Lines        2387     2409      +22     
==========================================
+ Hits         1871     1892      +21     
- Misses        516      517       +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@genematx
genematx merged commit 6ab4c6a into bluesky:main Oct 14, 2026
18 of 24 checks passed
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.

join method "stack" handling registers too few TIFF assets when one datum contains multiple single-frame files

2 participants