You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
refactor(cli)!: run apply-inv-tf through iohub's process_single_position, with --resume and per-reconstruction provenance keys - #583
The process_single_position API used here (num_workers, use_threads, resume, resume_token) is already in iohub 0.3.11, so iohub>=0.3.11 is the floor. #476 only changes what a finished unit's record guarantees.
Reconstruction settings move from one nested key to one top-level key per reconstruction. Stores written before this change keep their old key; nothing is migrated.
The key comes from ReconstructionSettings.provenance_key. get_reconstruction_provenance(settings) returns {key: settings} for create_empty_plate(extra_metadata=...), which biahub uses as well.
Who's affected: anyone reading zattrs["waveorder"][<channel names>], the access #547 documented. There are no readers in waveorder, biahub or imaging-qc-pipeline; biahub only copies the key forward, and still copies the legacy nested waveorder key for older stores.
Why one key per reconstruction
It matches biahub's layout. biahub records each processing step's config as its own top-level key, at the same level: biahub-flat_field, biahub-deskew, biahub-concatenate, cytoland. The reconstruction step becomes one more sibling (waveorder-Phase3D) instead of the only step nested under a package key. An assembled v6 position after this change:
It fixes lost provenance in biahub. The nested dict had two writers with different semantics. biahub recorded it at plate creation through create_empty_plate(extra_metadata=...), which overwrites the top-level key; waveorder merged into it after the timepoints. On the biahub path, a second config appending channels therefore wiped the first config's settings (reproduced: after a fluorescence then a phase config, only Phase3D remained). With one key per reconstruction, iohub's overwrite is exactly right and no merge is needed anywhere.
It's written once, at plate creation, before any timepoint runs, like every other step. wo apply-inv-tf records it in create_empty_plate, and apply_inverse_transfer_function_single_position no longer writes metadata per position. Verified: with the per-position step forced to fail, the output already holds waveorder-Phase3D while its data is still all zeros. It also suits --resume: the settings are on disk even if the run is interrupted.
Alternative considered: keep the nested dict, make both writers merge (option A)
iohub's extra_metadata can only overwrite. biahub's init would have to read and merge the existing dict before create_empty_plate, or iohub would need a deep-merge option.
waveorder would keep its per-position read-merge-write after reconstruction, so the settings only land once a position finishes.
The reconstruction step would stay the one step nested under a package key rather than sitting beside the others.
If the schema change is a blocker, option A is the fallback. It fixes the lost-provenance bug without breaking readers, at the cost of the merge code on both sides.
apply_inverse_transfer_function_single_position drove its own per-timepoint loop (apply_inverse_to_zyx_and_save). A reconstruct killed part-way through a position (Slurm preemption, walltime) therefore recomputed every timepoint on retry, unlike flat-field, deskew and assemble. It now runs through iohub's process_single_position, and gains --resume. The reconstruction's provenance moves to one top-level key per reconstruction, recorded at plate creation (breaking, see above).
Changes
cli/utils.py:
apply_inverse_czyx, a module-level numpy adapter. iohub reads each CZYX volume once and hands it over as numpy. The adapter labels it with the input's coordinates (taken once from Position.to_xarray(), so they come from the same store as the data), calls the waveorder.api function unchanged, and returns .values. It's module-level so it pickles into iohub's spawn pool.
reconstruction_fingerprint, the resume token (see below).
apply_inverse_to_zyx_and_save and waveorder's _check_nan_n_zeros are deleted.
cli/apply_inverse_transfer_function.py:
Channels are resolved by name, in config order, as one group, against the input and the output store. So several configs writing different channel groups into one plate (Namespace reconstruction settings by output channel names #547) still write their own channels.
Timepoints are addressed by index, passed as both input and output indices, since the output plate has the input's full T.
The partial and executor code is replaced by process_single_position(apply_inverse_czyx, ..., num_workers=num_processes, resume=..., resume_token=..., model_function=..., czyx_coords=..., **apply_inverse_args).
The per-position zattrs["waveorder"] merge-write after the timepoints is removed. Provenance is recorded at plate creation instead (see the breaking-change section).
get_reconstruction_provenance(settings): {settings.provenance_key: settings.model_dump()}. apply_inverse_transfer_function_cli passes it to create_empty_plate(extra_metadata=...).
cli/settings.py:ReconstructionSettings.provenance_key, e.g. waveorder-Phase3D, waveorder-Birefringence, waveorder-Birefringence,Phase3D, waveorder-GFP_Density3D.
--resume on wo apply-inv-tf and wo reconstruct, and resume=False on apply_inverse_transfer_function_cli / ..._single_position (default off, so biahub's call is unaffected).
The token hashes three things: the reconstruction settings (excluding time_indices, which selects timepoints rather than changing them), the settings stored in the transfer-function store, and the transfer function's shapes. Resuming after a settings change or a recomputed transfer function recomputes instead of silently reusing stale timepoints.
Resume needs an OME-Zarr v0.5 output. For v0.4 the CLI says so, since it silences iohub's UserWarning, and recomputes everything.
pyproject.toml:iohub>=0.3.11, plus the [tool.uv.sources] pin to iohub#476 (see merge order).
The transfer function is still passed with every task as torch tensors. import torch registers torch's pickling reductions on the stdlib ForkingPickler, so they cross iohub's stdlib spawn pool as a shared-memory handle. I checked this against iohub's exact pool pattern: 0.7 MB of pipe traffic for 32 tasks carrying a 1 GB tensor.
Each volume is read once. iohub reads it with oindex into numpy, checks it, then calls the adapter, so there's no dask array to recompute.
Behaviour changes
Skip rule (deliberate): a timepoint is now skipped when any input channel is all zeros or NaNs (iohub's rule), not only when all channels are. This only affects multi-channel (birefringence) inputs. A missing polarization state makes that reconstruction meaningless anyway. Decided in review of the proposal; worth a changelog note.
Bug fix: the old write mapped the input's t coordinate back to an index through the output's transform. For an input with a T translation, every timepoint was written to a shifted (or out-of-bounds) index. Addressing by index fixes this by construction.
test_append_channel_reconstruction (birefringence, then fluorescence into one plate) now checks waveorder-Birefringence and waveorder-GFP_Density3D against each config, and that no legacy waveorder key is written. New test_provenance_key[...] covers each reconstruction mode.
test_apply_inv_tf_addresses_timepoints_by_index: a time_indices subset on an input with a T translation lands on the right indices (the old code shifted them).
test_apply_inv_tf_writes_its_channels_by_name: writing Phase3D into a plate whose channel 0 belongs to another config leaves channel 0 untouched.
test_apply_inv_tf_skips_a_timepoint_with_a_blank_channel: the new skip rule, on a birefringence input.
test_apply_inv_tf_resume: resume skips finished timepoints. Without --resume, everything is recomputed. A settings change, or a transfer function recomputed with other settings, recomputes despite --resume.
test_apply_inv_tf_process_pool_matches_serial is kept: 2 spawned workers give output identical to the serial path.
Benchmark
wo apply-inv-tf -j 16 on one v6 position (C/2/000002), time_indices 1–16, TF with the correct pixel sizes. All runs are in one SLURM job on one node (cpu-g-2, 16 CPUs), alternating #582's head and this PR, both with iohub#476. The measurement method is the same as in #582: whole-process-tree I/O and CPU deltas; storage reads = rchar − pipe traffic; PSS for memory.
0 (the TF still crosses as a shared-memory handle)
storage reads
11.2 GB
11.2 GB (each volume still read once)
storage writes
9.6 GB
9.6 GB
peak PSS, process tree
106–117 GB
113–117 GB
output
bit-identical (16/16 time points, both repeats)
Moving to iohub's loop, with its per-unit completion markers and #476's fsync, costs nothing measurable (≤ 1 s of wall time, within noise).
Resume, rerun with --resume into a finished output: all 16 units skipped ("already written"), in 13.9 s, 65 s of CPU, 3.6 GB read (mostly the TF load and the completion probes) and 0 bytes written. A full run takes ~44 s, 510 s of CPU and 9.6 GB written.
apply_inverse_transfer_function_single_position bound the loaded
transfer function into the per-time-point partial. ProcessPoolExecutor
pickles every task's arguments, so the full transfer function was
serialised by the parent and deserialised by a worker once per time
point, and converted to tensors (with a copy) on every call. For a 3D
phase transfer function of (181, 768, 794) complex64 pairs (~1.8 GB),
768 time points meant ~1.36 TB of pipe traffic per position and ~30 GB
of queued pickles in the parent.
Each pool worker now loads the transfer function once in an initializer,
converts it to torch tensors, and passes them to every time point it
runs. The serial path loads it once in-process. _to_tensor returns
already-converted tensors as-is; xr.Dataset callers are unchanged.
The executor type, spawn context, and thread settings are unchanged.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
Loading the transfer function in every worker removed the per-time-point
pickling but read the store once per worker and kept one copy per worker
in RAM (16 x 3 GB for a v6 3D phase TF).
The parent now loads it once, moves the tensors into shared memory with
share_memory_(), and passes them as the pool initializer's argument.
torch.multiprocessing's pickling reductions send a shared tensor as a
handle to its memory, so every worker maps the same copy: one storage
read, one copy in RAM, and nothing large through the worker pipes. A
missing transfer-function key again fails in the parent, before the pool
starts.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
…nction
The explicit share_memory_() was redundant: importing torch.multiprocessing
registers pickling reductions that move a pickled CPU tensor into shared
memory and send a handle to it, which already happens when the tensors are
passed as the pool initializer's argument. Drop the call and document the
mechanism, including why it only applies once the arrays are torch tensors.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
torch.multiprocessing's pickling reductions share CPU tensors for any
ForkingPickler route, including the executor's per-task call queue, so a
tensor transfer function bound into each time point's partial already
crosses as a small shared-memory handle. The pool initializer, the
module-level worker global, and the task wrapper are no longer needed; the
executor code is back to main's.
_load_transfer_function_dataset becomes _load_transfer_function and
returns the tensors directly instead of an xr.Dataset of numpy arrays that
was converted afterwards. The public apply_inverse_transfer_function type
hints and docstrings now say they accept a mapping of tensors.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
apply_inverse_to_zyx_and_save called `.values` on the dask-backed input
slice twice, once for the NaN/zero check and again inside the model, so
every time point was read and decompressed from the store twice (0.48 GB
extra per time point on a v6 A549 position). Load the slice once.
_to_tensor copied every xr.Dataset variable before wrapping it. The
apply-inverse models never modify the transfer function in place, so wrap
it with torch.from_numpy directly and share its memory.
Tests: the input is read once per time point (fails with the old code),
and apply leaves an xr.Dataset transfer function unchanged while sharing
its memory, for phase and fluorescence.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
…on, with --resume
apply_inverse_transfer_function_single_position drove its own per-timepoint
loop through apply_inverse_to_zyx_and_save, so a reconstruct killed part-way
through a position (preemption, walltime) recomputed every timepoint on
retry, unlike flat-field, deskew and assemble. Run it through iohub's
process_single_position instead, as proposed in #574:
- apply_inverse_czyx is a module-level numpy adapter: iohub reads each CZYX
volume once and hands it over as numpy; the adapter labels it with the
input's coordinates (taken once from Position.to_xarray()), calls the
waveorder.api function unchanged, and returns its values.
- Channels are resolved by name, in config order, as one group, against the
input and the output stores, so configs writing different channel groups
into one plate still land on their own channels.
- Timepoints are addressed by index. The old write mapped the input's t
coordinate back through the output's transform, which shifted every
timepoint of an input with a T translation.
- --resume (wo apply-inv-tf, wo reconstruct; resume= on the Python functions,
default False) skips timepoints a previous interrupted run finished. The
resume token fingerprints the reconstruction settings, the settings the
transfer function was computed with, and its shapes, so a settings change
or a recomputed transfer function recomputes instead of reusing stale data.
An OME-Zarr v0.4 output cannot track progress; the CLI says so.
The transfer function is still passed as torch tensors with every task, so
it crosses iohub's spawn pool as a shared-memory handle, and each volume is
still read once.
Behaviour change: a timepoint is now skipped when any input channel is all
zeros or NaNs (iohub's rule), not only when all channels are. This only
matters for multi-channel (birefringence) inputs; a missing polarization
state makes that reconstruction meaningless anyway.
Requires iohub>=0.3.11 for the API; pinned via tool.uv.sources to
czbiohub-sf/iohub#476, which syncs a unit's shards before recording it as
done, until iohub releases it.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
…at plate creation
BREAKING CHANGE: reconstruction settings move from one nested key,
zattrs["waveorder"][<output channel names>], to one top-level key per
reconstruction, zattrs["waveorder-<group>"], e.g. waveorder-Phase3D,
waveorder-GFP_Density3D, waveorder-Birefringence (previously keyed
"Retardance,Orientation,Transmittance,Depolarization") and
waveorder-Birefringence,Phase3D. Existing stores keep the old key.
Why: the nested dict had two writers with different semantics. biahub
records it at plate creation through create_empty_plate(extra_metadata=...),
which overwrites the whole top-level key, and waveorder merged into it after
the timepoints. A second config appending channels to a plate therefore
dropped the first config's settings on the biahub path (#547's multi-group
provenance). With one key per reconstruction, iohub's overwrite is exactly
right, no merge is needed anywhere, and each reconstruction sits beside the
biahub-<step> keys of the other processing steps.
- ReconstructionSettings.provenance_key names the key;
get_reconstruction_provenance(settings) returns {key: settings} for
create_empty_plate(extra_metadata=...), which biahub uses too.
- apply_inverse_transfer_function_cli records it when creating the plate;
apply_inverse_transfer_function_single_position writes no metadata.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
ieivanov
added a commit
to czbiohub-sf/biahub
that referenced
this pull request
Sep 26, 2026
…r reconstruction
BREAKING CHANGE: reconstruct output positions record their settings under
top-level zattrs["waveorder-<group>"] keys (waveorder-Phase3D,
waveorder-Birefringence, waveorder-GFP_Density3D, ...) instead of
zattrs["waveorder"][<output channel names>], following
mehta-lab/waveorder#583.
_init_output_plate recorded {"waveorder": {group: settings}} through
create_empty_plate(extra_metadata=...), which overwrites the whole top-level
key, so a second config appending channels to a plate dropped the first
config's settings. It now records waveorder's own
get_reconstruction_provenance(settings): one key per reconstruction, beside
the biahub-<step> keys, so each config overwrites only its own entry.
PROVENANCE_METADATA_KEYS carries waveorder-* forward, and keeps the nested
waveorder key so older stores still carry theirs. The waveorder pin moves to
the #583 commit with this change.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
ieivanov
changed the title
refactor(cli): run apply-inv-tf through iohub's process_single_position, with --resume
refactor(cli)!: run apply-inv-tf through iohub's process_single_position, with --resume and per-reconstruction provenance keys
Sep 26, 2026
This aligns the metadata schema with biahub, writing one field per reconstruction steps (in this example Birefringence and GFP_Density3D fluor deconvolution are considered separate reconstruction steps, even if they are passed through the same waveorder yaml file) and allow us to reuse some code. We could also keep the current schema if you prefer.
Once we agree on that part, we can review the rest of the PR.
I don't see a problem with the change in change in metadata schema. Does this help WaveOrder align closer to the DCA specs (do they have anything any best practices w.r.t this)?
Does this help WaveOrder align closer to the DCA specs (do they have anything any best practices w.r.t this)
Not really - DCA spec so far is about what kind of sample the data is for (what cells, what markers, what perturbations, etc.) rather than what kind of processing was performed, so not relevant here.
The change in the schema is largely motivated by aligning to the biahub schema and reusing some of our existing code. This would be a breaking change for anyone reading that metadata programatically. Do you know of any such use cases?
… metadata
reconstruct rewrites the transfer function metadata from the full config on
every run, so time_indices leaked into apply-inv-tf's resume token and a new
time selection recomputed already-finished timepoints. Exclude time_indices
and input_channel_names, which select data rather than shape the transfer
function; both remain in the reconstruction settings part of the token.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
[tool.uv.sources] is only honored by uv, while this standard dependency is what pip uses (and pip is the documented installation path). A pip/VCS installation can therefore resolve iohub 0.3.11 without the shard-fsync fix that this PR says is required for trustworthy completion records, allowing --resume to skip data whose write was not durable. Require the patched commit/release in standard package metadata as well, or gate resume until an iohub release containing the fix is installed.
✅ All modified and coverable lines are covered by tests. ⚠️ Please upload report for BASE (perf/load-tf-once-per-worker@097ff97). Learn more about missing BASE report.
Conflicts with #582 (this branch's earlier perf commits, squash-merged)
resolved in favor of this branch, which supersedes them. Drops main's
test for apply_inverse_to_zyx_and_save, which this branch removed.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
Brings in #578 (auto_regularization). Test conflicts resolved by keeping
both sides' additions.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
For now, let's keep iohub pinned to a commit hash on main. I'm working with @srivarra thru a couple more changes to iohub. When we are through we'll create a release and I'll open a waveorder PR to pin the new iohub version. Merging now.
A reconstruct task preempted or killed at walltime recomputed every
timepoint of its position on retry; flat-field, deskew and assemble resume
per unit. waveorder now runs apply-inv-tf through iohub's
process_single_position and takes resume= (mehta-lab/waveorder#583), so:
- `biahub apply-inv-tf` and `biahub reconstruct` take @resume()
(--resume/--no-resume, default off) and pass it to
apply_inverse_transfer_function_single_position. waveorder computes the
resume token itself, from the reconstruction settings and the transfer
function, so a changed config or a recomputed transfer function
recomputes instead of being skipped.
- The reconstruct Nextflow module passes --resume, as deskew, flat-field and
assemble do.
- waveorder is pinned to #583 (which also carries #582's shared transfer
function) until a release contains both.
- The CLI-consistency canon lists @resume() in the decorator stack and says
which commands take it.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
ieivanov
added a commit
to czbiohub-sf/biahub
that referenced
this pull request
Oct 16, 2026
…r reconstruction
BREAKING CHANGE: reconstruct output positions record their settings under
top-level zattrs["waveorder-<group>"] keys (waveorder-Phase3D,
waveorder-Birefringence, waveorder-GFP_Density3D, ...) instead of
zattrs["waveorder"][<output channel names>], following
mehta-lab/waveorder#583.
_init_output_plate recorded {"waveorder": {group: settings}} through
create_empty_plate(extra_metadata=...), which overwrites the whole top-level
key, so a second config appending channels to a plate dropped the first
config's settings. It now records waveorder's own
get_reconstruction_provenance(settings): one key per reconstruction, beside
the biahub-<step> keys, so each config overwrites only its own entry.
PROVENANCE_METADATA_KEYS carries waveorder-* forward, and keeps the nested
waveorder key so older stores still carry theirs. The waveorder pin moves to
the #583 commit with this change.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
ieivanov
added a commit
to czbiohub-sf/biahub
that referenced
this pull request
Oct 16, 2026
mehta-lab/waveorder#583 is on waveorder main as 8d79650, so move the pin
off the PR branch commit. waveorder's own uv sources pin iohub to b7bc648
(the #476 merge) while biahub needs d1f9e89 (adds #479); uv refuses two
git URLs for one package, so override iohub to biahub's rev for the whole
graph. Relock.
Co-Authored-By: Claude Fable 5.1 <[email protected]>
ieivanov
added a commit
to czbiohub-sf/biahub
that referenced
this pull request
Oct 16, 2026
…ovenance key per reconstruction (#387)
* feat(reconstruct): resume interrupted apply-inv-tf positions
A reconstruct task preempted or killed at walltime recomputed every
timepoint of its position on retry; flat-field, deskew and assemble resume
per unit. waveorder now runs apply-inv-tf through iohub's
process_single_position and takes resume= (mehta-lab/waveorder#583), so:
- `biahub apply-inv-tf` and `biahub reconstruct` take @resume()
(--resume/--no-resume, default off) and pass it to
apply_inverse_transfer_function_single_position. waveorder computes the
resume token itself, from the reconstruction settings and the transfer
function, so a changed config or a recomputed transfer function
recomputes instead of being skipped.
- The reconstruct Nextflow module passes --resume, as deskew, flat-field and
assemble do.
- waveorder is pinned to #583 (which also carries #582's shared transfer
function) until a release contains both.
- The CLI-consistency canon lists @resume() in the decorator stack and says
which commands take it.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
* feat(reconstruct)!: record reconstruction provenance under one key per reconstruction
BREAKING CHANGE: reconstruct output positions record their settings under
top-level zattrs["waveorder-<group>"] keys (waveorder-Phase3D,
waveorder-Birefringence, waveorder-GFP_Density3D, ...) instead of
zattrs["waveorder"][<output channel names>], following
mehta-lab/waveorder#583.
_init_output_plate recorded {"waveorder": {group: settings}} through
create_empty_plate(extra_metadata=...), which overwrites the whole top-level
key, so a second config appending channels to a plate dropped the first
config's settings. It now records waveorder's own
get_reconstruction_provenance(settings): one key per reconstruction, beside
the biahub-<step> keys, so each config overwrites only its own entry.
PROVENANCE_METADATA_KEYS carries waveorder-* forward, and keeps the nested
waveorder key so older stores still carry theirs. The waveorder pin moves to
the #583 commit with this change.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
* build(deps): pin waveorder to the merged mehta-lab/waveorder#583mehta-lab/waveorder#583 is on waveorder main as 8d79650, so move the pin
off the PR branch commit. waveorder's own uv sources pin iohub to b7bc648
(the #476 merge) while biahub needs d1f9e89 (adds #479); uv refuses two
git URLs for one package, so override iohub to biahub's rev for the whole
graph. Relock.
Co-Authored-By: Claude Fable 5.1 <[email protected]>
---------
Co-authored-by: Claude Opus 5.5 (1M context) <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ieivanov
changed the title
talonchandler
left a comment
Merge order and dependencies
This is a stacked PR. Merge in this order:
--resumeto be trustworthy. This PR pins iohub to tests: run example tests on ci and mock show #476's commit via[tool.uv.sources].main.[tool.uv.sources]pin with a PyPI floor (iohub>=<that release>).--resumefrombiahub apply-inv-tf,biahub reconstructand the reconstruct Nextflow module, as deskew and flat-field do. It pins waveorder to this PR's commit until a release. It's stacked on build(deps): pin iohub to the fsync-before-marker fix czbiohub-sf/biahub#345, the biahub side of the iohub#476 pin.The
process_single_positionAPI used here (num_workers,use_threads,resume,resume_token) is already in iohub 0.3.11, soiohub>=0.3.11is the floor. #476 only changes what a finished unit's record guarantees.Reconstruction settings move from one nested key to one top-level key per reconstruction. Stores written before this change keep their old key; nothing is migrated.
Before (#547): output position zattrs
After
zattrs["waveorder"][...]"Phase3D"/"Phase2D""waveorder-Phase3D"/"waveorder-Phase2D""GFP_Density3D""waveorder-GFP_Density3D""Retardance,Orientation,Transmittance,Depolarization""waveorder-Birefringence"(shortened)"Retardance,…,Phase3D,Retardance_Joint_Decon,Orientation_Joint_Decon,Phase_Joint_Decon""waveorder-Birefringence,Phase3D"(joint-decon channels implied)The key comes from
ReconstructionSettings.provenance_key.get_reconstruction_provenance(settings)returns{key: settings}forcreate_empty_plate(extra_metadata=...), which biahub uses as well.Who's affected: anyone reading
zattrs["waveorder"][<channel names>], the access #547 documented. There are no readers in waveorder, biahub or imaging-qc-pipeline; biahub only copies the key forward, and still copies the legacy nestedwaveorderkey for older stores.Why one key per reconstruction
biahub-flat_field,biahub-deskew,biahub-concatenate,cytoland. The reconstruction step becomes one more sibling (waveorder-Phase3D) instead of the only step nested under a package key. An assembled v6 position after this change:{ "ome": {...}, "biahub-flat_field": {...}, "biahub-deskew": {...}, "waveorder-Phase3D": {...}, "cytoland": {...}, "biahub-concatenate": {...} }create_empty_plate(extra_metadata=...), which overwrites the top-level key; waveorder merged into it after the timepoints. On the biahub path, a second config appending channels therefore wiped the first config's settings (reproduced: after a fluorescence then a phase config, onlyPhase3Dremained). With one key per reconstruction, iohub's overwrite is exactly right and no merge is needed anywhere.wo apply-inv-tfrecords it increate_empty_plate, andapply_inverse_transfer_function_single_positionno longer writes metadata per position. Verified: with the per-position step forced to fail, the output already holdswaveorder-Phase3Dwhile its data is still all zeros. It also suits--resume: the settings are on disk even if the run is interrupted.Alternative considered: keep the nested dict, make both writers merge (option A)
extra_metadatacan only overwrite. biahub's init would have to read and merge the existing dict beforecreate_empty_plate, or iohub would need a deep-merge option.If the schema change is a blocker, option A is the fallback. It fixes the lost-provenance bug without breaking readers, at the cost of the merge code on both sides.
Summary
Closes #574, following the path proposed in #574 (comment).
apply_inverse_transfer_function_single_positiondrove its own per-timepoint loop (apply_inverse_to_zyx_and_save). A reconstruct killed part-way through a position (Slurm preemption, walltime) therefore recomputed every timepoint on retry, unlike flat-field, deskew and assemble. It now runs through iohub'sprocess_single_position, and gains--resume. The reconstruction's provenance moves to one top-level key per reconstruction, recorded at plate creation (breaking, see above).Changes
cli/utils.py:apply_inverse_czyx, a module-level numpy adapter. iohub reads each CZYX volume once and hands it over as numpy. The adapter labels it with the input's coordinates (taken once fromPosition.to_xarray(), so they come from the same store as the data), calls thewaveorder.apifunction unchanged, and returns.values. It's module-level so it pickles into iohub's spawn pool.reconstruction_fingerprint, the resume token (see below).apply_inverse_to_zyx_and_saveand waveorder's_check_nan_n_zerosare deleted.cli/apply_inverse_transfer_function.py:partialand executor code is replaced byprocess_single_position(apply_inverse_czyx, ..., num_workers=num_processes, resume=..., resume_token=..., model_function=..., czyx_coords=..., **apply_inverse_args).zattrs["waveorder"]merge-write after the timepoints is removed. Provenance is recorded at plate creation instead (see the breaking-change section).get_reconstruction_provenance(settings):{settings.provenance_key: settings.model_dump()}.apply_inverse_transfer_function_clipasses it tocreate_empty_plate(extra_metadata=...).cli/settings.py:ReconstructionSettings.provenance_key, e.g.waveorder-Phase3D,waveorder-Birefringence,waveorder-Birefringence,Phase3D,waveorder-GFP_Density3D.--resumeonwo apply-inv-tfandwo reconstruct, andresume=Falseonapply_inverse_transfer_function_cli/..._single_position(default off, so biahub's call is unaffected).time_indices, which selects timepoints rather than changing them), the settings stored in the transfer-function store, and the transfer function's shapes. Resuming after a settings change or a recomputed transfer function recomputes instead of silently reusing stale timepoints.UserWarning, and recomputes everything.pyproject.toml:iohub>=0.3.11, plus the[tool.uv.sources]pin to iohub#476 (see merge order).What #582 bought is preserved:
import torchregisters torch's pickling reductions on the stdlibForkingPickler, so they cross iohub's stdlib spawn pool as a shared-memory handle. I checked this against iohub's exact pool pattern: 0.7 MB of pipe traffic for 32 tasks carrying a 1 GB tensor.oindexinto numpy, checks it, then calls the adapter, so there's no dask array to recompute.Behaviour changes
Testing
tests/api_tests+tests/cli_tests: 197 passed, 1 skipped, 2 xfailed, against iohub#476.test_append_channel_reconstruction(birefringence, then fluorescence into one plate) now checkswaveorder-Birefringenceandwaveorder-GFP_Density3Dagainst each config, and that no legacywaveorderkey is written. Newtest_provenance_key[...]covers each reconstruction mode.test_apply_inv_tf_addresses_timepoints_by_index: atime_indicessubset on an input with a T translation lands on the right indices (the old code shifted them).test_apply_inv_tf_writes_its_channels_by_name: writingPhase3Dinto a plate whose channel 0 belongs to another config leaves channel 0 untouched.test_apply_inv_tf_skips_a_timepoint_with_a_blank_channel: the new skip rule, on a birefringence input.test_apply_inv_tf_resume: resume skips finished timepoints. Without--resume, everything is recomputed. A settings change, or a transfer function recomputed with other settings, recomputes despite--resume.test_apply_inv_tf_resume_on_v04_output_recomputes_and_says_so.test_apply_inverse_czyx_labels_the_volume_for_the_model.test_apply_inv_tf_process_pool_matches_serialis kept: 2 spawned workers give output identical to the serial path.Benchmark
wo apply-inv-tf -j 16on one v6 position (C/2/000002),time_indices1–16, TF with the correct pixel sizes. All runs are in one SLURM job on one node (cpu-g-2, 16 CPUs), alternating #582's head and this PR, both with iohub#476. The measurement method is the same as in #582: whole-process-tree I/O and CPU deltas; storage reads =rchar − pipe traffic; PSS for memory.Moving to iohub's loop, with its per-unit completion markers and #476's fsync, costs nothing measurable (≤ 1 s of wall time, within noise).
Resume, rerun with
--resumeinto a finished output: all 16 units skipped ("already written"), in 13.9 s, 65 s of CPU, 3.6 GB read (mostly the TF load and the completion probes) and 0 bytes written. A full run takes ~44 s, 510 s of CPU and 9.6 GB written.🤖 Generated with Claude Code