From bdb0d25f90db4203d497a2c6b0bd025a6f5a89b3 Mon Sep 17 00:00:00 2001 From: x01dc Date: Thu, 3 Sep 2026 17:01:00 +0200 Subject: [PATCH] fix(omny): reset progress fields before zero-degree reference shots progress["projection"]/["subtomo_projection"]/["subtomo_total_projections"]/ ["total_projections"] are otherwise only ever set inside sub_tomo_scan()'s per-point loop. The zero_deg_reference_at_each_subtomo pre/post shots call _tomo_scan_at_angle() directly, bypassing that -- so the printed progress showed whatever was left over from a previous scan/subtomo instead of a value that made sense for the shot. Confirmed live: a fresh scan's first reference shot printed a stale "Projection: 9 / Total: 32" from an earlier run. Same bug exists in flomni.py (this was ported from there faithfully, bug included) -- flagged in OPEN_ISSUES.md but not touched, out of this branch's scope. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01QLrD7sVYGLAzsQjLVJpCgt --- .../plugins/omny/AI_docs/OPEN_ISSUES.md | 32 +++++++++++ .../bec_ipython_client/plugins/omny/omny.py | 27 +++++++++- .../test_omny_tomo_scan.py | 54 +++++++++++++++++++ 3 files changed, 112 insertions(+), 1 deletion(-) diff --git a/csaxs_bec/bec_ipython_client/plugins/omny/AI_docs/OPEN_ISSUES.md b/csaxs_bec/bec_ipython_client/plugins/omny/AI_docs/OPEN_ISSUES.md index 045162b4..7d69baa6 100644 --- a/csaxs_bec/bec_ipython_client/plugins/omny/AI_docs/OPEN_ISSUES.md +++ b/csaxs_bec/bec_ipython_client/plugins/omny/AI_docs/OPEN_ISSUES.md @@ -762,6 +762,38 @@ was wired in). Full suite: 773 passed. confirming: the rotation-skip-if-already-at-0 branch, the direction/magnitude of the x shift, and that `omny_osamx_to_scan_center()`'s recenter fires (or correctly doesn't) as expected. +## zero_deg_reference_at_each_subtomo's progress fields were stale (fixed for OMNY; same bug likely in flomni, not touched) + +Mirko, live-testing the just-landed feature: "the first zero degree projection does not seem to +reset the projection number. not sure if it is correctly done in the case of flomni" -- and a +pasted transcript showing the very first (pre-subtomo-1) reference shot printing "Projection: 9 / +Total projections expected: 32" (stale leftover from an earlier run), followed moments later by +subtomo 1's own real first projection correctly printing "Projection: 0 / Total: 10". + +Root cause: `progress["projection"]`/`["subtomo_projection"]`/`["subtomo_total_projections"]`/ +`["total_projections"]` are otherwise only ever set inside `sub_tomo_scan()`'s per-point loop. The +0-degree reference shots (both the pre-subtomo-N and the final post-loop one) call +`_tomo_scan_at_angle()` directly, bypassing `sub_tomo_scan()` entirely -- so whatever those fields +happened to hold from a previous scan (or the previous subtomo) is what gets printed, not a value +that makes sense for this shot. + +**Fixed for OMNY**: both call sites in `tomo_scan()`'s type-1-like loop now explicitly set all +four fields immediately before calling `_tomo_scan_at_angle(0, ...)` -- the pre-shot is treated as +sub-tomogram `ii`'s own projection 0 (matching what `sub_tomo_scan()` is about to set for its real +first point right after), the final shot as one past the last real point of the final +sub-tomogram. Regression test: +`test_tomo_scan_zero_deg_reference_progress_fields_are_fresh_not_stale` (seeds `progress` with +stale values matching the live report, asserts both shots' snapshots are fully fresh/correct, not +just non-crashing). + +**Same bug confirmed present in flomni's own code** (`flomni.py`'s equally-spaced-sub-tomograms +loop, `_tomo_scan_at_angle(0, ii)`/`_tomo_scan_at_angle(0, 8)` calls, same as OMNY's before this +fix -- flomni's `sub_tomo_scan()` is likewise the only place that sets these fields) -- this is +where OMNY's version was ported from, so it inherited the bug faithfully. **Not fixed here**: +`flomni.py` is a different, currently-in-production plugin outside this branch's scope +(`update-omny`); flagging for a separate, dedicated look rather than bundling an unreviewed +flomni-affecting change into this branch. + ## Environment note (not code, but will bite again if forgotten) Both `csaxs_bec` and `bec_widgets` were pip-installed editable pointing at pre-repo- diff --git a/csaxs_bec/bec_ipython_client/plugins/omny/omny.py b/csaxs_bec/bec_ipython_client/plugins/omny/omny.py index 90136fed..d07a2ea1 100644 --- a/csaxs_bec/bec_ipython_client/plugins/omny/omny.py +++ b/csaxs_bec/bec_ipython_client/plugins/omny/omny.py @@ -1380,6 +1380,7 @@ class OMNY( if self.tomo_type in (1, 4, 5): # equally spaced sub-tomograms (2, 4, or 8) n_subtomos = {1: 2, 4: 4, 5: 8}[self.tomo_type] + n_points = int(180.0 / self.tomo_angle_stepsize) self.progress["tomo_type"] = f"Equally spaced sub-tomograms ({n_subtomos})" for ii in range(subtomo_start, n_subtomos + 1): if ( @@ -1399,6 +1400,24 @@ class OMNY( # redundant. Skipped when resuming mid-sub-tomogram # (start_angle given explicitly) since we're not # actually passing through 0 deg at that moment. + # + # progress["projection"]/["subtomo_projection"]/etc. + # are otherwise only ever set inside sub_tomo_scan() + # -- set them explicitly here too, or the printed + # progress for this shot shows whatever was left + # over from the previous scan/subtomo instead of a + # value that makes sense for it (confirmed live: a + # stale "Projection: 9 / Total: 32" from a prior + # run, on the very first projection of a fresh + # scan). Treated as sub-tomogram ii's own + # projection 0, matching what sub_tomo_scan() is + # about to set for its real first point right + # after this. + self.progress["subtomo"] = ii + self.progress["subtomo_projection"] = 0 + self.progress["subtomo_total_projections"] = n_points + self.progress["projection"] = (ii - 1) * n_points + self.progress["total_projections"] = n_points * n_subtomos self._tomo_scan_at_angle(0, ii) self.sub_tomo_scan(ii, n_subtomos=n_subtomos, start_angle=start_angle) start_angle = None @@ -1407,7 +1426,13 @@ class OMNY( # Final reference shot at exactly 0 degrees once the whole # tomogram is complete, giving a clean "before vs after" # pair for radiation-damage comparison across the full - # acquisition. + # acquisition. One past the last real point of the final + # sub-tomogram, same progress-freshness reasoning as above. + self.progress["subtomo"] = n_subtomos + self.progress["subtomo_projection"] = n_points + self.progress["subtomo_total_projections"] = n_points + self.progress["projection"] = n_points * n_subtomos + self.progress["total_projections"] = n_points * n_subtomos self._tomo_scan_at_angle(0, n_subtomos) elif self.tomo_type == 2: diff --git a/tests/tests_bec_ipython_client/test_omny_tomo_scan.py b/tests/tests_bec_ipython_client/test_omny_tomo_scan.py index 99a1791f..4be38cca 100644 --- a/tests/tests_bec_ipython_client/test_omny_tomo_scan.py +++ b/tests/tests_bec_ipython_client/test_omny_tomo_scan.py @@ -196,6 +196,60 @@ def test_tomo_scan_zero_deg_reference_type1_fires_before_subtomo1_and_after_subt assert calls == [(0, 1), (0, 2)] +def test_tomo_scan_zero_deg_reference_progress_fields_are_fresh_not_stale(monkeypatch): + """progress["projection"]/["total_projections"]/["subtomo_projection"]/ + ["subtomo_total_projections"] are otherwise only ever set inside + sub_tomo_scan() -- the 0-degree reference shots bypass that, calling + _tomo_scan_at_angle() directly, so without an explicit reset they show + whatever was left over from a previous scan/subtomo instead of a value + that makes sense for this shot. Confirmed live: a fresh scan's very + first (pre-subtomo-1) reference shot printed a stale "Projection: 9 / + Total projections expected: 32" from an earlier run.""" + omny = make_omny_for_tomo_scan(monkeypatch, 45.0, active_account="") + omny.tomo_type = 1 + omny.zero_deg_reference_at_each_subtomo = True + # simulate stale leftover progress from an unrelated previous scan + omny.progress["projection"] = 9 + omny.progress["total_projections"] = 32 + omny.progress["subtomo_projection"] = 9 + omny.progress["subtomo_total_projections"] = 16 + + fields = ( + "subtomo", + "subtomo_projection", + "subtomo_total_projections", + "projection", + "total_projections", + ) + snapshots = [] + + def _fake_at_angle(angle, subtomo_number): + snapshots.append({key: omny.progress[key] for key in fields}) + + omny._tomo_scan_at_angle = _fake_at_angle + + omny.tomo_scan() + + # n_points = int(180/45) = 4, n_subtomos = 2 -> total = 8 + pre_shot = snapshots[0] # before sub-tomogram 1 + assert pre_shot == { + "subtomo": 1, + "subtomo_projection": 0, + "subtomo_total_projections": 4, + "projection": 0, + "total_projections": 8, + } + + final_shot = snapshots[-1] # after sub-tomogram 2 + assert final_shot == { + "subtomo": 2, + "subtomo_projection": 4, + "subtomo_total_projections": 4, + "projection": 8, + "total_projections": 8, + } + + def test_tomo_scan_zero_deg_reference_type5_fires_before_every_odd_subtomo(monkeypatch): """Type 5 (n_subtomos=8): a pre-shot before every odd/forward sub-tomogram (1, 3, 5, 7), plus the final shot after sub-tomogram 8."""