Merge dev-rowpacking: selectable row packing policy
Row packing is now a choice rather than a fixed behaviour. Pad (default, unchanged) squares a mis-triggered row up to the declared n_frames and warns; strict stops the scan instead, so a data run cannot quietly contain a padded row that nothing in the file marks as padded. Both paths check the frame count before writing any of the row's channels, so a strict abort ends the file on a whole-row boundary. 92 tests passing, ruff clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -24,6 +24,7 @@ class ScanDefaults:
|
|||||||
save_dir: str = str(DEFAULTS_PATH.parent / "scans")
|
save_dir: str = str(DEFAULTS_PATH.parent / "scans")
|
||||||
helios_port: str = "/dev/ttyUSB2"
|
helios_port: str = "/dev/ttyUSB2"
|
||||||
burst_mode: bool = False
|
burst_mode: bool = False
|
||||||
|
strict_rows: bool = False
|
||||||
|
|
||||||
@classmethod
|
@classmethod
|
||||||
def load(cls, path: Path = DEFAULTS_PATH) -> "ScanDefaults":
|
def load(cls, path: Path = DEFAULTS_PATH) -> "ScanDefaults":
|
||||||
|
|||||||
+23
-4
@@ -96,7 +96,7 @@ class ScanEngine:
|
|||||||
plan: ScanPlan, out_path: Path,
|
plan: ScanPlan, out_path: Path,
|
||||||
resume: ResumeState | None = None,
|
resume: ResumeState | None = None,
|
||||||
callbacks: ScanCallbacks | None = None,
|
callbacks: ScanCallbacks | None = None,
|
||||||
burst_mode: bool = False):
|
burst_mode: bool = False, strict_rows: bool = False):
|
||||||
self._stage = stage
|
self._stage = stage
|
||||||
self._scope = scope
|
self._scope = scope
|
||||||
self._rotator = rotator
|
self._rotator = rotator
|
||||||
@@ -107,6 +107,9 @@ class ScanEngine:
|
|||||||
# Burst mode acquires as many whole rows per FastFrame acquisition as
|
# Burst mode acquires as many whole rows per FastFrame acquisition as
|
||||||
# the scope's frame memory holds, instead of one row per acquisition.
|
# the scope's frame memory holds, instead of one row per acquisition.
|
||||||
self._burst_mode = burst_mode
|
self._burst_mode = burst_mode
|
||||||
|
# Strict row packing stops the scan on a frame-count mismatch
|
||||||
|
# instead of squaring the row up (see _check_frame_delta).
|
||||||
|
self._strict_rows = strict_rows
|
||||||
self._max_frames = 0
|
self._max_frames = 0
|
||||||
self._preflight_done = False
|
self._preflight_done = False
|
||||||
|
|
||||||
@@ -434,7 +437,7 @@ class ScanEngine:
|
|||||||
self._cb.on_status(f"Fetching CH{ch} data …")
|
self._cb.on_status(f"Fetching CH{ch} data …")
|
||||||
waveforms = scope_sras.transfer_channel(scope, ch)
|
waveforms = scope_sras.transfer_channel(scope, ch)
|
||||||
if ch == SCAN_CHANNELS[0]:
|
if ch == SCAN_CHANNELS[0]:
|
||||||
self._warn_frame_delta(row_idx, len(waveforms), n_frames)
|
self._check_frame_delta(row_idx, len(waveforms), n_frames)
|
||||||
row = scope_burst.normalize_row(
|
row = scope_burst.normalize_row(
|
||||||
b"".join(waveforms), 0, len(waveforms), n_frames, samples_per_frame)
|
b"".join(waveforms), 0, len(waveforms), n_frames, samples_per_frame)
|
||||||
if ch == 4:
|
if ch == 4:
|
||||||
@@ -577,7 +580,7 @@ class ScanEngine:
|
|||||||
total_frames = sum(counts)
|
total_frames = sum(counts)
|
||||||
|
|
||||||
for r, count in enumerate(counts):
|
for r, count in enumerate(counts):
|
||||||
self._warn_frame_delta(first_row + r, count, n_frames)
|
self._check_frame_delta(first_row + r, count, n_frames)
|
||||||
|
|
||||||
for ch_idx, ch in enumerate(SCAN_CHANNELS):
|
for ch_idx, ch in enumerate(SCAN_CHANNELS):
|
||||||
if ch == 3:
|
if ch == 3:
|
||||||
@@ -608,10 +611,26 @@ class ScanEngine:
|
|||||||
|
|
||||||
scan_file.seek(burst_start + len(counts) * row_bytes)
|
scan_file.seek(burst_start + len(counts) * row_bytes)
|
||||||
|
|
||||||
def _warn_frame_delta(self, row_idx: int, count: int, n_frames: int):
|
def _check_frame_delta(self, row_idx: int, count: int, n_frames: int):
|
||||||
|
"""Decide what to do with a row that did not acquire n_frames frames.
|
||||||
|
|
||||||
|
v6 declares n_frames per row in the header and has no per-row length
|
||||||
|
field, so a mismatched row cannot just be written as-is — that would
|
||||||
|
shift every later row in the file. The only two safe options are to
|
||||||
|
square it up or to stop, which is what strict_rows selects between.
|
||||||
|
|
||||||
|
Called before anything for the row is written (CH1 leads
|
||||||
|
SCAN_CHANNELS), so raising here leaves no partial row behind.
|
||||||
|
"""
|
||||||
if count == n_frames:
|
if count == n_frames:
|
||||||
return
|
return
|
||||||
verb = "zero-padded" if count < n_frames else "truncated"
|
verb = "zero-padded" if count < n_frames else "truncated"
|
||||||
|
if self._strict_rows:
|
||||||
|
raise RuntimeError(
|
||||||
|
f"Row {row_idx + 1}: {count} frames acquired, {n_frames} "
|
||||||
|
f"expected. Strict row packing is on, so the scan stops here "
|
||||||
|
f"rather than writing a row that would be {verb}."
|
||||||
|
)
|
||||||
msg = (f"Row {row_idx + 1}: {count} frames acquired, {n_frames} "
|
msg = (f"Row {row_idx + 1}: {count} frames acquired, {n_frames} "
|
||||||
f"expected — {verb} to keep the file layout intact.")
|
f"expected — {verb} to keep the file layout intact.")
|
||||||
logger.warning(msg)
|
logger.warning(msg)
|
||||||
|
|||||||
+4
-2
@@ -32,7 +32,8 @@ class QtScanController(QObject):
|
|||||||
paused_changed = pyqtSignal(bool) # True while paused at a row boundary
|
paused_changed = pyqtSignal(bool) # True while paused at a row boundary
|
||||||
|
|
||||||
def __init__(self, stage, scope, rotator, plan, out_path,
|
def __init__(self, stage, scope, rotator, plan, out_path,
|
||||||
resume=None, on_scan_active=None, burst_mode=False):
|
resume=None, on_scan_active=None, burst_mode=False,
|
||||||
|
strict_rows=False):
|
||||||
super().__init__()
|
super().__init__()
|
||||||
self._prompt_event = threading.Event()
|
self._prompt_event = threading.Event()
|
||||||
self._on_scan_active = on_scan_active
|
self._on_scan_active = on_scan_active
|
||||||
@@ -48,7 +49,8 @@ class QtScanController(QObject):
|
|||||||
)
|
)
|
||||||
self._engine = ScanEngine(stage, scope, rotator, plan, out_path,
|
self._engine = ScanEngine(stage, scope, rotator, plan, out_path,
|
||||||
resume=resume, callbacks=callbacks,
|
resume=resume, callbacks=callbacks,
|
||||||
burst_mode=burst_mode)
|
burst_mode=burst_mode,
|
||||||
|
strict_rows=strict_rows)
|
||||||
|
|
||||||
# ── Engine control (called from the GUI thread) ───────────────────────────
|
# ── Engine control (called from the GUI thread) ───────────────────────────
|
||||||
|
|
||||||
|
|||||||
@@ -1007,6 +1007,16 @@
|
|||||||
</property>
|
</property>
|
||||||
</widget>
|
</widget>
|
||||||
</item>
|
</item>
|
||||||
|
<item>
|
||||||
|
<widget class="QCheckBox" name="strict_rows_check">
|
||||||
|
<property name="toolTip">
|
||||||
|
<string>Stop the scan if a row does not acquire the expected number of frames, instead of zero-padding a short row or truncating a long one. Use for data runs where a silently squared-up row would be worse than a failed scan.</string>
|
||||||
|
</property>
|
||||||
|
<property name="text">
|
||||||
|
<string>Strict row packing (abort on frame-count mismatch)</string>
|
||||||
|
</property>
|
||||||
|
</widget>
|
||||||
|
</item>
|
||||||
<item>
|
<item>
|
||||||
<widget class="QPushButton" name="start_scan_btn">
|
<widget class="QPushButton" name="start_scan_btn">
|
||||||
<property name="text">
|
<property name="text">
|
||||||
|
|||||||
@@ -918,6 +918,7 @@ class MainWindow(QMainWindow):
|
|||||||
self.scan_prefix_edit.setText("scan")
|
self.scan_prefix_edit.setText("scan")
|
||||||
self.scan_save_dir_edit.setText(DEFAULTS.save_dir)
|
self.scan_save_dir_edit.setText(DEFAULTS.save_dir)
|
||||||
self.burst_mode_check.setChecked(DEFAULTS.burst_mode)
|
self.burst_mode_check.setChecked(DEFAULTS.burst_mode)
|
||||||
|
self.strict_rows_check.setChecked(DEFAULTS.strict_rows)
|
||||||
|
|
||||||
# Hide the old T3R manual controls; the connect toggle becomes the panel button.
|
# Hide the old T3R manual controls; the connect toggle becomes the panel button.
|
||||||
for w in (
|
for w in (
|
||||||
@@ -979,6 +980,7 @@ class MainWindow(QMainWindow):
|
|||||||
self.bbd202_comport_edit.editingFinished.connect(self._persist_defaults)
|
self.bbd202_comport_edit.editingFinished.connect(self._persist_defaults)
|
||||||
self.oscope_ip_edit.editingFinished.connect(self._persist_defaults)
|
self.oscope_ip_edit.editingFinished.connect(self._persist_defaults)
|
||||||
self.burst_mode_check.toggled.connect(self._persist_defaults)
|
self.burst_mode_check.toggled.connect(self._persist_defaults)
|
||||||
|
self.strict_rows_check.toggled.connect(self._persist_defaults)
|
||||||
|
|
||||||
# Camera
|
# Camera
|
||||||
self.show_camera_toggle.toggled.connect(self._on_camera_toggle)
|
self.show_camera_toggle.toggled.connect(self._on_camera_toggle)
|
||||||
@@ -1137,6 +1139,7 @@ class MainWindow(QMainWindow):
|
|||||||
DEFAULTS.oscope_ip = self.oscope_ip_edit.text().strip()
|
DEFAULTS.oscope_ip = self.oscope_ip_edit.text().strip()
|
||||||
DEFAULTS.save_dir = self.scan_save_dir_edit.text().strip()
|
DEFAULTS.save_dir = self.scan_save_dir_edit.text().strip()
|
||||||
DEFAULTS.burst_mode = self.burst_mode_check.isChecked()
|
DEFAULTS.burst_mode = self.burst_mode_check.isChecked()
|
||||||
|
DEFAULTS.strict_rows = self.strict_rows_check.isChecked()
|
||||||
DEFAULTS.save()
|
DEFAULTS.save()
|
||||||
|
|
||||||
# ── Camera toggle ─────────────────────────────────────────────────────────
|
# ── Camera toggle ─────────────────────────────────────────────────────────
|
||||||
@@ -1261,6 +1264,7 @@ class MainWindow(QMainWindow):
|
|||||||
on_scan_active=lambda active: setattr(
|
on_scan_active=lambda active: setattr(
|
||||||
self._bbd_worker, "scanning_active", active),
|
self._bbd_worker, "scanning_active", active),
|
||||||
burst_mode=self.burst_mode_check.isChecked(),
|
burst_mode=self.burst_mode_check.isChecked(),
|
||||||
|
strict_rows=self.strict_rows_check.isChecked(),
|
||||||
)
|
)
|
||||||
self._scan_worker.moveToThread(self._scan_thread)
|
self._scan_worker.moveToThread(self._scan_thread)
|
||||||
self._scan_thread.started.connect(self._scan_worker.run)
|
self._scan_thread.started.connect(self._scan_worker.run)
|
||||||
|
|||||||
+17
-3
@@ -222,9 +222,23 @@ re-armed for each acquiring pass. Row boundaries inside the burst come from
|
|||||||
`ACQuire:NUMFRAMESACQuired?` sampled after each pass — the burst itself carries
|
`ACQuire:NUMFRAMESACQuired?` sampled after each pass — the burst itself carries
|
||||||
no row markers. See `core/scope_burst.py`.
|
no row markers. See `core/scope_burst.py`.
|
||||||
|
|
||||||
Either path squares each row up to the declared `n_frames` (zero-padding a
|
### Row packing
|
||||||
short row, dropping the tail of a long one), because the format has no per-row
|
|
||||||
length field and a mismatch would shift every later row.
|
The format has no per-row length field, so a row that over- or under-triggers
|
||||||
|
cannot be written as it arrived — that would shift every later row. Two
|
||||||
|
policies are selectable (`ScanEngine(strict_rows=…)`, a checkbox in the app),
|
||||||
|
and the choice is not recorded in the file:
|
||||||
|
|
||||||
|
| | Pad (default) | Strict |
|
||||||
|
|---|---|---|
|
||||||
|
| Short row | zero-padded to `n_frames`, warned | scan stops |
|
||||||
|
| Long row | trailing frames dropped, warned | scan stops |
|
||||||
|
|
||||||
|
Pad keeps a scan running through an occasional mis-trigger, at the cost that
|
||||||
|
the affected row is indistinguishable from a good one afterwards — nothing in
|
||||||
|
the file records that it was padded. Strict is for data runs where that
|
||||||
|
ambiguity is worse than a failed scan: it aborts before writing the row, so
|
||||||
|
the file always ends on a whole-row boundary.
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
|
|||||||
+80
-13
@@ -27,7 +27,8 @@ def make_plan(num_angles=1, y_delta=0.005):
|
|||||||
|
|
||||||
|
|
||||||
def build(tmp_path, num_angles=1, callbacks=None, resume=None, plan=None,
|
def build(tmp_path, num_angles=1, callbacks=None, resume=None, plan=None,
|
||||||
burst_mode=False, max_frames=4096, out_name="out.sras", **kw):
|
burst_mode=False, max_frames=4096, out_name="out.sras",
|
||||||
|
strict_rows=False, **kw):
|
||||||
trace = Trace()
|
trace = Trace()
|
||||||
scope = FakeScope(trace, samples_per_frame=SPF, max_frames=max_frames)
|
scope = FakeScope(trace, samples_per_frame=SPF, max_frames=max_frames)
|
||||||
stage = FakeStage(trace, scope=scope)
|
stage = FakeStage(trace, scope=scope)
|
||||||
@@ -37,7 +38,7 @@ def build(tmp_path, num_angles=1, callbacks=None, resume=None, plan=None,
|
|||||||
engine = ScanEngine(stage, scope, rotator, plan, tmp_path / out_name,
|
engine = ScanEngine(stage, scope, rotator, plan, tmp_path / out_name,
|
||||||
resume=resume,
|
resume=resume,
|
||||||
callbacks=callbacks or ScanCallbacks(),
|
callbacks=callbacks or ScanCallbacks(),
|
||||||
burst_mode=burst_mode)
|
burst_mode=burst_mode, strict_rows=strict_rows)
|
||||||
return engine, trace, plan
|
return engine, trace, plan
|
||||||
|
|
||||||
|
|
||||||
@@ -356,6 +357,26 @@ def test_burst_preflight_rejects_a_dark_laser(tmp_path):
|
|||||||
engine.run()
|
engine.run()
|
||||||
|
|
||||||
|
|
||||||
|
def _clip_one_row(engine, which_pass=2, lost=1):
|
||||||
|
"""Make one acquiring pass come up `lost` frames short.
|
||||||
|
|
||||||
|
`which_pass` counts acquiring passes from 1, so the default clips the
|
||||||
|
second data row (row 2) — far enough in that a mishandled short row shows
|
||||||
|
up as a shift in the rows after it.
|
||||||
|
"""
|
||||||
|
scope = engine._scope
|
||||||
|
real_acquire = scope.acquire_frames
|
||||||
|
passes = {"n": 0}
|
||||||
|
|
||||||
|
def clipped(n):
|
||||||
|
if scope._running:
|
||||||
|
passes["n"] += 1
|
||||||
|
if passes["n"] == which_pass:
|
||||||
|
n -= lost
|
||||||
|
real_acquire(n)
|
||||||
|
scope.acquire_frames = clipped
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.parametrize("burst_mode", [False, True])
|
@pytest.mark.parametrize("burst_mode", [False, True])
|
||||||
def test_short_row_is_padded_to_declared_frame_count(tmp_path, burst_mode):
|
def test_short_row_is_padded_to_declared_frame_count(tmp_path, burst_mode):
|
||||||
"""A clipped row must not shift every later row in the file.
|
"""A clipped row must not shift every later row in the file.
|
||||||
@@ -374,17 +395,7 @@ def test_short_row_is_padded_to_declared_frame_count(tmp_path, burst_mode):
|
|||||||
# acquiring-pass count below identical in both modes.
|
# acquiring-pass count below identical in both modes.
|
||||||
engine._preflight_done = True
|
engine._preflight_done = True
|
||||||
|
|
||||||
scope = engine._scope
|
_clip_one_row(engine)
|
||||||
real_acquire = scope.acquire_frames
|
|
||||||
passes = {"n": 0}
|
|
||||||
|
|
||||||
def clipped(n):
|
|
||||||
if scope._running:
|
|
||||||
passes["n"] += 1
|
|
||||||
if passes["n"] == 2: # second data row loses one frame
|
|
||||||
n -= 1
|
|
||||||
real_acquire(n)
|
|
||||||
scope.acquire_frames = clipped
|
|
||||||
|
|
||||||
result = engine.run()
|
result = engine.run()
|
||||||
|
|
||||||
@@ -398,6 +409,62 @@ def test_short_row_is_padded_to_declared_frame_count(tmp_path, burst_mode):
|
|||||||
assert bytes(sras.load_row(0, 2, 0)[0]) != bytes(SPF)
|
assert bytes(sras.load_row(0, 2, 0)[0]) != bytes(SPF)
|
||||||
|
|
||||||
|
|
||||||
|
# ── Strict row packing ───────────────────────────────────────────────────────
|
||||||
|
|
||||||
|
@pytest.mark.parametrize("burst_mode", [False, True])
|
||||||
|
def test_strict_row_packing_aborts_on_a_short_row(tmp_path, burst_mode):
|
||||||
|
"""Strict mode fails the scan instead of silently squaring a row up.
|
||||||
|
|
||||||
|
The default padding keeps the file readable but makes a mis-triggered row
|
||||||
|
indistinguishable from a good one after the fact, since v6 records no
|
||||||
|
per-row frame count. Strict mode trades the salvaged rows for knowing.
|
||||||
|
"""
|
||||||
|
plan = make_plan(**BURST_PLAN)
|
||||||
|
engine, _, _ = build(tmp_path, plan=plan, burst_mode=burst_mode,
|
||||||
|
max_frames=BURST_MAX_FRAMES, strict_rows=True)
|
||||||
|
engine._preflight_done = True
|
||||||
|
_clip_one_row(engine)
|
||||||
|
|
||||||
|
with pytest.raises(RuntimeError, match="Row 2: 3 frames acquired, 4 expected"):
|
||||||
|
engine.run()
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize("burst_mode", [False, True])
|
||||||
|
def test_strict_row_packing_does_not_disturb_a_clean_scan(tmp_path, burst_mode):
|
||||||
|
"""Strict mode is inert when every row acquires what it declared."""
|
||||||
|
plan = make_plan(**BURST_PLAN)
|
||||||
|
engine, _, _ = build(tmp_path, plan=plan, burst_mode=burst_mode,
|
||||||
|
max_frames=BURST_MAX_FRAMES, strict_rows=True)
|
||||||
|
engine._preflight_done = True
|
||||||
|
|
||||||
|
result = engine.run()
|
||||||
|
|
||||||
|
assert result.rows_written == plan.total_rows
|
||||||
|
sras = SrasFile(tmp_path / "out.sras")
|
||||||
|
assert [s.status for s in sras.angle_status()] == ["OK"]
|
||||||
|
|
||||||
|
|
||||||
|
def test_strict_row_packing_writes_nothing_for_the_failed_row(tmp_path):
|
||||||
|
"""The abort must not leave a half-written row behind.
|
||||||
|
|
||||||
|
CH1 leads SCAN_CHANNELS, so the frame count is known before any of the
|
||||||
|
row's channels are written — the file should end on a whole-row boundary.
|
||||||
|
"""
|
||||||
|
plan = make_plan(**BURST_PLAN)
|
||||||
|
engine, _, _ = build(tmp_path, plan=plan, strict_rows=True)
|
||||||
|
engine._preflight_done = True
|
||||||
|
_clip_one_row(engine)
|
||||||
|
|
||||||
|
with pytest.raises(RuntimeError, match="Strict row packing"):
|
||||||
|
engine.run()
|
||||||
|
|
||||||
|
# Row 1 was written in full; row 2 aborted before writing anything, so
|
||||||
|
# the file ends exactly on a row boundary.
|
||||||
|
sras = SrasFile(tmp_path / "out.sras")
|
||||||
|
written = (tmp_path / "out.sras").stat().st_size - sras.data_start_offset
|
||||||
|
assert written == sras.row_bytes(0)
|
||||||
|
|
||||||
|
|
||||||
def test_engine_imports_without_qt():
|
def test_engine_imports_without_qt():
|
||||||
"""The engine must be usable from a non-Qt front end."""
|
"""The engine must be usable from a non-Qt front end."""
|
||||||
import subprocess
|
import subprocess
|
||||||
|
|||||||
Reference in New Issue
Block a user