fix(omny): reset progress fields before zero-degree reference shots
CI for csaxs_bec / test (push) Successful in 3m55s
CI for csaxs_bec / test (push) Successful in 3m55s
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QLrD7sVYGLAzsQjLVJpCgt
This commit is contained in:
1 parent
71eb9d858f
commit
bdb0d25f90
3 files changed
+112
-1
No files matched your search
@@ -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-
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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."""
|
||||
|
||||
Reference in new issue
Block a user