feat: multi-plane recording — N z-planes per FOV (stacks on #564)#579
Open
hongquanli wants to merge 18 commits into
Open
feat: multi-plane recording — N z-planes per FOV (stacks on #564)#579hongquanli wants to merge 18 commits into
hongquanli wants to merge 18 commits into
Conversation
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…onfig Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ecordZStackWorker Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… at Nz=1 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…mera/Z on fail-fast path; harden validate-wiring test Final whole-branch review findings on feat/multi-plane-recording: - record(): move to the first plane BEFORE turning illumination on (pre-branch parity — illumination was previously off during the Z move + settle; the multi-plane refactor had regressed this to illuminate during every FOV's first Z move, causing extra photobleaching). - record(): widen the try/finally so the camera-mode + stage-Z restore also runs when a plane raises the fail-fast RuntimeError (previously only ran on the normal path, leaving the stage at the plane offset on error). - test_validate_wires_recording_nz_dz: spinbox minimums make invalid Nz/dz unreachable, so the test asserted nothing beyond validate() returning None. Now monkeypatches _validate_record_zstack_params to capture and assert the actual recording_nz/recording_dz_um kwargs it receives. - Updated record()/_record_one_plane docstrings and comments to describe the new control flow.
…ynamic Z-offset caption Nz moves to the FPS/Duration row (with a divider) and the computed plane-summary label is removed; the offset caption reads "Z offset" at Nz=1 and "Bottom Z offset" at Nz>1, on one row with dz. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…dz | FPS, Dur Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…led; slim spinbox widths Without Laser AF there is no reference plane to offset from — the field (and caption) hide, and build_parameters emits 0.0 so a hidden stale value cannot shift the recording planes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…work Merges origin/feat/record-zstack-acquisition (18 commits: widget layout changes + Record/Z-Stack settings reuse via YAML + drag-and-drop) into feat/multi-plane-recording (#579) so the stacked PR is unblocked. Conflicts resolved in: - control/core/record_zstack_controller.py: kept both the multi-plane recording_plane_offsets_um() helper and the settings-reuse _build_objective_info()/_save_record_zstack_yaml() helpers. - control/widgets.py: kept HEAD's Nz/dz/bottom-Z recording-plane redesign (which superseded the single entry_recording_z_offset field) and dropped the now-redundant old Z-offset widget from the other side's layout tweaks; updated _apply_yaml_settings() and its widgets_to_block list (a non-conflicting hunk that still referenced the removed widget/field) to use the new Nz/bottom-Z/dz widgets. - tests/core/test_record_zstack_worker.py and tests/test_record_zstack_widget.py: unioned both branches' new test functions (git's diff3 had misaligned several of them because they shared boilerplate setup code); updated the handful of assertions that referenced the pre-multi-plane recording_z_offset_um field. Also renamed RecordZStackYAMLData.recording_z_offset_um -> recording_bottom_z_offset_um + new recording_nz/recording_dz_um fields (and the corresponding acquisition.yaml recording: keys) in control/acquisition_yaml_loader.py, since the settings-reuse YAML schema needs to round-trip the multi-plane fields that superseded the single z-offset -- this wasn't flagged as a textual conflict but broke without the update. Updated tests/control/test_acquisition_yaml_loader.py to match. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…d-trip test test_full_save_load_round_trip_preserves_settings previously ran with Laser AF off, which forces recording_bottom_z_offset_um to 0.0 in build_parameters() and left recording_Nz/recording_dz_um at their defaults -- so the multi-plane recording fields introduced by this branch were never actually exercised by the round trip. Enable Laser AF and set Nz/dz/bottom-Z to distinctive values so the save->parse->apply cycle is verified for these fields too.
…quisition Restores the plane-count/Z-range/per-FOV-duration summary that a prior UI change removed (it was an inline label, dropped for being too wide) as a QMessageBox.question shown right after validate() succeeds and before the acquisition actually starts, giving the user one last look. Answering No aborts the start and re-unchecks the button; the dialog is skipped entirely when the Recording phase is disabled. The summary's Z offset uses the same effective-value logic as build_parameters() (0.0 when Laser AF is off) so it can never show a stale value from the hidden entry_recording_bottom_z field that doesn't match what actually runs. Also fixes entry_recording_bottom_z's tooltip, which stayed frozen at "Bottom plane offset relative to the Z reference" even after Nz dropped to 1 and the caption switched to "Z offset:" -- it now tracks the same multi/ single wording as the label.
…offset moves to its own row Live-testing feedback: FPS/Duration should come first, and the Bottom-Z offset field (visible only when Laser AF is on) shouldn't share a row with Nz/dz/FPS/Dur since it shifts that row's width when it appears/disappears.
…yout Live-testing feedback: the offset row should line up with Nz, not start flush-left. Switched the recording-controls block from two independent QHBoxLayouts to one QGridLayout so the offset shares Nz's columns and aligns exactly regardless of font/platform metrics.
…into feat/multi-plane-recording
Live-testing feedback across several rounds landed on: FPS/Duration always lead row 1; Nz/dz/offset append to row 1 too as long as at most 2 of them are visible, but move to their own row 2 (as a group) only when all 3 are shown at once (Nz>1 AND Laser AF on) -- that's too many fields for one row. When on row 2, Nz/dz get temporarily widened to match FPS/Dur's widths so the columns align exactly (both rows share the same left margin, so equal leading widths are sufficient -- no grid needed). Otherwise they use their own compact widths sized to the actual label/spinbox content, fixing an earlier pass that forced the wider shared width unconditionally and left visible dead space in the common single-row case. Also: dz now shows 1 decimal instead of 2, and both rows share one uniform inter-item spacing instead of a larger explicit gap between field groups.
…into feat/multi-plane-recording
…trigger live can't break recording
Starting a recording while Live ran in SOFTWARE-trigger mode aborted the run on
real hardware. _probe_frame_shape() sized the recording dataset by sending a
software trigger, but Live's last trigger was still outstanding
(get_ready_for_trigger() == False), so send_trigger() was rejected ("trigger too
early"). The probe then fell back to get_resolution() — the raw binned sensor
size (3104x2084) — while the camera delivers cropped 2084x2084 frames, so every
write_frame failed with a dimension-alignment error and the store sealed
incomplete, fail-fasting the whole acquisition.
Probe in CONTINUOUS mode instead — the same mode recording uses — and read one
free-run frame (no send_trigger, so trigger readiness can't block it); the probed
shape is then exactly what recording writes. On a genuine probe failure, abort
with a clear error instead of sizing from get_resolution() and grinding out a
blank, incomplete recording.
Also fix SimulatedCamera.read_camera_frame() to wait for a frame (matching the
real camera / AbstractCamera contract) rather than returning None before the
first CONTINUOUS frame — otherwise the continuous probe gets no frame in sim.
Verified on a real ITR3CMOS26000KMA: the continuous probe returns (2084, 2084)
(matching delivery) vs get_resolution()'s (3104, 2084), and get_ready_for_trigger()
is False right after a software trigger (reproducing the reject). New tests cover
the probe using CONTINUOUS/never triggering and aborting (not get_resolution) on
no frame.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary
At each FOV, the recording phase now records a video at N z-planes (parameters: Bottom Z offset, Nz, dz) instead of a single plane — sequential per-plane videos, one Zarr store per plane. Extends the Record + Z-Stack feature (#564); plane
jrecords atz_ref + bottom_z_offset_um + j*dz_umusing the same z-reference rule as today (laser AF when enabled+referenced, else Z at FOV arrival).fov_{k}.ome.zarrand byte-identical metadata.fov_{k}_z{j}.ome.zarrand carry self-describing_squidattrs:plane_index,plane_z_offset_um,n_planes..donemarker still run.finally, so they also happen on the fail-fast write-error path.ZarrAcquisitionConfiggained an optionalextra_squid_attrspassthrough (None for single-plane output).Spec:
design-multi-plane-recording-2026-07-03.md(approved 2026-07-03).Test plan
_squidattrs readback, dz visibility, planes label, validate() wiring.fov_0_z0/z1/z2.ome.zarr, shape(5,1,1,H,W), correct per-plane metadata, dz visible at Nz=3, zero tracebacks; screenshot confirms the new fields row.🤖 Generated with Claude Code