tka/sras-viewer
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Stop discarding precomputed FFT data on view switches

Browse Source 
The display path hardcoded row_avg_n=0 when checking a file's stored FFT
cache, since the main window has no row-averaging control (only the
batch-only RowAverageFftOptionsDialog). Once a file was batch-computed with
row-averaging, every view switch saw a phantom provenance mismatch and
silently launched a full raw recompute, discarding the precomputed data.

The display now asks for the stored image at the settings it was actually
computed under (bg-sub/pad/row-averaging), rather than the window's live
controls, so a stored or already-computed image is always shown once
present. Those controls now only affect a first-time compute for an
uncached angle or an explicit batch recompute -- never what's already on
screen. DC threshold is unaffected: it stays live, since re-masking a
cached image is free.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Thomas Ales
2026-08-09 18:21:34 -05:00
parent c30c8b1815
commit d5914b5793
4 changed files with 153 additions and 54 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/design.md
+19
View File
@@ -164,6 +164,25 @@ batching), `_cache_mismatch_notes` says so in the scan info panel, because
the symptom otherwise is just "the file I pre-computed got slow again" with
no visible cause.
That divergence note is informational only, not a warning of an impending
recompute. The *display* path (`_stored_fft_image`) never asks
`cached_rf_image` whether a stored image matches the window's live
bg-sub/pad/row-averaging controls — it asks whether the image matches its
*own* recorded settings (`sras.precomputed_bg_sub`/`precomputed_pad_factor`/
`precomputed_row_avg_n`), which is always true whenever a stored image
exists. So presence alone decides whether it's shown; the live controls
never gate it. They still matter for two things: a genuinely never-computed
angle's first live compute, and an explicit batch recompute — both of which
read the live controls and produce new stored data, at which point it's the
new data's *own* settings that get self-matched from then on. This is what
keeps a view switch (angle, channel, or flipping bg-sub/pad) from ever
discarding precomputed data — only an explicit batch recompute does, and it
already reloads the file afterward so the new data displays immediately.
Row-averaging has no live control to diverge from in the first place (it's
only ever set inside the batch dialog), so it never appears in the
divergence note — the "Cached images" line's own `row-averaged n=…` phrase
already covers it.
### Two caches, in cost order
`_refresh_display` consults this window's in-session `_fft_cache`/`_dc_cache`
sras_viewer/main_window.py
+67 -33
View File
@@ -90,10 +90,13 @@ class SrasViewerWindow(QMainWindow):
# computed lazily (with a progress popup) the first time an
# angle/threshold combination is viewed — using the cached DC4
# image to skip the FFT entirely for masked-out pixels — and
# cached per (angle, bg_sub, n_fft, threshold) so revisiting the
# same combination is free.
# cached per (angle, threshold) so revisiting the same combination
# is free. bg-sub/pad are deliberately not part of the key: once an
# angle has any FFT image (live or from the file's own stored
# cache), it stays displayed regardless of those controls — see
# _fft_cache_key.
self._dc_cache: dict[tuple[int, int], np.ndarray] = {}
self._fft_cache: dict[tuple[int, bool, int | None, float], np.ndarray] = {}
self._fft_cache: dict[tuple[int, float], np.ndarray] = {}
self._dc_generation: int = 0
# Angle alignment ("Fusion" menu)
@@ -275,7 +278,9 @@ class SrasViewerWindow(QMainWindow):
self.chk_bg_sub.setEnabled(False)
self.chk_bg_sub.setToolTip(
"Subtract the stored background waveform from each CH1 frame\n"
"before computing the FFT (v4+ files only)."
"before computing the FFT (v4+ files only). Applies to angles\n"
"not yet computed and to future batch recomputes — it does not\n"
"change an image already shown or already stored in the file."
)
self.chk_bg_sub.toggled.connect(self._on_bg_sub_toggled)
vl.addWidget(self.chk_bg_sub)
@@ -622,16 +627,20 @@ class SrasViewerWindow(QMainWindow):
self.lbl_frame_warn.setText("\n".join(notes))
def _cache_mismatch_notes(self) -> list[str]:
"""Why the file's stored FFT images can't serve the current view, if
they can't. Padding, bg-sub and row-averaging are all baked into the
stored numbers, so changing any of them silently sends every angle
back through a real FFT — worth saying out loud rather than leaving
the user to wonder why a file they batch-computed got slow.
"""Informational only: whether the file's stored FFT cache was
computed under different bg-sub/pad settings than these controls
currently say. The display always shows the stored image as-is
regardless (see _stored_fft_image) — these controls only affect a
future live compute for an angle with nothing cached yet, or an
explicit batch recompute, never what's already on screen.
row_avg_n is compared against the file's own recorded value (a
self-match), so it never contributes a reason here — there's no
live control for it to diverge from, and the "Cached images" line
above already reports it.
Asks compute for the reasons rather than restating the accept rule,
so a new provenance field can only be added in one place. The display
always asks for a raw per-pixel image, since row-averaging is a batch
option with no display control.
so a new provenance field can only be added in one place.
"""
s = self._sras
if s is None or all(x is None for x in s.precomputed_freq_mhz):
@@ -639,11 +648,13 @@ class SrasViewerWindow(QMainWindow):
reasons = compute.cache_mismatch_reasons(
s, n_fft=self._current_n_fft(),
apply_bg_sub=self.chk_bg_sub.isChecked(), row_avg_n=0)
apply_bg_sub=self.chk_bg_sub.isChecked(),
row_avg_n=s.precomputed_row_avg_n)
if not reasons:
return []
return ["! Cached FFT unusable for this view — " + "; ".join(reasons)
+ ". FFT angles will recompute."]
return ["Note: current bg-sub/pad controls differ from the stored "
"cache — " + "; ".join(reasons) + ". Shown as stored; use "
"Batch Compute to recompute with these settings."]
# ------------------------------------------------------------------
# Controls
@@ -691,11 +702,12 @@ class SrasViewerWindow(QMainWindow):
self._on_view_changed()
def _on_bg_sub_toggled(self):
# Background subtraction changes the FFT input, so it genuinely
# invalidates the cached raw FFT (the cache key includes it) —
# _refresh_display() recomputes only on a miss for the new state.
# bg-sub no longer gates the display: it only affects a future live
# compute for an angle with nothing cached yet, or an explicit batch
# recompute — never what's already shown. Just keep the info panel's
# divergence note current.
if self._is_fft_mode():
self._refresh_display()
self._update_scan_info_labels()
def _on_grating_changed(self):
# Grating is a pure post-multiply on the cached frequency image —
@@ -889,8 +901,15 @@ class SrasViewerWindow(QMainWindow):
return freq_mhz
def _fft_cache_key(self, angle_idx: int) -> tuple:
return (angle_idx, self.chk_bg_sub.isChecked(), self._current_n_fft(),
self.spin_threshold_mv.value())
"""Keyed by angle and DC threshold only. Once any FFT image exists
for an angle this session — live-computed or pulled from the file's
own stored cache — it stays the displayed image for that angle
regardless of later bg-sub/pad toggles; those only affect a future
live compute for an angle with nothing cached yet, or an explicit
batch recompute (see _stored_fft_image). Threshold stays in the key
because re-masking against it is free and meant to stay interactive
(see _on_threshold_changed)."""
return (angle_idx, self.spin_threshold_mv.value())
def _aligned_cache_key(self, angle_idx: int, ch_idx: int) -> tuple:
"""Mirrors _fft_cache's key granularity so a stale aligned image is
@@ -916,20 +935,35 @@ class SrasViewerWindow(QMainWindow):
return cached
def _stored_fft_image(self, angle_idx: int) -> np.ndarray | None:
"""The open file's own stored peak-frequency image for the current
view, masked and ready to display, or None if the file has nothing
that answers this exact view.
"""The open file's own stored peak-frequency image for this angle,
masked and ready to display, or None if the file has nothing stored
for it.
Asks for the image at the settings it was actually computed under
(sras.precomputed_bg_sub / precomputed_pad_factor /
precomputed_row_avg_n) rather than the window's live bg-sub/pad
controls, so a stored image is always shown once present — those
controls never gate whether it's used, only what a *future* compute
produces. See _cache_mismatch_notes for the informational (non-
blocking) note when the live controls diverge from what's shown.
Only the DC threshold is taken live: re-masking a stored image
against it is free, unlike bg-sub/pad/row-averaging which are baked
irreversibly into the stored numbers.
allow_dc_recompute=False keeps this off the I/O path: if the mask
would mean reading a whole CH4 channel, this declines and the caller
falls through to the background worker, which reaches the same stored
image via compute_rf_image and pays for the mask off the GUI thread.
"""
s = self._sras
n_fft = (s.samples_per_frame * s.precomputed_pad_factor
if s.precomputed_pad_factor > 1 else None)
return compute.cached_rf_image(
self._sras, angle_idx,
s, angle_idx,
dc_threshold_mv=self.spin_threshold_mv.value(),
apply_bg_sub=self.chk_bg_sub.isChecked(),
n_fft=self._current_n_fft(),
apply_bg_sub=s.precomputed_bg_sub,
n_fft=n_fft,
row_avg_n=s.precomputed_row_avg_n,
dc4_mv=self._dc_cache.get((angle_idx, CH4_IDX)),
allow_dc_recompute=False)
@@ -1109,8 +1143,7 @@ class SrasViewerWindow(QMainWindow):
ch_idx = self._pending_ch
if ch_idx in CH1_DERIVED_MODES:
self._fft_cache[(angle_idx, self._pending_bg_sub,
self._current_n_fft(), self._pending_threshold)] = result
self._fft_cache[(angle_idx, self._pending_threshold)] = result
img = self._scale_for_display(result, ch_idx)
else:
img = result
@@ -1427,11 +1460,12 @@ class SrasViewerWindow(QMainWindow):
self._fft_pad_factor = dlg.get_pad_factor()
self._settings.setValue("fft/backend", compute.get_fft_backend())
self._settings.setValue("fft/pad_factor", self._fft_pad_factor)
# Pad factor changes the FFT bin count, so it genuinely invalidates
# the cached raw FFT (part of the cache key) — _refresh_display()
# recomputes only on a cache miss.
# Pad factor no longer gates the display: it only affects a future
# live compute for an angle with nothing cached yet, or an explicit
# batch recompute — never what's already shown. Just keep the info
# panel's divergence note current.
if self._is_fft_mode():
self._refresh_display()
self._update_scan_info_labels()
# ------------------------------------------------------------------
tests/test_gui.py
+14 -5
View File
@@ -222,16 +222,25 @@ def test_threshold_change_recomputes(ctx):
def test_bg_sub_toggle(ctx):
"""bg-sub no longer gates the display: it only affects a future live
compute for an angle with nothing cached yet, or an explicit batch
recompute. Toggling it on an angle that already has an FFT image must
leave that image on screen, untouched."""
win = ctx.win
n_before = len(win._fft_cache)
img_before = win._current_image
win.chk_bg_sub.setChecked(False)
assert wait_until(
lambda: not win._job_running("compute") and len(win._fft_cache) > n_before), \
"recomputed without bg-sub"
win.chk_bg_sub.setChecked(True)
pump(200)
assert not win._job_running("compute"), \
"returning to bg-sub was a cache hit (no recompute)"
"toggling bg-sub alone must not dispatch a recompute"
assert len(win._fft_cache) == n_before, "no new cache entry from the toggle"
assert np.array_equal(win._current_image, img_before), \
"displayed image unchanged by the bg-sub toggle"
win.chk_bg_sub.setChecked(True)
pump(200)
assert not win._job_running("compute")
assert len(win._fft_cache) == n_before
assert np.array_equal(win._current_image, img_before)
def test_roi_and_csv_export(ctx):
tests/test_stored_cache.py
+53 -16
View File
@@ -256,11 +256,16 @@ def test_viewer_shows_stored_angles_without_computing(rig, no_fft, monkeypatch):
rig.fresh[0] * win.spin_grating_um.value(), atol=1e-3)
assert dispatched == [] and not no_fft
# ...but a setting the stored image cannot serve must still recompute,
# or the fast path would be showing the wrong picture.
# ...and a live control that no longer matches the stored image's own
# provenance must NOT force a recompute either — the stored image is
# shown as-is; only an explicit batch recompute changes what's shown.
win.combo_channel.setCurrentIndex(CH1_IDX)
pump(60)
win.chk_bg_sub.setChecked(False)
assert wait_until(lambda: not win._job_running("compute") and bool(no_fft)), \
"bg-sub off falls through to a real FFT"
pump(200)
assert not win._job_running("compute") and not no_fft, \
"bg-sub off still shows the stored image, no real FFT"
assert np.allclose(win._current_image, rig.fresh[0], atol=1e-3)
finally:
win.close()
pump(300)
@@ -322,16 +327,21 @@ def test_batch_caches_at_the_viewers_pad_factor(tmp_path, no_fft, monkeypatch):
f"no recompute at pad 10 (jobs={dispatched}, fft={no_fft})"
assert "unusable" not in win.lbl_frame_warn.text()
# Change the pad and the cache legitimately stops applying — and the
# info panel has to say so rather than leave it a mystery.
# Change the live pad control so it no longer matches the stored
# image's own provenance — the info panel has to say so rather than
# leave it a mystery, but the stored pad-10 image keeps displaying;
# only an explicit batch recompute would ever produce a pad-4 one.
win._fft_pad_factor = 4
win._update_scan_info_labels()
assert "Cached FFT unusable" in win.lbl_frame_warn.text(), \
assert "differ from the stored cache" in win.lbl_frame_warn.text(), \
win.lbl_frame_warn.text()
assert "pad 10x" in win.lbl_frame_warn.text()
last_angle = win._current_angle
win._refresh_display()
assert wait_until(lambda: not win._job_running("compute") and bool(no_fft)), \
"pad 4 recomputes rather than reusing the pad-10 cache"
assert wait_until(lambda: not win._job_running("compute")), "settled"
assert not no_fft, "no real FFT ran — the pad-10 cache still served the view"
assert np.allclose(win._current_image, expected[last_angle], atol=1e-3), \
"pad-10 cache still shown after the live pad control diverged"
finally:
win.close()
pump(300)
@@ -500,16 +510,17 @@ def test_cach_v1_backward_compat_defaults_row_avg_n_zero(tmp_path):
"a v1 tail (predating this feature) can never satisfy a row-averaged request"
def test_viewer_batch_row_average_dispatch(tmp_path, monkeypatch):
def test_viewer_batch_row_average_dispatch(tmp_path, monkeypatch, no_fft):
"""Driving the new 'Batch Compute Row-Averaged FFT and Store' action
end-to-end through the real menu handler: dialog values reach the
worker, the worker reaches cache_file, and the written file is
self-describing afterward. Deliberately does not assert anything about
whether viewing an angle afterward dispatches a compute job -- that is
a separate, pre-existing gap in _refresh_display shared with the plain
DC/FFT batch actions (see test_viewer_shows_stored_angles_without_computing
/ test_viewer_shows_stored_dc_without_computing above), not something
row-averaging introduces or is responsible for fixing."""
self-describing afterward. Also the exact scenario the row-averaged-FFT
recompute bug reported: before the fix, the display always asked for
row_avg_n=0 regardless of what the file actually had stored, so viewing
an angle after this batch action saw a phantom mismatch and launched a
full raw recompute on every view switch. This asserts that no longer
happens -- the stored row-averaged image is shown directly, with no
dispatched compute job and no real FFT."""
path = tmp_path / "rowavg_gui.sras"
gen.write(path, n_angles=2, seed=26, samples_per_frame=128)
@@ -531,6 +542,12 @@ def test_viewer_batch_row_average_dispatch(tmp_path, monkeypatch):
app = QApplication.instance() or QApplication([]) # noqa: F841
win = SrasViewerWindow()
win.show()
dispatched = []
original_start = type(win)._start_compute
monkeypatch.setattr(type(win), "_start_compute",
lambda self: (dispatched.append(self.spin_angle.value()),
original_start(self))[1])
try:
win._load_file(str(path))
assert wait_until(lambda: win._sras is not None), "file loaded"
@@ -553,6 +570,26 @@ def test_viewer_batch_row_average_dispatch(tmp_path, monkeypatch):
apply_bg_sub=win.chk_bg_sub.isChecked(),
row_avg_n=6)
assert expected is not None, "the batch write left a readable row-averaged cache"
# The regression this batch action used to leave unfixed: viewing an
# angle afterward must show the stored row-averaged image directly,
# never fall through to a real (raw) recompute.
no_fft.clear()
dispatched.clear()
win.combo_channel.setCurrentIndex(CH1_IDX)
assert wait_until(lambda: win._current_ch == CH1_IDX), "CH1 displayed"
for a in range(win._sras.n_angles):
win.spin_angle.setValue(a)
pump(60)
assert np.allclose(win._current_image,
compute.cached_rf_image(
win._sras, a,
dc_threshold_mv=win.spin_threshold_mv.value(),
apply_bg_sub=win.chk_bg_sub.isChecked(),
row_avg_n=6), atol=1e-3), \
f"angle {a} shows the stored row-averaged image"
assert dispatched == [] and not no_fft, \
f"no recompute for row-averaged angles (jobs={dispatched}, fft={no_fft})"
finally:
win.close()
pump(300)