From 858d10fe5375bb171515973501ce5c30e95718a0 Mon Sep 17 00:00:00 2001 From: Hongquan Li Date: Mon, 6 Jul 2026 10:03:38 -0400 Subject: [PATCH] =?UTF-8?q?feat(record-zstack):=20Hamamatsu=20fps=20not=20?= =?UTF-8?q?settable=20=E2=80=94=20report=20achievable=20rate,=20don't=20tr?= =?UTF-8?q?y=20to=20set?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hamamatsu ORCA can't command a frame rate in CONTINUOUS mode (INTERNAL_FRAMERATE and INTERNAL_FRAMEINTERVAL are read-only, bench-verified: DCAMERR.NOTWRITABLE). It free-runs at the exposure/readout-limited max, overlapping exposure with readout. Make the UI and worker match, generalized via a camera capability so ToupTek (which CAN set via PRECISE_FRAMERATE) is unaffected. - AbstractCamera.can_set_frame_rate() (default True) + estimate_frame_rate(exposure_ms) (default sequential 1000/(exposure+strobe)). - HamamatsuCamera: can_set_frame_rate() -> False. set_frame_rate() reports the camera's AUTHORITATIVE free-run rate by reading the read-only INTERNAL_FRAMERATE (which reflects the current readout mode fast/standard/ultraquiet, bit depth, ROI and exposure), falling back to the overlap estimate only if the read fails. estimate_frame_rate() = overlap 1000/max(readout, exposure) (readout = line_interval * rows) for the widget's preview at a hypothetical exposure. Neither path writes anything (removes the NOTWRITABLE ERROR + the READOUTSPEED side-effect of the earlier approach). - Record widget: when the camera can't set fps, the fps field is read-only and shows "~ X fps", recomputed from the (still-editable) recording exposure (incl. after a YAML load). - Worker: for not-settable cameras, size/pace the recording to the achievable max instead of honoring params.fps. Tests: base/sim capability + estimate; worker sizes to achievable; widget read-only vs editable. Hamamatsu-specific test (authoritative-read vs formula-fallback + overlap math) guarded on the DCAM lib (absent off-instrument); the overlap math is also covered by the ToupTek continuous-max tests. Co-Authored-By: Claude Opus 4.8 (1M context) --- software/control/camera_hamamatsu.py | 38 ++++++++++++ software/control/core/record_zstack_worker.py | 13 +++- software/control/widgets.py | 34 +++++++++++ software/squid/abc.py | 22 +++++++ .../tests/core/test_record_zstack_worker.py | 31 ++++++++++ .../tests/test_record_zstack_camera_fps.py | 60 +++++++++++++++++++ software/tests/test_record_zstack_widget.py | 41 ++++++++++++- 7 files changed, 237 insertions(+), 2 deletions(-) diff --git a/software/control/camera_hamamatsu.py b/software/control/camera_hamamatsu.py index 2d6c3d45c..2f911f5b6 100644 --- a/software/control/camera_hamamatsu.py +++ b/software/control/camera_hamamatsu.py @@ -210,6 +210,44 @@ def get_strobe_time(self) -> float: return (line_interval_s + trigger_delay_s) * 1000.0 + def _readout_time_ms(self) -> float: + """Pure, exposure-independent sensor readout period in ms (line interval x rows). + + Unlike get_strobe_time() this excludes trigger_delay: in CONTINUOUS free-run the + exposure pipelines (overlaps) with readout, so the frame period is + max(readout, exposure) rather than their sum. + """ + resolution = self.get_resolution() + line_interval_s = self._camera.prop_getvalue(DCAM_IDPROP.INTERNAL_LINEINTERVAL) + if isinstance(line_interval_s, bool): + raise CameraError("Failed to read INTERNAL_LINEINTERVAL from camera") + return line_interval_s * resolution[1] * 1000.0 + + def estimate_frame_rate(self, exposure_ms: float) -> float: + """Achievable continuous free-run fps at `exposure_ms`. + + In CONTINUOUS mode the ORCA overlaps exposure with sensor readout, so the frame + period is max(readout, exposure) - NOT get_total_frame_time() (readout + exposure), + which under-reports the free-run rate by ~2x. Reads timing only; never mutates. + """ + return 1000.0 / max(self._readout_time_ms(), exposure_ms) + + def can_set_frame_rate(self) -> bool: + # In CONTINUOUS mode the ORCA free-runs at the exposure/readout-limited max; + # INTERNAL_FRAMERATE and INTERNAL_FRAMEINTERVAL are read-only (bench-verified: + # DCAMERR.NOTWRITABLE in every camera state). The rate can be read, not commanded. + return False + + def set_frame_rate(self, fps: float) -> float: + # Not settable (see can_set_frame_rate). Report the camera's authoritative free-run + # rate: INTERNAL_FRAMERATE is read-only but READABLE, and reflects the current readout + # mode (fast/standard/ultraquiet), bit depth, ROI and exposure — no modeling needed. + # Fall back to the exposure/readout estimate only if the read fails. Writes nothing. + rate = self._camera.prop_getvalue(DCAM_IDPROP.INTERNALFRAMERATE) + if isinstance(rate, bool) or not rate or rate <= 0: + return self.estimate_frame_rate(self.get_exposure_time()) + return float(rate) + def set_frame_format(self, frame_format: CameraFrameFormat): if frame_format != CameraFrameFormat.RAW: raise ValueError("Only the RAW frame format is supported by this camera.") diff --git a/software/control/core/record_zstack_worker.py b/software/control/core/record_zstack_worker.py index 7d1cec2dc..661a84ee8 100644 --- a/software/control/core/record_zstack_worker.py +++ b/software/control/core/record_zstack_worker.py @@ -366,7 +366,18 @@ def record(self, t_idx: int, region_id, fov_idx: int, z_ref: float) -> int: effective_fps = self.params.fps try: achievable_fps = self.camera.set_frame_rate(self.params.fps) - if achievable_fps and 0 < achievable_fps < self.params.fps: + if not self.camera.can_set_frame_rate(): + # fps is not settable on this camera (e.g. Hamamatsu free-runs at its + # exposure/readout-limited max in CONTINUOUS). Record at the achievable + # rate; params.fps is only a display value for these cameras. + if achievable_fps and achievable_fps > 0: + if abs(achievable_fps - self.params.fps) > 0.01: + log.info( + f"camera fps not settable; recording at achievable ≈ {achievable_fps:.2f} fps " + f"(requested {self.params.fps:g})" + ) + effective_fps = achievable_fps + elif achievable_fps and 0 < achievable_fps < self.params.fps: log.warning( f"camera cannot deliver {self.params.fps:g} fps " f"(achievable ≈ {achievable_fps:.2f}); recording at the achievable rate" diff --git a/software/control/widgets.py b/software/control/widgets.py index f8dc4f627..945be6f56 100644 --- a/software/control/widgets.py +++ b/software/control/widgets.py @@ -17489,6 +17489,21 @@ def _build_recording_group(self) -> QGroupBox: self.entry_fps.setMaximumWidth(78) fps_row.addWidget(self.entry_fps) + # On cameras that can't command a frame rate (e.g. Hamamatsu free-runs at its + # exposure/readout-limited max in continuous mode), fps is not settable: present + # the achievable rate read-only and keep it in sync with the (editable) exposure. + self._fps_is_settable = self.liveController.camera.can_set_frame_rate() + if not self._fps_is_settable: + self.entry_fps.setReadOnly(True) + self.entry_fps.setButtonSymbols(QAbstractSpinBox.NoButtons) + self.entry_fps.setPrefix("≈ ") + self.entry_fps.setToolTip( + "This camera runs at its exposure/readout-limited maximum in continuous " + "mode; frame rate is not settable — adjust exposure to change it." + ) + self._recording_exp_spin.valueChanged.connect(self._update_estimated_fps) + self._update_estimated_fps() + fps_row.addSpacing(4) fps_row.addWidget(QLabel("Dur:")) self.entry_duration = QDoubleSpinBox() @@ -17681,6 +17696,20 @@ def _on_recording_channel_changed(self, index: int) -> None: self._recording_gain_spin.setValue(gain) self._recording_illum_spin.setValue(illum) + def _update_estimated_fps(self) -> None: + """Refresh the read-only fps display from the camera's achievable free-run rate at + the current recording exposure (only for cameras where fps is not settable). + + Fires on any recording-exposure change (manual edit, channel reseed, copy-from-live) + since those all go through _recording_exp_spin.setValue. + """ + try: + fps = self.liveController.camera.estimate_frame_rate(self._recording_exp_spin.value()) + except Exception: + return # a transient camera-read failure isn't fatal; keep the previous display + if fps and fps > 0: + self.entry_fps.setValue(fps) + # ---------------------------------------------------------------------- recording table accessors def _recording_channel_name(self) -> Optional[str]: @@ -18043,6 +18072,11 @@ def _apply_yaml_settings(self, yaml_data) -> None: finally: for widget in widgets_to_block: widget.blockSignals(False) + # The recording exposure was set with signals blocked, so the read-only "≈ X fps" + # display (cameras that can't set fps) wasn't recomputed — refresh it from the + # freshly-loaded exposure rather than leaving the saved yaml_data.fps showing. + if not self._fps_is_settable: + self._update_estimated_fps() # checkbox_time's toggled signal was blocked above, so the normal # _on_time_toggled-driven visibility refresh didn't fire. Set the # frame's visibility directly rather than calling _on_time_toggled diff --git a/software/squid/abc.py b/software/squid/abc.py index 79bf2afa2..3aeb04ef2 100644 --- a/software/squid/abc.py +++ b/software/squid/abc.py @@ -536,6 +536,28 @@ def set_frame_rate(self, fps: float) -> float: self._log.debug(f"set_frame_rate({fps}) no-op on base camera; max≈{max_fps:.2f} fps") return max_fps + def can_set_frame_rate(self) -> bool: + """Whether this camera can be commanded to a specific continuous frame rate. + + Defaults to True; a subclass returns False only when it is known to merely free-run at + its exposure/readout-limited maximum in continuous mode (e.g. Hamamatsu, whose + INTERNAL_FRAMERATE/FRAMEINTERVAL are read-only). Note: the True default does NOT prove a + camera truly honors a commanded rate — a subclass still on the base set_frame_rate no-op + reports True yet only free-runs. Callers use this to decide whether to present fps as + settable / honor a requested fps, or to record at the achievable max and show it read-only. + """ + return True + + def estimate_frame_rate(self, exposure_ms: float) -> float: + """Best-effort continuous free-run fps at a hypothetical exposure, without changing + camera state (a UI preview helper). + + Base default assumes sequential timing (readout + exposure). Cameras whose + continuous mode pipelines exposure with readout (overlap) should override to + max(readout, exposure). Reads timing properties only; never mutates. + """ + return 1000.0 / (exposure_ms + self.get_strobe_time()) + @abc.abstractmethod def set_frame_format(self, frame_format: CameraFrameFormat): """ diff --git a/software/tests/core/test_record_zstack_worker.py b/software/tests/core/test_record_zstack_worker.py index dcf37aa9a..1703ae889 100644 --- a/software/tests/core/test_record_zstack_worker.py +++ b/software/tests/core/test_record_zstack_worker.py @@ -386,6 +386,37 @@ def _build_worker_harness(tmp_path, recording_enabled, zstack_enabled, zstack_ch return scope, live_controller, channels, worker, aborted +def test_record_not_settable_camera_sizes_recording_to_achievable_max(tmp_path, monkeypatch): + """When the camera can't command a frame rate (e.g. Hamamatsu free-runs at its + exposure/readout-limited max), the recording dataset is sized to the achievable rate + the camera reports, not to the requested params.fps.""" + pytest.importorskip("tensorstore") + import tensorstore as tstore + from control.core.record_zstack_worker import frame_count + + scope, live_controller, channels, worker, aborted = _build_worker_harness( + tmp_path, recording_enabled=True, zstack_enabled=False + ) + + # params.fps is 10 fps; report the camera as not-settable with a DIFFERENT achievable max. + ACHIEVABLE = 25.0 + monkeypatch.setattr(scope.camera, "can_set_frame_rate", lambda: False) + monkeypatch.setattr(scope.camera, "set_frame_rate", lambda fps: ACHIEVABLE) + + worker.run() + assert not aborted["v"] + + expected_T = max(1, frame_count(ACHIEVABLE, worker.params.duration_s)) + requested_T = max(1, frame_count(worker.params.fps, worker.params.duration_s)) + assert expected_T != requested_T # sanity: the two rates size the dataset differently + + rec = sorted((Path(str(tmp_path)) / worker.params.experiment_id / "recording").rglob("*.ome.zarr")) + assert rec, "no recording dataset produced" + for path in rec: + ds = tstore.open({"driver": "zarr3", "kvstore": {"driver": "file", "path": str(path)}}).result() + assert ds.shape[0] == expected_T, f"recording T {ds.shape[0]} != achievable-sized {expected_T}" + + def test_record_only_restores_trigger_mode(tmp_path): """F8: a record-only run must not leave the camera in SOFTWARE_TRIGGER when the LiveController was in CONTINUOUS (or HARDWARE) before the acquisition.""" diff --git a/software/tests/test_record_zstack_camera_fps.py b/software/tests/test_record_zstack_camera_fps.py index 1a14bbea7..c9850d6a5 100644 --- a/software/tests/test_record_zstack_camera_fps.py +++ b/software/tests/test_record_zstack_camera_fps.py @@ -87,3 +87,63 @@ def get_Option(self, opt): # Request 30 fps; camera can't reach it, so we get its true continuous max (~28), not ~14. achievable = cam.set_frame_rate(30.0) assert 27.5 < achievable < 28.5, achievable + + +# --- Frame-rate capability (settable vs not) ------------------------------------------ + + +def test_simulated_camera_reports_frame_rate_settable(): + # The simulated (and ToupTek) cameras can command a rate; only cameras like Hamamatsu can't. + assert _sim_camera().can_set_frame_rate() is True + + +def test_base_estimate_frame_rate_is_sequential(): + # Base default assumes sequential timing: 1000 / (exposure + strobe). + cam = _sim_camera() + cam.set_exposure_time(10) + expected = 1000.0 / (10.0 + cam.get_strobe_time()) + assert abs(cam.estimate_frame_rate(10.0) - expected) < 1e-6 + + +def test_hamamatsu_frame_rate_not_settable_and_overlap_estimate(): + # Hamamatsu's DCAM library isn't present off-instrument; this runs only where it imports. + import pytest + + try: + import control.camera_hamamatsu as ham + except (ImportError, OSError): + pytest.skip("Hamamatsu DCAM library not available on this platform") + + from types import SimpleNamespace + + from control.dcamapi4 import DCAM_IDPROP + + cam = object.__new__(ham.HamamatsuCamera) + assert cam.can_set_frame_rate() is False + + # Per-property stub: INTERNAL_FRAMERATE (authoritative free-run rate) vs INTERNAL_LINEINTERVAL + # (10 us/line, used by the formula). line_interval 10 us over 2000 rows -> 20 ms readout. + def _prop(pid): + if pid == DCAM_IDPROP.INTERNALFRAMERATE: + return 42.0 + return 0.00001 + + cam._camera = SimpleNamespace(prop_getvalue=_prop) + cam._capabilities = SimpleNamespace(binning_to_resolution={(1, 1): (2000, 2000)}) + cam._exposure_time_ms = 5.0 + + # estimate_frame_rate (widget preview at a hypothetical exposure) uses the overlap formula. + assert abs(cam.estimate_frame_rate(5.0) - 50.0) < 1e-6 # readout(20) > exposure(5) -> 1000/20 + assert abs(cam.estimate_frame_rate(40.0) - 25.0) < 1e-6 # exposure(40) > readout(20) -> 1000/40 + + # set_frame_rate reports the camera's authoritative INTERNAL_FRAMERATE, not the formula. + assert abs(cam.set_frame_rate(999.0) - 42.0) < 1e-6 + + # ...falling back to the formula estimate when the authoritative read fails. + def _prop_fr_unreadable(pid): + if pid == DCAM_IDPROP.INTERNALFRAMERATE: + return False # DCAM read failure sentinel + return 0.00001 + + cam._camera = SimpleNamespace(prop_getvalue=_prop_fr_unreadable) + assert abs(cam.set_frame_rate(999.0) - 50.0) < 1e-6 # exposure 5ms < readout 20ms -> 1000/20 diff --git a/software/tests/test_record_zstack_widget.py b/software/tests/test_record_zstack_widget.py index 1938a809a..70b1ecd56 100644 --- a/software/tests/test_record_zstack_widget.py +++ b/software/tests/test_record_zstack_widget.py @@ -1570,7 +1570,12 @@ def test_get_expected_widget_type_is_record_zstack(qtbot, simulated_widget_deps) def test_get_camera_for_binning_check_uses_live_controller(qtbot, simulated_widget_deps): from control.widgets import RecordZStackMultiPointWidget - camera = object() + class _CameraSentinel: + # A real camera always exposes can_set_frame_rate(); the widget queries it at build. + def can_set_frame_rate(self): + return True + + camera = _CameraSentinel() simulated_widget_deps["liveController"].camera = camera w = RecordZStackMultiPointWidget(**simulated_widget_deps) qtbot.addWidget(w) @@ -1790,3 +1795,37 @@ def test_full_save_load_round_trip_preserves_settings(qtbot, simulated_widget_de assert ch2.exposure_time == pytest.approx(ch1.exposure_time) assert ch2.analog_gain == pytest.approx(ch1.analog_gain) assert ch2.illumination_intensity == pytest.approx(ch1.illumination_intensity) + + +def test_fps_control_readonly_and_shows_estimate_when_camera_cannot_set_rate(qtbot, simulated_widget_deps): + """On a camera that can't command a frame rate (e.g. Hamamatsu), the recording fps field + is read-only and displays the camera's achievable rate, recomputed from the (editable) + recording exposure.""" + from control.widgets import RecordZStackMultiPointWidget + + ctrl = simulated_widget_deps["liveController"] + ctrl.camera.can_set_frame_rate.return_value = False + # overlap model: fps = 1000 / max(readout=20 ms, exposure) + ctrl.camera.estimate_frame_rate.side_effect = lambda exp_ms: 1000.0 / max(20.0, exp_ms) + + w = RecordZStackMultiPointWidget(**simulated_widget_deps) + qtbot.addWidget(w) + + assert w.entry_fps.isReadOnly() + w._recording_exp_spin.setValue(5.0) # readout(20) > exposure(5) -> 1000/20 = 50 fps + assert w.entry_fps.value() == pytest.approx(50.0, abs=0.5) + w._recording_exp_spin.setValue(40.0) # exposure(40) > readout(20) -> 1000/40 = 25 fps + assert w.entry_fps.value() == pytest.approx(25.0, abs=0.5) + + +def test_fps_control_editable_when_camera_can_set_rate(qtbot, simulated_widget_deps): + """Cameras that can command a rate (ToupTek, simulated) keep the editable fps field.""" + from control.widgets import RecordZStackMultiPointWidget + + ctrl = simulated_widget_deps["liveController"] + ctrl.camera.can_set_frame_rate.return_value = True + + w = RecordZStackMultiPointWidget(**simulated_widget_deps) + qtbot.addWidget(w) + + assert not w.entry_fps.isReadOnly()