Skip to content

[Sad Catapella B6][Performance] TIFF-to-Zarr conversion reopens headers to read physical calibration already inspected #96

Closed

Description

@guijacquemet

Scope and classification

Sad Catapella Batch 6, pinned d7a538e56d41ac1093ba79adcad397c21b3dfd32. Confirmed redundant metadata/I/O calls, performance impact not measured; source consistency requirements from #90 must remain enforced.

Evidence

Impact

Repeated directory opens, TIFF IFD reads, OME XML parsing and metadata object allocation per converted file. In experiments containing thousands of many-page or remote TIFFs this is avoidable work. Extra non-atomic rereading can also observe newer metadata than the preflight header that determined axis handling, compounding #90.

Fix / acceptance

Use the already inspected immutable header.physical_sizes for conversion metadata, or explicitly invalidate and re-inspect the entire TIFF header when the source changes; never mix metadata epochs. Consider a shared validated header/IFD descriptor for plane_layout and readers if safe under file changes and concurrent workers. Benchmark 100/1k/10k TIFFs (small/many-page/OME XML heavy/local/network), open() counts, elapsed time and peak memory; compare exact NGFF axis scales, units and converted data. No Numba benefit for metadata parsing/I/O.

Activity

  1. guijacquemet commented on Nov 2, 2026

    @guijacquemet
    ContributorAuthor

    Obsolete in v0.4.0: convert_tiffs, zarr_export and OME-Zarr copy reuse were removed in PP-2 (#66, commit 3c2802d; plan question 46). Named-axis copies of images with axis errors are now the OME-TIFF convert step (PP-11, #124, merged as 8db1f4a). Closing as not planned (owner-approved triage, 2026-11-02).

    PP-11 takes sizes from the inspected header once, recorded in the conversion record, and never mixes metadata epochs.


    Generated by Claude Code

  2. guijacquemet commented on Nov 2, 2026

    @guijacquemet
    ContributorAuthor

    Reopening: an independent review of commit 8db1f4a found the closure premature. Not resolved: plan_conversion inspects the headers in preflight, then takes Fingerprint.of() of each source afterwards; _converted() builds the copy's axes, sizes and channel names from the earlier header, while write_named_copy reopens the file. A source changed between header inspection and fingerprint capture is copied with its new pixels but the old header's calibration and channel names, and the before/after fingerprint checks both pass. Fix: bind the header snapshot to the source identity (fingerprint before and after header inspection, refuse if they differ), and check the same fingerprint still holds before and after copying; regression test that mutates the source between inspection and fingerprinting. Tracked in PR PP-11c.


    Generated by Claude Code

  3. guijacquemet commented on Nov 2, 2026

    @guijacquemet
    ContributorAuthor

    Fixed by PR #126, merged as 6ac81ad (fix commit 4312b5a). This closes the correctness part, as the owner decided. The header reads that remain are structural and are listed in the PR.

    I reproduced the race independently:

    • On main 8db1f4a, a source rewritten right after its header is read is converted with the new pixels and the old header's PhysicalSizeZ, so the copy mixes two versions of the file.
    • On the branch, preflight(bind_headers=True) takes a fingerprint before and after each header read, and such an image is refused with source_changed_during_check; nothing is written.
    • Recorded fingerprint: the fingerprint bound to the header is the one recorded as the source's, and it is checked before and after copying.
    • Pixel type: write_named_copy takes the dtype from the bound header and no longer reads the header again (plane_layout). A plane of another type is refused, not cast.

    No performance claim is made: no benchmark has been run.


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions