From 0106aca69b4492bee9f4149bfe598fdd77a13722 Mon Sep 17 00:00:00 2001 From: Martino Ferrari Date: Tue, 13 Oct 2026 13:31:55 +0200 Subject: [PATCH] docs: design for noise rejection in the trigger, the measurements and a math node EM interference is impulsive, so each place gets an amplitude qualifier and a frequency one, both optional and both defaulting to today's behaviour: the documented reading set has to come out unchanged. Records why the trigger's zero-band path stays bit-for-bit, why a measurement filter gives up the bucket fast paths, and why the new op is appended rather than placed beside the filters. Co-Authored-By: Claude Opus 4.6 --- .../2026-10-13-noise-rejection-design.md | 342 ++++++++++++++++++ 1 file changed, 342 insertions(+) create mode 100644 docs/superpowers/specs/2026-10-13-noise-rejection-design.md diff --git a/docs/superpowers/specs/2026-10-13-noise-rejection-design.md b/docs/superpowers/specs/2026-10-13-noise-rejection-design.md new file mode 100644 index 0000000..9529154 --- /dev/null +++ b/docs/superpowers/specs/2026-10-13-noise-rejection-design.md @@ -0,0 +1,342 @@ + + + +# Noise rejection: design + +Noise rejection in three places — the measurements pane, the trigger, and a new math node — aimed +at one problem: **electromagnetic peaks**. EM interference is *impulsive*. It arrives as a few +samples of large excursion, not as a raised noise floor, and that distinction decides every choice +below. An impulse recrosses a threshold several times per true crossing, so it inflates an edge +count; it moves a `min`, a `max` and a peak-to-peak outright; and it is the one kind of noise a +low-pass filter handles *badly*, smearing the spike across its settling time instead of removing +it while rounding the real edge beside it. + +Each of the three places therefore gets two independent, optional controls: + +| | amplitude qualifier | frequency qualifier | +| --- | --- | --- | +| **Trigger** | hysteresis band about the level, in signal units | one-pole HF reject on the comparator's input, `fc` in Hz | +| **Measurements** | hysteresis band, % of the window's peak-to-peak | one-pole HF reject over the measured window, `fc` in Hz | +| **Math** | — | the `de-noise` node: median over N samples, then a one-pole at `fc` | + +Nothing here changes a default. Every control is off, or set to the value the code already uses, +so the whole documented reading set must come out of this work unchanged. That is the first +verification assertion, not an afterthought. + +## The decisions taken, and what each one rules out + +| Decision | Alternative rejected | +| --- | --- | +| The trigger band is **absolute, in the channel's own units** | A percentage of a running peak-to-peak: the producer has no measured span, so it would need per-channel sliding-window state in the hot loop *and* a warm-up during which the band is wrong — which shows up as a missed or early latch, the one failure a trigger must not have | +| `band == 0` takes today's crossing test **verbatim**, with the latched-side comparator used only when the band is non-zero | One unified latched-side comparator: at `v == level` exactly the two are *not* equivalent (today it fires, a zero-width band decides nothing), and `feed_window`'s own comment records a ±1 square against a `[1, 1]` band firing 19 times on exact float equality. `probe-readings.md`'s `[condition-probe]` table of four exact latch times is a regression suite in disguise; unifying would require re-measuring all of it to prove a change nobody asked for | +| The band and the filter live **inside `EventDetector`** | Filtering in `Acquisition` before `engine.process()`: the buffer search deliberately runs the same evaluator (`condition.hpp:46-49`), so anything upstream of the detector has to be replicated there, and the live trigger and the search would drift apart | +| No shared hysteresis primitive extracted | A common `side_of()` header used by `measure.cpp` and `condition.hpp`: the genuinely shared part is about five lines, and the two call sites want different things from it — `measure.cpp` `continue`s on an in-band sample while counting enters, widths and folded buckets, the detector needs a side to feed four state machines. Three similar lines beat a premature abstraction; the *invariant* is stated once and pointed at | +| The measurement filter is a **real one-pole at a user `fc`**, and it disables the pyramid fast path | Measuring the pyramid's per-bucket means, which would be free and keep the fast path — but its corner is wherever the bucket size puts it (`fs/64`), so it is HF reject at one fixed, rate-dependent frequency rather than a cutoff the user can aim at the interference they actually have | +| The filter runs **inside each of the two raw walks** | Materializing a filtered copy of the window: `MeasureChunk`'s comment (`measure.hpp:108-115`) records that widening the window cost 60 % of the pane's time before a single statistic had been computed, and 17 MB at a 2.2 M-sample window | +| One combined `de-noise` node (median **then** pole) | A plain median node chained to the existing `LowPass`: two boxes for one idea, and it lets the user wire them in the wrong order — a pole *before* a median lets one spike contaminate the filter state for its whole settling time, which defeats the purpose | +| The median window is **N samples, fixed** | A duration converted to N from `dt`: N would then grow with decimation, so a modest duration at a long window becomes a large sort in the producer's hot loop and needs a hidden cap the user did not set | +| `MathOp::DeNoise` is **appended**, at index 12 | Inserting it beside the filters in `math_op_at()`, where it belongs in the dropdown: a node persists as its *index through that accessor* (`layout_store.cpp:358-365`), so inserting would silently reinterpret every node in every existing session file | +| The median's ring is **pooled in `MathRuntime`** | A `double[9]` member of `MathNodeRt`: 72 bytes added to *every* node, doubling a struct whose shape README's math-cost section records as a deliberate choice, to serve the one op whose state size is a parameter | +| A new **impulsive** mock source | Verifying against `--noise` (Gaussian): a median is specifically the wrong tool for a raised noise floor and the right one for impulses, so a Gaussian test would prove nothing about the feature | + +## 1. The trigger + +### Config + +`EventCondition` (`condition.hpp:27`) gains two fields: + +```cpp +double noise_band = 0.0; ///< hysteresis half-width about `level`, signal units. 0 = off. +double hf_fc = 0.0; ///< HF reject corner on the comparator's input, Hz. 0 = off. +``` + +Both **must** join `operator==` (`condition.hpp:37-41`). This is not bookkeeping: +`EventDetector::configure()` returns early when the condition compares equal +(`condition.cpp:9`) and the producer calls it on every packet, so a field omitted from the +comparison is a control that silently never takes effect. + +The filter needs the sample interval to turn `fc` into a coefficient, and the detector is fed +`(value, t)` pairs rather than a rate. `configure()` therefore takes `dt` as a second argument and +keeps it as a `dt_` member, and the early return compares the new `dt` against `dt_` as well as the +condition against the stored one — a re-plan changes `dt`, and a coefficient left over from the +previous partition is a filter at the wrong corner. + +### Comparator + +`EventDetector` gains `int side_ = 0` (0 undecided, ±1 above/below) and one pole state word. +`crossed_up`/`crossed_down` (`condition.hpp:77-82`) stop being pure functions of `prev_` and +become flags computed **once per sample** in `feed()`: + +- `noise_band == 0` → `prev_ < level && v >= level` and its mirror, bit for bit as today. +- `noise_band > 0` → `v >= level + band` latches `+1`, `v <= level - band` latches `-1`, anything + between decides nothing; a crossing *is* a change of latch. + +Computing the pair once also removes a duplicated computation: `feed_edge`, `feed_pulse` and +`feed_dropout` each call both predicates today. + +`feed_window` (`condition.cpp:115`) does not use those predicates, so it takes the band +explicitly on each edge of its own band: becoming *inside* requires `[lo+band, hi-band]`, +becoming *outside* requires leaving `[lo-band, hi+band]`. A band wide enough to close that inner +region is clamped to just under half the window's width — the same shape of clamp +`measure_set_level()` already applies for the same reason, and it keeps the panel's existing +refusal of a zero-width window meaningful. + +`reset()` (`condition.cpp:14`) clears `side_` and the pole state. + +### The filter + +One pole on the comparator's input, `y += a·(x − y)`, with `a = 1 − exp(−2π·fc·dt)` built in +`configure()` and clamped below Nyquist as `biquad()` does (`math_channel.cpp:28`). `fc = 0` +gives `a = 1`, i.e. `y = x`, so "off" needs no branch in the inner loop. + +Two consequences, both stated rather than discovered: + +- **The latch moves later** by roughly `1/(2π·fc)` — the pole's group delay. This is inherent, + it is what a bench scope's HF reject also does, and it is why the control is off by default. +- **A NaN stays benign.** `feed()`'s comment (`condition.hpp:68-70`) records that a NaN sample + cannot fire because every comparison against it is false, and that *rejecting* one would leave + `prev_` stale and let the next sample fire a crossing that today cannot happen. The filter + preserves this: a non-finite input returns NaN (which flows into the existing comparisons, all + false) and leaves the pole state alone. + +`last_value()` (`condition.hpp:73`) comes to report the comparator's input rather than the raw +sample. Its only consumer is the `v=%.4g` field of the trig-engine diagnostic +(`acquisition.cpp:918`), whose purpose is to say what the comparator saw. + +### Panel + +A `trigger_band_spin` and a `trigger_hf_spin` in `build_trigger_panel()` (`ui_trigger.cpp:219`), +read in `publish_trigger_fields()` (`:56`), shown for all four condition types in +`sync_trigger_rows()` (`:94`). The band takes the level spin's unit, digits and range, floored at +0. + +### The search gets both for free + +`EventDetector` holds no arming state precisely so the buffer search can run the same evaluator +over retained samples (`condition.hpp:46-49`). A band or a filter set in the panel therefore +changes what a search finds, consistently, with no second code path and no second thing to keep +in step. + +### Not persisted + +The trigger has never been persisted, so neither control is either; a restart comes up with both +off. That is also what makes every current `[condition-probe]` latch time hold unchanged. + +## 2. The measurements + +### Config + +`MeasureOptions` (`measure.hpp:37`) gains two fields: + +```cpp +double noise_pct = 10.0; ///< hysteresis half-width, % of the window's peak-to-peak. 0 = off. +double hf_fc = 0.0; ///< HF reject corner, Hz. 0 = off. +``` + +`noise_pct = 10.0` reproduces today's `kHysteresisFrac = 0.10` (`measure.cpp:36`) exactly, so +every current reading is identical. `measure_set_level()` (`measure.hpp:195`, +`measure.cpp:40`) already takes a `pct` — the *threshold* percent — so it gains a second, +separately named `noise_frac` argument for the band, replacing its read of the file-scope +`kHysteresisFrac`; the constant stays as `MeasureOptions::noise_pct`'s default so its +measured-value rationale — 5 % reads a 0.1 interferer correctly and fails at +0.2, 10 % holds to 0.3 and to white noise at σ=0.05 — does not become an orphan comment. The +clamp to half the room between the level and the nearer extreme survives untouched: it is what +keeps a threshold at 90 % measurable instead of reading `enters=0`. + +### The band reaches the edge types only + +`MeasureLevel::hysteresis`'s comment (`measure.hpp:165-169`) is explicit that the band moves the +*edge decision* only, and that the `sum`/`count` split stays at the bare level — because that +split **is** `Top`/`Bottom`, the mean of the samples above or below the threshold, and banding it +would change the documented exact `top = 2/π` on a unit sine. + +So `noise_pct` is meaningful for `PulseDuration`, `Period` and `Frequency`, and does nothing for +`Top`/`Bottom`. The file's own rule (`measure.hpp:34-36`) is that an option which does nothing +for a type is left out of the dialog rather than stored and ignored, so this adds a +`measure_uses_noise()` beside `measure_uses_threshold()` and `measure_uses_side()` +(`measure.hpp:242-245`), and `measure_dialog_sync()` (`ui_measure.cpp:90`) shows the field for +those three types only. + +A *raised* band also costs reads: `measure_accumulate_bucket()` already refuses a bucket where +the signal passes `level ± hysteresis` (`measure.hpp:211-216`), so a wider band opens more +buckets. The existing `measure cost:` line is where that shows up. + +### HF reject reaches every type + +The filter changes the samples rather than a decision, so `min`, `max`, `average`, +`peak to peak`, `amplitude`, `top`, `bottom` and every crossing type all read the filtered +signal. The control is therefore shown for all types. + +It is seeded with the window's first sample, so the step transient at the left edge does not read +as a measurement. (Priming from samples *before* the window is possible — the ring holds the +history — and is deliberately not done: it would make a measurement depend on data outside the +window it reports on.) + +### What it costs + +`hf_fc > 0` disables **both** bucket fast paths, `measure_scan_buckets()` (`measure.hpp:147`) and +`measure_accumulate_bucket()` (`:217`): a bucket's `lo`/`hi`/`sum` describe raw samples and cannot +answer for a filtered signal. Both passes walk raw, which is the 1.2–1.6 ms → ~20 ms step at a +2.2 M-sample window that README's measurement-cost section was written about. Against the pane's +250 ms cadence that is a few percent of a core and a visible frame hitch. + +This is accepted deliberately: the control is per-measurement and off by default, so nothing gets +slower unasked, and `measure cost:`'s `covered`/`buckets`/`raw` makes the cost a *reading* rather +than a mystery. `raw ≈ covered` is the signature of a filtered measurement. + +The filter runs inside each of the two raw walks — three flops on a sample the walk is already +reading — rather than materializing a filtered window once. Filtering twice is deterministic and +free compared with the alternative measured in `MeasureChunk`'s comment. + +NaN keeps the existing rule: the walks already skip a non-finite sample, and the filter state is +left alone, so a hole stays a hole and the filter resumes on real samples. + +### Two documented deviations + +1. `docs/claude/invariants-readouts-and-cursors.md` holds that a readout quotes a stored sample + and never a value between two. A filtered `max` is a value that was never stored, and with + `show_on_plot` its ruler sits at a level the drawn — raw — trace never reaches. This is + inherent to filtering, so it becomes a stated exception in that invariant rather than a silent + violation. +2. The documented exact baselines (`peak to peak = 2`, `top = 0.636620`) hold with the filter + off. With it on they move, correctly; the probe reads both. + +### Persistence + +Free. A measurement is written as keyed TOML (`layout_store.cpp:659-672`) and read key by key +(`:544-556`), so two new keys load in an old session at their defaults. + +## 3. The `de-noise` math node + +### The op, appended + +`MathOp::DeNoise` goes last in the enum (`math_channel.hpp:15-28`) and at index 12 in +`math_op_at()` (`math_channel.cpp:99-114`), with `Div` given an explicit `case 11` so the current +`default` no longer swallows it; `math_op_count()` becomes 13 and `math_op_name()` gains +`"de-noise"`. + +Indices 0–11 keeping their meaning is exactly what leaves every existing session file reading +correctly, because a node persists as its index through that accessor. The price is that +de-noise sits last in the dialog's dropdown rather than beside the filters, which is the right +side of that trade. + +### Parameters + +`MathNode` (`math_channel.hpp:48-72`) gains `int med_n = 5;` — odd, 3 to 9, rounded to odd and +clamped. `fc` is reused for the HF reject; `q` is unused. `fc = 0` means median only, and it +expresses "off" without a branch: the pole's coefficient `a = 1 − exp(−2π·fc·dt)` is exactly 1 at +`fc = 0`. + +The coefficient is built in `math_prepare()` (`math_channel.cpp:140-165`) where `dt` is known and +rebuilt on a `dt` change, exactly as the biquads are, with the same sub-Nyquist clamp +(`:28`). + +### Predicates + +`math_op_uses_filter()` (`math_channel.cpp:135`) keeps meaning **biquad** — it gates coefficient +construction at `:162`, and a de-noise node must not be given biquad coefficients. The dialog +gets a new `math_op_uses_fc()` (filter ‖ de-noise) and a `math_op_uses_median()` for the new spin +row. `math_dialog_sync_form()` (`ui_math.cpp:889-981`) already shows and hides one widget set per +op, and `q`'s row being visible for `BandPass` alone is the precedent for a per-op subset of the +filter row. + +### Node text + +`math_node_text()` (`layout_store.cpp:358`) emits a 13th pipe field. `math_node_from_text()` +accepts **12 or 13** fields, defaulting `med_n` when it reads 12, so existing sessions load +unchanged. A file written by this build and read by an older one is rejected by that build's +`f.size() != 12u` check (`:385`); there is no forward-compatibility requirement here, but it is +recorded rather than left as a surprise. + +### Runtime state, pooled + +`MathRuntime` (`math_channel.hpp:115-120`) gains one `std::vector` holding the median +rings of whichever nodes need them; `MathNodeRt` gains an offset into it, a ring position and the +effective N. Not a fixed array member, for the reason in the decisions table. One indirection on +a path already doing N compares is free, and the choice is reversible if measurement disagrees. + +### Evaluation + +In `math_step()` (`math_channel.cpp:167-230`), the new case is: push the input into the ring, +take the median, then one pole step. **Median first** — de-spiking before smoothing is the whole +point, and the reverse lets one spike contaminate the filter state for its entire settling time. + +The median is an insertion sort of at most nine doubles in a local array: ~20 compares worst +case, ~4 at N=3, against a documented 80–100 ns per slot for a whole three-graph runtime. That +ratio is measured and reported through the existing math-cost line, not assumed. + +**Warm-up:** before the ring holds `med_n` samples the node outputs the median of what it holds, +which is a correct median of a shorter window. Deliberately not NaN — in this application a NaN +output *means a hole in the data* (`math_channel.cpp:64-70`), and spending that meaning on a +start-up transient would make a gap ambiguous. + +**NaN** matches `biquad_step` exactly: a non-finite input returns NaN and leaves both the median +ring and the pole state alone, so the hole stays visible and the node resumes on real samples. + +## 4. Verification + +There is no unit test suite; the check is the application's own headless self-check under +`xvfb-run` against `tools/mock_udpstreamer.py`, read against two baselines (an exact warning +count, and no `Gtk-CRITICAL`). + +### The regression assertion, first + +Every default is today's behaviour, so the documented reading set must be **unchanged**: + +- `[condition-probe]`'s four exact latch times (`t0=1.710000`, `2.160100`, `1.706700`, + `4.120200`); +- `peak to peak=2`, `amplitude=1`, `average=0`, `top=0.636620`; +- `measure cost:` with `raw ≪ covered`; +- the build's warning count — adding a field to `MathNode` leaves + `ui_probe.cpp:304` and `:313` at one `-Wmissing-field-initializers` each, so four on a clean + build and two on an incremental one still holds. + +### The mock needs an impulsive source + +`tools/mock_udpstreamer.py` offers `--noise` (Gaussian), `--glitch-at`/`--glitch-width` (a single +glitch at one time) and `--clock-jitter`. None of them is a *stream* of EM-like peaks, and a +median is the wrong tool for Gaussian noise, so none of the three pieces could be shown to work +against the mock as it stands. + +Add `--spike-every S --spike-amp A --spike-width N`: a recurring impulse of N samples at +amplitude A, every S seconds. One flag set, and it is what every reading below is measured on. + +### The four readings + +1. **Trigger band** — a new `[condition-probe]` leg on a slow sine plus spikes. With + `noise_band = 0` the latch lands on a spike; with the band above the spike amplitude it lands + on the sine's own crossing time. The latched `t0` is the assertion, and the four existing + shapes are re-read unchanged in the same run. +2. **Trigger HF reject** — the same leg with the band off and `hf_fc` set. The latch moves + *later* by roughly `1/(2π·fc)`, reported beside the prediction. A latch that does not move is + the filter not being in the comparator's path. +3. **Measurements** — `Frequency` on the spiky mock read three ways: `noise_pct = 0` (counts + spikes, frequency far too high), `= 10` (the true frequency), and `hf_fc` set (the true + frequency *and* `raw ≈ covered`, which is the fast-path loss as a reading). Plus `peak to + peak` with the filter off and on, since that is the number the filter is supposed to move. +4. **The de-noise node** — `math_probe` (`ui_probe.cpp:304`) gains a de-noise graph on the spiky + channel. Its output's peak-to-peak must fall to the clean waveform's while a `LowPass` at the + same `fc` does not: that contrast is the entire justification for the node existing. Plus the + per-slot cost through the existing math-cost line. + +## 5. Documentation + +| File | What it gains | +| --- | --- | +| `README.md` | A noise-rejection section covering the three places, and the filtered-readout deviation stated rather than quietly taken | +| `docs/claude/invariants-readouts-and-cursors.md` | The stated exception: a filtered readout is not a stored sample, and its on-plot ruler can sit where the raw trace never goes | +| `docs/claude/invariants-measurements.md` | A filter on a measurement disables both bucket fast paths, and `raw ≈ covered` is its signature | +| `docs/claude/invariants-trigger-and-pinning.md` | The band, the `band == 0` verbatim path and why, and the latch delay a filter adds | +| `docs/claude/invariants-math-channels.md` | Op indices are append-only because the session format stores them; the pooled median state | +| `docs/claude/verification.md`, `docs/claude/probe-readings.md` | The four readings above and what a wrong value names | +| `docs/claude/build-and-run.md` | The new mock flags | + +## 6. Out of scope + +- Persisting the trigger. The trigger has never been persisted; the band and the filter follow + that, rather than this change being the one that introduces trigger persistence. +- Any averaging acquisition mode. Noise rejection here is per-measurement, per-trigger and + per-node; nothing changes what the ingest stores, so a raw trace stays raw and every reading + remains traceable to stored samples. +- Noise rejection on the cursors or the search's own UI. The search inherits the trigger's + condition and so inherits both controls; nothing further is added.