From 626250e2a0488adfe5481f35e02c2645fffdf317 Mon Sep 17 00:00:00 2001 From: Peter Jacobson Date: Fri, 12 Jun 2026 08:15:58 +1000 Subject: [PATCH] Decouple streak pairs from the mains overlay; harmonics 0 disables mains MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Field feedback on the just-shipped streak pairs: they were only drawn (and therefore only draggable) while "Show mains-pickup overlay" was on, and the harmonics spin bottomed out at 1 — so using a custom pair forced at least one mains notch into the removal. Mains pickup (electrical noise) and custom streaks (scan-parameter noise, e.g. tip speed) are physically distinct signals; neither should require the other. - "Show streak pairs" checkbox next to the pair controls gates the custom lines independently of the mains overlay; adding a pair switches it on automatically. Hidden pairs no longer capture pan clicks. - The harmonics spin allows 0, and predict_mains_fft_positions returns no predictions for an explicit 0 (previously clamped to 1, silently forcing the fundamental in) — so a custom pair can be applied alone even when the scan speed is known, and the same params replay identically from provenance. Status line reports "Mains notches disabled (harmonics = 0)". Kernel + GUI tests: harmonics=0 prediction contract, custom-streak-only apply leaving the mains fundamental untouched, pair visibility without the overlay, hidden-pair click transparency, and the harmonics-0 apply path. Co-Authored-By: Claude Fable 5 --- .../gui/dialogs/fft_viewer_mains_mixin.py | 51 ++++++++++++++--- probeflow/processing/mains_pickup.py | 12 +++- tests/test_mains_pickup.py | 39 +++++++++++++ tests/test_mains_pickup_gui.py | 56 +++++++++++++++++++ 4 files changed, 148 insertions(+), 10 deletions(-) diff --git a/probeflow/gui/dialogs/fft_viewer_mains_mixin.py b/probeflow/gui/dialogs/fft_viewer_mains_mixin.py index 4e85e79..79db7d3 100644 --- a/probeflow/gui/dialogs/fft_viewer_mains_mixin.py +++ b/probeflow/gui/dialogs/fft_viewer_mains_mixin.py @@ -98,11 +98,14 @@ def _build_mains_tab(self) -> QWidget: self._mains_speed_spin.setMaximumWidth(_WIDE_FIELD_W) self._mains_harm_spin = QSpinBox() - self._mains_harm_spin.setRange(1, 20) + self._mains_harm_spin.setRange(0, 20) self._mains_harm_spin.setValue(3) self._mains_harm_spin.setEnabled(False) self._mains_harm_spin.setToolTip(_tip( - "Manual number of mains harmonics to mark when Auto is off.")) + "Manual number of mains harmonics to mark when Auto is off. " + "0 disables mains notches entirely — custom streak pairs then " + "apply alone, even with a known scan speed (mains pickup and " + "scan-parameter streaks are physically distinct signals).")) self._mains_harm_spin.valueChanged.connect(self._on_mains_changed) self._mains_harm_spin.setMaximumWidth(_FIELD_W) @@ -177,6 +180,17 @@ def _build_mains_tab(self) -> QWidget: # ── custom streak pairs (pickup at non-mains frequencies) ─────────────── custom_row = QHBoxLayout() + # Streak pairs are physically independent of mains pickup (scan- + # parameter noise vs electrical noise), so their visibility has its + # own toggle — previously they were only drawn when "Show mains-pickup + # overlay" was on, which forced at least one mains harmonic into the + # picture just to see a custom pair. + self._mains_streaks_cb = QCheckBox("Show streak pairs") + self._mains_streaks_cb.setChecked(False) + self._mains_streaks_cb.setToolTip(_tip( + "Show the custom ±q streak pairs on the FFT, independent of the " + "mains-pickup overlay. Auto-enabled when a pair is added.")) + self._mains_streaks_cb.toggled.connect(self._on_mains_changed) self._mains_add_streak_btn = QPushButton("Add streak pair") self._mains_add_streak_btn.setToolTip(_tip( "Add a symmetric ±q pair of notch lines for a vertical streak " @@ -190,6 +204,7 @@ def _build_mains_tab(self) -> QWidget: self._mains_remove_streak_btn.clicked.connect(self._on_mains_remove_streak) for b in (self._mains_add_streak_btn, self._mains_remove_streak_btn): b.setMaximumWidth(130) + custom_row.addWidget(self._mains_streaks_cb) custom_row.addWidget(self._mains_add_streak_btn) custom_row.addWidget(self._mains_remove_streak_btn) custom_row.addStretch(1) @@ -303,9 +318,16 @@ def _draw_mains_overlay(self) -> None: Rebuilt on every FFT redraw (the axes are cleared by ``ax.cla()``). """ self._mains_artists = [] - if not getattr(self, "_mains_overlay_cb", None): + if self._qx is None or self._qy is None: return - if not self._mains_overlay_cb.isChecked() or self._qx is None or self._qy is None: + # Independent visibility gates: mains predictions and custom streak + # pairs are physically different signals (electrical pickup vs + # scan-parameter noise) — neither toggle requires the other. + overlay_cb = getattr(self, "_mains_overlay_cb", None) + streaks_cb = getattr(self, "_mains_streaks_cb", None) + show_mains = overlay_cb is not None and overlay_cb.isChecked() + show_streaks = streaks_cb is not None and streaks_cb.isChecked() + if not (show_mains or show_streaks): return min_q = self._mains_min_q_nm_inv() half_w = float(self._mains_radius_spin.value()) * self._mains_q_per_px() @@ -339,12 +361,14 @@ def _draw_pair(q: float, color: str, style: str) -> None: ) self._mains_artists.append(band) - for p in self._mains_predictions(): - _draw_pair(p["q_nm_inv"], "#f9e2af", "--") + if show_mains: + for p in self._mains_predictions(): + _draw_pair(p["q_nm_inv"], "#f9e2af", "--") # User streak pairs: solid cyan so they read as hand-placed, not # predicted; draggable on the FFT while the Mains tab is active. - for q in self._mains_custom_streaks(): - _draw_pair(q, "#89dceb", "-") + if show_streaks: + for q in self._mains_custom_streaks(): + _draw_pair(q, "#89dceb", "-") def _on_mains_changed(self) -> None: """Fast path: refresh the overlay + status when a control changes.""" @@ -374,6 +398,10 @@ def _update_mains_status(self) -> None: "Scan speed unavailable; enter nm/s to show the mains overlay." + custom_note) return + if self._mains_harmonics() == 0: + self._mains_status_lbl.setText( + "Mains notches disabled (harmonics = 0)." + custom_note) + return preds = self._mains_predictions() if not preds: self._mains_status_lbl.setText( @@ -419,6 +447,10 @@ def _on_mains_add_streak(self) -> None: q_new = q_max * (0.35 + 0.12 * (n % 5)) self._mains_custom_streaks().append(q_new) self._mains_remove_streak_btn.setEnabled(True) + # Adding a pair means the user wants to see it — switch its own + # visibility on without touching the mains overlay. + if not self._mains_streaks_cb.isChecked(): + self._mains_streaks_cb.setChecked(True) # triggers _on_mains_changed self._on_mains_changed() self._mains_status_lbl.setText( f"Streak pair added at ±{q_new:.2f} nm⁻¹ — drag either line onto " @@ -440,6 +472,9 @@ def _mains_tab_active(self) -> bool: def _mains_handle_press(self, event) -> bool: """Begin dragging the custom streak pair nearest the click, if close.""" self._mains_drag_idx = getattr(self, "_mains_drag_idx", None) + streaks_cb = getattr(self, "_mains_streaks_cb", None) + if streaks_cb is None or not streaks_cb.isChecked(): + return False # invisible lines must not capture pan clicks if (not self._mains_tab_active() or event.inaxes is not self._ax_fft or event.button != 1 or not self._mains_custom_streaks()): return False diff --git a/probeflow/processing/mains_pickup.py b/probeflow/processing/mains_pickup.py index f96c161..26134c9 100644 --- a/probeflow/processing/mains_pickup.py +++ b/probeflow/processing/mains_pickup.py @@ -135,7 +135,11 @@ def predict_mains_fft_positions( ``dx``/``dy`` are integer pixel offsets from the fftshift-centred DC, ready for :func:`probeflow.processing.filters.periodic_notch_filter`. Returns an - empty list when the scan speed is unknown/non-positive. + empty list when the scan speed is unknown/non-positive, or when + ``harmonics`` is explicitly 0 — "no mains notches", so user-placed streak + pairs can be applied alone even though a scan speed is known (mains + pickup and e.g. tip-speed-related streaks are physically distinct + signals; one must not force the other). """ if not scan_speed_m_per_s or scan_speed_m_per_s <= 0: return [] @@ -157,7 +161,11 @@ def predict_mains_fft_positions( if harmonics is None: max_harmonic = int(math.ceil(max_fft_offset / harmonic_index)) + 1 else: - max_harmonic = int(max(1, harmonics)) + max_harmonic = int(harmonics) + if max_harmonic <= 0: + # Explicit 0 = mains notching disabled (previously clamped to 1, + # which silently forced the fundamental in). + return [] out: list[dict] = [] seen_indices: set[int] = set() diff --git a/tests/test_mains_pickup.py b/tests/test_mains_pickup.py index 517fcf6..edbca5b 100644 --- a/tests/test_mains_pickup.py +++ b/tests/test_mains_pickup.py @@ -397,3 +397,42 @@ def test_params_replay_through_processing_state(self): extra_streaks_px=[37], notch_fill="background", snap_window_px=0, ) np.testing.assert_allclose(replayed, direct, atol=1e-15) + + +class TestHarmonicsZeroDisablesMains: + """harmonics=0 must mean "no mains notches" — previously it was clamped + to 1, silently forcing the fundamental in. Mains pickup (electrical) and + custom streaks (scan-parameter noise) are physically distinct; applying + one must not require the other.""" + + def test_predict_returns_empty_for_zero_harmonics(self): + from probeflow.processing.mains_pickup import predict_mains_fft_positions + + assert predict_mains_fft_positions(160, 10e-9, 2e-8, harmonics=0) == [] + assert len(predict_mains_fft_positions(160, 10e-9, 2e-8, harmonics=1)) == 1 + + def test_custom_streak_applies_alone_despite_known_speed(self): + """With a scan speed present and harmonics=0, only the user's streak + is notched — the mains fundamental column is left untouched.""" + import numpy as np + from probeflow.processing.mains_pickup import mains_pickup_suppression + + N, W = 160, 10e-9 + yy, xx = np.mgrid[:N, :N] + # Mains-like stripe at bin 25 (50 Hz at v=2e-8) + custom stripe at 60. + img = (5e-11 * np.sin(2 * np.pi * 25 * xx / N) + + 5e-11 * np.sin(2 * np.pi * 60 * xx / N)) + + def col_mag(arr, k): + F = np.fft.fftshift(np.fft.fft2(arr - arr.mean())) + return float(np.abs(F[:, N // 2 + k]).mean()) + + out = mains_pickup_suppression( + img, scan_speed_m_per_s=2e-8, scan_range_m=(W, W), + harmonics=0, notch_shape="streak", + extra_streaks_px=[60], snap_window_px=0, + ) + assert col_mag(out, 60) < 0.05 * col_mag(img, 60), "custom streak kept" + assert col_mag(out, 25) > 0.9 * col_mag(img, 25), ( + "harmonics=0 still notched the mains fundamental" + ) diff --git a/tests/test_mains_pickup_gui.py b/tests/test_mains_pickup_gui.py index c88b226..a337965 100644 --- a/tests/test_mains_pickup_gui.py +++ b/tests/test_mains_pickup_gui.py @@ -246,3 +246,59 @@ def test_fft_auto_contrast_cycles_presets(self, qapp): assert len(set(seen[:3])) == 3, "Auto must change the range each click" assert seen[3] == seen[0], "cycle must wrap back to the full range" dlg.deleteLater() + + +class TestStreakPairIndependenceFromMains: + """2026-06-12 feedback: streak pairs were only drawn (and draggable) when + 'Show mains-pickup overlay' was on, and harmonics could not be 0 — so + using a custom pair forced at least one mains notch in. The two signals + are physically distinct and fully independent now.""" + + def test_pairs_visible_without_mains_overlay(self, qapp): + from matplotlib.lines import Line2D + + dlg = _dialog(qapp) + dlg._mains_overlay_cb.setChecked(False) + dlg._tab_widget.setCurrentIndex(dlg._mains_tab_index) + dlg._on_mains_add_streak() + + assert dlg._mains_streaks_cb.isChecked(), "add must enable visibility" + lines = [a for a in dlg._mains_artists if isinstance(a, Line2D)] + assert lines, "custom pair invisible without the mains overlay" + assert all(a.get_color() == "#89dceb" for a in lines), ( + "mains lines drawn despite the overlay being off" + ) + dlg.deleteLater() + + def test_hidden_pairs_do_not_capture_clicks(self, qapp): + from types import SimpleNamespace + + dlg = _dialog(qapp) + dlg._tab_widget.setCurrentIndex(dlg._mains_tab_index) + dlg._on_mains_add_streak() + q0 = dlg._mains_custom_streaks()[0] + dlg._mains_streaks_cb.setChecked(False) + + press = SimpleNamespace(inaxes=dlg._ax_fft, button=1, xdata=q0, ydata=0.0) + assert dlg._mains_handle_press(press) is False, ( + "invisible streak line stole a pan click" + ) + assert not [a for a in dlg._mains_artists], "hidden pairs still drawn" + dlg.deleteLater() + + def test_harmonics_zero_disables_mains_and_applies_pairs_alone(self, qapp): + captured: dict = {} + dlg = _dialog(qapp, captured=captured) # scan speed known + dlg._tab_widget.setCurrentIndex(dlg._mains_tab_index) + dlg._mains_auto_cb.setChecked(False) + dlg._mains_harm_spin.setValue(0) + + assert dlg._mains_predictions() == [] + assert "disabled" in dlg._mains_status_lbl.text().lower() + + dlg._on_mains_add_streak() + dlg._on_mains_apply() + assert captured["op"] == "mains_pickup_suppression" + assert captured["params"]["harmonics"] == 0 + assert captured["params"]["extra_streaks_px"] + dlg.deleteLater()