Implement FFT pad-factor caching and the stored-cache display fast path
tests/test_stored_cache.py exercised two features that were never built, so six of its tests had been failing on main. Both are now implemented. Pad-factor caching. A padded view could not use a stored FFT cache at all: the store was pad 1 by definition and cached_rf_image rejected any n_fft outright, so a user working at a pad factor got nothing from batch-computing a file. CACH tail v3 records the pad the images were resolved at, cache_file computes at a requested pad, and the batch actions pass the viewer's own pad down — while still refusing a store resolved at a different pad, since a padded FFT interpolates between the natural bins and so resolves genuinely different peak frequencies. v1/v2 tails read as pad 1 and keep working. Stored-cache dispatch. _refresh_display only ever consulted this window's in-session dicts, so after a batch every angle change still queued a worker and a progress popup for an image already on disk — the exact cost the batch was run to avoid. It now checks the file's own DC/FFT blocks first, asking with allow_dc_recompute=False so the GUI thread never touches I/O. When the stored cache genuinely cannot serve the view, the scan info panel says why rather than leaving the silent recompute a mystery. Two bugs surfaced on the way: - The angle spinbox was wired on editingFinished, which QAbstractSpinBox emits only on Return or focus-out — never on a step. Clicking its arrows, the ordinary way to walk a scan, moved the number and left the image behind. Now valueChanged with keyboard tracking off, which fires on a step and once on commit, but not per keystroke mid-typing. Every existing GUI test called _on_view_changed() by hand and so could not have caught this; two new tests pin it and fail against the old wiring. - A plain "fft" batch over a file previously cached with fft_rowavg carried the old row_avg_n forward, labelling raw images as row-averaged. It now writes row_avg_n=0 explicitly. Verified: 111 passed (was 103 passed / 6 failed), and tools/check_equivalence.py is byte-identical to the pre-change baseline. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Thomas Ales
@@ -224,7 +224,14 @@ class SrasViewerWindow(QMainWindow):
|
||||
self.spin_angle.setRange(0, 0)
|
||||
self.spin_angle.setEnabled(False)
|
||||
self.spin_angle.setMinimumWidth(64)
|
||||
self.spin_angle.editingFinished.connect(self._on_view_changed)
|
||||
# valueChanged with keyboard tracking off, not editingFinished: the
|
||||
# latter fires only on Return or focus-out, so stepping the angle (an
|
||||
# arrow click or Up/Down, the ordinary way to walk a scan) changed the
|
||||
# number and left the image behind. Tracking off is what keeps
|
||||
# valueChanged from also firing per keystroke mid-typing, which on a
|
||||
# large scan would launch a compute for every intermediate angle.
|
||||
self.spin_angle.setKeyboardTracking(False)
|
||||
self.spin_angle.valueChanged.connect(self._on_view_changed)
|
||||
self.lbl_angle_deg = QLabel("—")
|
||||
angle_field = QWidget()
|
||||
ar = QHBoxLayout(angle_field)
|
||||
@@ -606,15 +613,49 @@ class SrasViewerWindow(QMainWindow):
|
||||
bg_note = " (bg-sub)" if s.precomputed_bg_sub else " (no bg-sub)"
|
||||
avg_note = (f", row-averaged n={s.precomputed_row_avg_n}"
|
||||
if s.precomputed_row_avg_n else "")
|
||||
pad_note = (f", pad {s.precomputed_pad_factor}x"
|
||||
if s.precomputed_pad_factor > 1 else "")
|
||||
notes.append(
|
||||
f"Cached images: DC {n_dc}/{s.n_angles} angles, "
|
||||
f"FFT {n_fft}/{s.n_angles} angles{bg_note if n_fft else ''}"
|
||||
f"{avg_note if n_fft else ''} "
|
||||
f"{avg_note if n_fft else ''}{pad_note if n_fft else ''} "
|
||||
"— display is instant for cached angles")
|
||||
notes += self._cache_mismatch_notes()
|
||||
elif s.version == 7:
|
||||
notes.append("v7 format: no cache blocks stored yet")
|
||||
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.
|
||||
|
||||
Deliberately mirrors cached_rf_image's accept rule; the display
|
||||
always asks for a raw per-pixel image, since row-averaging is a batch
|
||||
option with no display control.
|
||||
"""
|
||||
s = self._sras
|
||||
if s is None or all(x is None for x in s.precomputed_freq_mhz):
|
||||
return []
|
||||
|
||||
reasons = []
|
||||
if compute.pad_factor_for(s, self._current_n_fft()) != s.precomputed_pad_factor:
|
||||
reasons.append(f"stored at pad {s.precomputed_pad_factor}x, "
|
||||
f"viewing at pad {self._fft_pad_factor}x")
|
||||
if s.precomputed_bg_sub != (self.chk_bg_sub.isChecked()
|
||||
and s.background is not None):
|
||||
reasons.append("stored with background subtraction "
|
||||
f"{'on' if s.precomputed_bg_sub else 'off'}")
|
||||
if s.precomputed_row_avg_n:
|
||||
reasons.append(f"stored row-averaged (n={s.precomputed_row_avg_n}), "
|
||||
"the display shows raw per-pixel FFTs")
|
||||
if not reasons:
|
||||
return []
|
||||
return ["! Cached FFT unusable for this view — " + "; ".join(reasons)
|
||||
+ ". FFT angles will recompute."]
|
||||
|
||||
# ------------------------------------------------------------------
|
||||
# Controls
|
||||
# ------------------------------------------------------------------
|
||||
@@ -885,23 +926,60 @@ class SrasViewerWindow(QMainWindow):
|
||||
self._aligned_cache[key] = cached
|
||||
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.
|
||||
|
||||
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.
|
||||
"""
|
||||
return compute.cached_rf_image(
|
||||
self._sras, angle_idx,
|
||||
dc_threshold_mv=self.spin_threshold_mv.value(),
|
||||
apply_bg_sub=self.chk_bg_sub.isChecked(),
|
||||
n_fft=self._current_n_fft(),
|
||||
dc4_mv=self._dc_cache.get((angle_idx, CH4_IDX)),
|
||||
allow_dc_recompute=False)
|
||||
|
||||
def _refresh_display(self):
|
||||
"""Show the image for the current angle/channel/threshold, using
|
||||
cached data whenever possible and only falling back to a background
|
||||
compute (with progress popup) when genuinely nothing is cached yet."""
|
||||
compute (with progress popup) when genuinely nothing is cached yet.
|
||||
|
||||
Two caches are consulted, in cost order: this window's own in-session
|
||||
dicts, then the file's stored v5/v7 cache blocks. The second is what
|
||||
makes a batch-computed file worth having — without it every angle
|
||||
change queued a worker and a progress popup for an image already on
|
||||
disk, which is exactly the cost the batch was run to avoid.
|
||||
"""
|
||||
if self._sras is None:
|
||||
return
|
||||
angle_idx = self.spin_angle.value()
|
||||
ch_idx = self.combo_channel.currentIndex()
|
||||
|
||||
if ch_idx in CH1_DERIVED_MODES:
|
||||
raw = self._fft_cache.get(self._fft_cache_key(angle_idx))
|
||||
key = self._fft_cache_key(angle_idx)
|
||||
raw = self._fft_cache.get(key)
|
||||
if raw is None:
|
||||
raw = self._stored_fft_image(angle_idx)
|
||||
if raw is not None:
|
||||
# Masking the stored image is cheap but not free; keep the
|
||||
# result so revisiting this angle costs nothing at all.
|
||||
self._fft_cache[key] = raw
|
||||
if raw is not None:
|
||||
self._show_image_now(self._scale_for_display(raw, ch_idx),
|
||||
angle_idx, ch_idx)
|
||||
return
|
||||
else:
|
||||
# A stored DC image needs no post-processing, so the file's own
|
||||
# parsed array is served directly — as the compute path already
|
||||
# does, _current_image is treated as read-only by every consumer.
|
||||
cached = self._dc_cache.get((angle_idx, ch_idx))
|
||||
if cached is None:
|
||||
cached = self._sras.cached_dc_mv(angle_idx, ch_idx)
|
||||
if cached is not None:
|
||||
self._show_image_now(cached, angle_idx, ch_idx)
|
||||
return
|
||||
@@ -1171,7 +1249,10 @@ class SrasViewerWindow(QMainWindow):
|
||||
return
|
||||
|
||||
self._batch_errors = []
|
||||
worker = BatchCacheWorker(paths, mode, self.chk_bg_sub.isChecked())
|
||||
# Cache the FFT at the pad the viewer is actually displaying at,
|
||||
# otherwise the batch stores images this window can never use.
|
||||
worker = BatchCacheWorker(paths, mode, self.chk_bg_sub.isChecked(),
|
||||
pad_factor=self._fft_pad_factor)
|
||||
started = self._run_worker(
|
||||
Jobs.BATCH, worker,
|
||||
connect=(
|
||||
@@ -1212,7 +1293,8 @@ class SrasViewerWindow(QMainWindow):
|
||||
|
||||
self._batch_errors = []
|
||||
worker = BatchCacheWorker(paths, "fft_rowavg", self.chk_bg_sub.isChecked(),
|
||||
dc_threshold_mv=threshold_mv, row_avg_n=n)
|
||||
dc_threshold_mv=threshold_mv, row_avg_n=n,
|
||||
pad_factor=self._fft_pad_factor)
|
||||
started = self._run_worker(
|
||||
Jobs.BATCH, worker,
|
||||
connect=(
|
||||
|
||||
Reference in New Issue
Block a user