From c38630643dbeadb760d40c750a4ab7d0a9273767 Mon Sep 17 00:00:00 2001 From: Martino Ferrari Date: Tue, 13 Oct 2026 16:01:44 +0200 Subject: [PATCH] test: the noise probe reads the de-noise node, and the math probe builds one The probe gains a staged chain rather than one phase, and the schedule moves with it: `UDPSCOPE_PROBE_NOISE_MS` now times the *setup* stage, which installs the de-noise graph through `apply_math_channels()` -- a structural rebuild that replaces the buffer and drops its history -- five seconds before anything scans it. The legs and the five measurements then run at about 10.5 s, and the reading that says the median rejected every impulse at 15.5 s, so the documented run needs a 20 s mock and an 18 s exit. The de-noise channel is installed **disabled** and enabled only at 13 s, for its own reading. An acquired channel is a share of the ring, so a fifth one shortens every channel's retained span -- and the six `BufferSearch` leg counts and `pkpk bare` are all peaks and counts *of that window*. Installing it enabled (as the plan had it) moved `bare` from 49 to 21, `ratio` from 7.00 to 3.00 and `pkpk bare` from 2.988 to 2.488, i.e. it would have rewritten the older feature's documented proof to record a regression that is not one. With it enabled only at the end, every reading from Tasks 4 and 8 is bit-for-bit what the doc records, and the new line reads `denoise raw=2.987696/1 denoised=2.000000/1 ratio=0.669 channels=1` -- `denoised` exactly the clean sine's 2.0, which is the claim the node makes. Two other findings, both folded in: a derived channel whose source is switched off through the panel's checkbox follows it off and does *not* come back when the source returns (the enable-all probe's last act switches `sine` off at ~3 s), and the probes reload leftover math channels from `$XDG_CONFIG_HOME/udpscope`, which is why a `setup channels=` count above 1 means an earlier run's session, not a defect. `math_probe` gains a third channel with a `DeNoise` node so the editor's median row is built and synced on a baseline run: `3 math channels on 'sine' and 'noise'`, every other line as before, `math cost: 103.3 ns/slot` (one reading of the same path). verification.md: the recipe, and why the two stages may not be collapsed -- a probe that rebuilt the buffer under its own legs reads `bare=0` and looks like a broken comparator. Also that the noise and math probes may not share a run: both write `app.math`. --- docs/claude/invariants-math-channels.md | 10 ++- docs/claude/probe-readings.md | 51 ++++++++++-- docs/claude/verification.md | 29 +++++-- src/ui.cpp | 5 +- src/ui_app.hpp | 3 + src/ui_probe.cpp | 104 +++++++++++++++++++++++- 6 files changed, 181 insertions(+), 21 deletions(-) diff --git a/docs/claude/invariants-math-channels.md b/docs/claude/invariants-math-channels.md index f8f3faa..584769c 100644 --- a/docs/claude/invariants-math-channels.md +++ b/docs/claude/invariants-math-channels.md @@ -26,9 +26,10 @@ Part of [CLAUDE.md](../../CLAUDE.md). These are rules that cost a defect to lear drag pans, `f` fits, `Ctrl+0` resets**. Its widgets are created once and only shown, hidden and set by `math_dialog_sync()`, so no rebuild can invalidate a live callback handle -- the same shape the measurements dialog uses. - `UDPSCOPE_PROBE_MATH_MS` defines two graphs and opens the editor (a construction crash test - as much as a state check), and the dump prints each derived channel's extremes beside the - source's. Read it on the fast mock, where the two assertions that matter are **`gaps=0`** (the + `UDPSCOPE_PROBE_MATH_MS` defines three graphs -- the third a `Source` of the first channel into a + `DeNoise` node, so the editor's median row is built and synced on a baseline run -- and opens the + editor (a construction crash test as much as a state check), and the dump prints each derived + channel's extremes beside the source's. Read it on the fast mock, where the two assertions that matter are **`gaps=0`** (the math is on the producer thread, so what it can break is packet capture, never the frame rate) and each `math '': ... N samples vs M time slots` being **equal**, with `math x2` at `y=[-2, 2]`, `math lp` at `y=[-0.998, 0.998]` and `math3` at `y=[-2, 2]`. The `0 visible @@ -61,7 +62,8 @@ Part of [CLAUDE.md](../../CLAUDE.md). These are rules that cost a defect to lear which is why the fix is in the push and not in the loader. `math cost: S slots, N ns/slot (T timed), X ms total est` is the per-slot cost; see the math-cost invariant below for why it is sampled, why it is an upper bound, and why a figure - anywhere in 78--101 ns/slot is one reading of the same thing. + anywhere in 78--101 ns/slot is one reading of the same thing; the three-graph run reads **103**, the + de-noise channel's per-sample median being the difference. The probe also asserts the new editor mechanics. `dialog build:` reads the identity row back as well -- `name='math3' unit=''` -- which is the pair the `.ui` holds the widgets for and the code fills from the channel being edited, so a row that stopped being the one the code writes shows there. diff --git a/docs/claude/probe-readings.md b/docs/claude/probe-readings.md index 1c5e9f4..b1c48b9 100644 --- a/docs/claude/probe-readings.md +++ b/docs/claude/probe-readings.md @@ -1284,19 +1284,41 @@ while the edit runs) and never falls to 0, which is what a rebuild would do. Eve ## `[noise-probe]` ``` +[noise-probe] setup channels=1 [noise-probe] trigger band: bare=49/2 band=7/2 ratio=7.00 -[noise-probe] trigger hf: hf=7/2 band+hf=7/2 band=7 -[noise-probe] trigger delay: clean=4 filtered=4 delay=0.002206 want=0.002206 ratio=1.000 +[noise-probe] trigger hf: hf=7/2 band+hf=6/2 band=7 +[noise-probe] trigger delay: clean=3 filtered=4 delay=0.002206 want=0.002206 ratio=1.000 ``` `UDPSCOPE_PROBE_NOISE_MS`, and it needs the mock's **spiked** sine -- `--spike-every 0.005 --spike-amp 0.5 ---spike-width 2` -- where every other probe uses the plain mock. It arms in two phases: the first enables the -signals it measures *itself*, because a probe must not depend on another probe's side effects -(`enable_all_probe` ends by switching one off again, which is how `sine` went missing from the first run of -this one), and waits 2.5 s for the ring to refill after that structural change. The legs then each search the -whole retained buffer through `BufferSearch`, one after another **without yielding**, so all six see the same -buffer. +--spike-width 2` -- where every other probe uses the plain mock. It runs as **four stages**, and the chain is the point: +1. At `UDPSCOPE_PROBE_NOISE_MS` the setup installs the de-noise graph through `apply_math_channels()`. + That is a structural rebuild that *drops the buffer's history*, so the five seconds to the next stage are + what let the ring refill before anything scans it. +2. Five seconds later the legs run. They enable the signals they measure *themselves*, because a probe must + not depend on another probe's side effects (`enable_all_probe` ends by switching one off again, which is + how `sine` went missing from the first run of this one), and wait 2.5 s for the ring to refill after that + structural change. The legs then each search the whole retained buffer through `BufferSearch`, one after + another **without yielding**, so all six see the same buffer -- and the five measurements come next, over + the same retained history. +3. Two and a half seconds later the de-noise channel is **enabled**, by name, through the same switch the + panel drives. The setup installs it *disabled* and this stage turns it on, because an acquired channel is + a share of the ring: a fifth one shortens *every* channel's retained span, and every figure above scales + with that span -- the six leg counts, and `pkpk bare`, which is a peak-to-peak *of the retained window*. + The readings above are the older feature's proof, so the new channel stays out of the acquisition plan + until they have all run. +4. Two and a half seconds after that, the de-noise reading. A math channel starts producing when it is + applied *and* acquired, so the channel's first sample is the first one after stage 3, and a peak-to-peak + needs a window holding a trough *and* a crest: twelve cycles of the 5 Hz source. + +With `UDPSCOPE_PROBE_NOISE_MS=3000` the legs therefore land at about 10.5 s and the last line at 15.5 s, +which is why the documented run needs `UDPSCOPE_EXIT_AFTER_MS=18000`. + +- **`setup channels=1`** -- one math channel was installed, and it is printed by the *first* stage, before + any leg: it is the line that says the graph exists at all. A count above 1 means a session file from an + earlier run was loaded on top of it (`$XDG_CONFIG_HOME/udpscope`); those channels load disabled, so they + do not change the partition, and a clean run of the documented command prints `1`. - **`trigger band:`** -- `bare` is the bare comparator on the spiked sine (every impulse that lands near zero carries it across, so the count is dominated by spikes), `band` the same with a 0.8 band. **`ratio` must be at or above 5**, and `band` about the number of sine cycles retained. Neither count is a *fixed* number: @@ -1324,6 +1346,7 @@ The same probe then puts five measurements on the same spiked `sine` as **two wi [noise-probe] measure period band noise=25.0 hf=0.0 valid=1 value=0.200000 [noise-probe] measure period hf noise=0.0 hf=50.0 valid=1 value=0.200000 [noise-probe] measure windows=2 +[noise-probe] denoise raw=2.987696/1 denoised=2.000000/1 ratio=0.669 channels=1 ``` - **`pkpk bare`** -- about **3.0**: the sine's 2 plus an impulse either way. The impulses dominate the @@ -1341,6 +1364,18 @@ The same probe then puts five measurements on the same spiked `sine` as **two wi `MeasureOptions::noise_pct`'s own rationale says (10 % holds to a 0.3 interferer). - **`period hf`** -- exactly **0.200000** with the band off: the reject alone. Near `period noband` means the pole coefficient stayed at 1 -- check `measure_filter_set` was given a non-zero `dt`. +- **`denoise`** -- the feature under test. `raw` is the spiked sine's peak-to-peak over the visible window and + `denoised` the same reading from the **`math denoise`** channel -- a `Source` of `sine` into `DeNoise` with + `med_n=5` and `fc=0`, i.e. the median alone. **`denoised` must read exactly `2.000000`**: that is the clean + sine's own peak-to-peak, so *not one impulse survived*, which is the whole claim the node makes (the + impulses are 2 samples wide and the median is 5, shorter than half a window). `denoised` near `raw` means the + median is passing them: check `med_n` arrived as 5 and not 0, since a `med_n <= 0` node returns NaN and a + peak-to-peak over holes would read `--` instead. **`denoised=0.000000` with `valid=0` means the channel never + produced** -- it was never enabled, its source `sine` was never re-enabled, or `apply_math_channels()` was + never called. `ratio` is `denoised / raw` and follows `raw`, which is whatever the visible window's extremes + happen to hold: 0.669 here, and up to about 0.8 in a run whose window misses the spike at one extreme. The + assertion is on `denoised` and on `raw` being above it; the ratio is the reading, not the test. + `channels=` is the math channel *count* (`a->math.size()`), not whether the channel is acquired. - **`measure windows=2`** -- the five legs ask for corners `0, 50, 0, 0, 50` on one channel, which is two distinct windows. **`windows=1` means the corner is not part of the key**: the five collapsed onto one walk and two of them read a signal they did not ask for. diff --git a/docs/claude/verification.md b/docs/claude/verification.md index 0c446d0..1e99578 100644 --- a/docs/claude/verification.md +++ b/docs/claude/verification.md @@ -532,21 +532,36 @@ same run). ```bash cd /home/martino/Projects/tools/udpscope -python3 tools/mock_udpstreamer.py --port 8108 --rate 1300000 --batch 1000 --duration 16 \ +python3 tools/mock_udpstreamer.py --port 8108 --rate 1300000 --batch 1000 --duration 20 \ --spike-every 0.005 --spike-amp 0.5 --spike-width 2 > /tmp/noise-mock.log 2>&1 & sleep 1.5 -xvfb-run -a env UDPSCOPE_EXIT_AFTER_MS=14000 UDPSCOPE_ENABLE_ALL_MS=1000 \ - UDPSCOPE_PROBE_NOISE_MS=6000 \ +xvfb-run -a env UDPSCOPE_EXIT_AFTER_MS=18000 UDPSCOPE_ENABLE_ALL_MS=1000 \ + UDPSCOPE_PROBE_NOISE_MS=3000 \ ./build/udpscope --server 127.0.0.1 --port 8108 2>&1 | grep -E 'noise-probe|CRITICAL' ``` -The probe arms at 6 s rather than the 8 s a single-phase probe would use: its first phase enables the signals -it measures and waits 2.5 s for the ring to refill after that structural change. `--spike-every`/`--spike-amp`/ +`UDPSCOPE_PROBE_NOISE_MS` is **not the time its readings are taken**. It times the probe's first stage, which +installs a math graph through `apply_math_channels()` -- and that is a *structural rebuild* that replaces the +buffer and drops its history. Every reading the probe takes is over retained history, so the rebuild has to +happen seconds *before* them, and the chain is what the number is really choosing: `3000` puts the graph in at +3 s, `noise_probe` at 8 s (it enables the signals it measures itself and waits 2.5 s for the ring to refill +after *that* change), the legs and the five measurements at about 10.5 s, the de-noise channel enabled at +13 s, and the de-noise reading at 15.5 s -- hence the 18 s exit and the 20 s mock. **Collapsing the two +stages into one is the failure mode**: a probe that rebuilt the buffer at 10.5 s and then scanned it +immediately would read `bare=0` on every search leg, which looks exactly like a broken comparator. + +The de-noise channel is installed *disabled* and enabled only for its own reading (stage 3). An acquired +channel is a share of the ring, so a fifth one shortens every channel's retained span -- and the six leg +counts and `pkpk bare` all scale with that span. The readings above the de-noise line belong to the older +feature and are the proof it still works, so the new channel stays out of the acquisition plan until they +have run. `--spike-every`/`--spike-amp`/ `--spike-width` are the mock's own; `--batch` must stay at or below 1000 (2000 samples x 5 signals x f64 is 80 kB, past UDP's ~65507-byte payload, and the mock dies on its first `sendto` while the run still exits 0). The three lines and what each must read are in `probe-readings.md`. -`UDPSCOPE_PROBE_NOISE_MS` and `UDPSCOPE_PROBE_MEASURE_MS` must **not** be scheduled in the same run: both push -onto `app.measures`, and the later one would read the earlier one's rows as its own. The noise probe carries +`UDPSCOPE_PROBE_NOISE_MS`, `UDPSCOPE_PROBE_MEASURE_MS` and `UDPSCOPE_PROBE_MATH_MS` must **not** be scheduled +in the same run. The noise and measurement probes both push onto `app.measures`, and the later one would read +the earlier one's rows as its own; the noise and math probes both write `app.math`, so the later one's graph +lands on top of the earlier one's and both sets of readings describe a state neither probe set up. The noise probe carries its own measurement legs (see `probe-readings.md`), so nothing is lost by keeping them apart. diff --git a/src/ui.cpp b/src/ui.cpp index 8b66d41..330e0ee 100644 --- a/src/ui.cpp +++ b/src/ui.cpp @@ -5423,7 +5423,10 @@ int run_ui(int argc, char** argv, const Config& cfg, ScopeModel& model, Acquisit g_timeout_add(static_cast(std::atoi(env)), drop_probe, &app); } if (const char* env = g_getenv("UDPSCOPE_PROBE_NOISE_MS")) { - g_timeout_add(static_cast(std::atoi(env)), noise_probe, &app); + const int ms = std::atoi(env); + // Times the *setup* stage; `noise_probe` runs 5000 ms after it, so 3000 puts the legs at the same + // 8000 ms they were measured at. + if (ms > 0) g_timeout_add(static_cast(ms), noise_probe_setup, &app); } if (const char* env = g_getenv("UDPSCOPE_PROBE_ROWMENU_MS")) { rowmenu_probe_next(app, 0, static_cast(std::atoi(env))); diff --git a/src/ui_app.hpp b/src/ui_app.hpp index fd742a5..7159f6f 100644 --- a/src/ui_app.hpp +++ b/src/ui_app.hpp @@ -1422,6 +1422,9 @@ gboolean events_group_probe_step(gpointer user); void events_group_probe_next(App& a, int step, guint delay_ms); gboolean drop_probe(gpointer user); gboolean noise_probe(gpointer user); +gboolean noise_probe_setup(gpointer user); +gboolean noise_probe_math(gpointer user); +gboolean noise_probe_denoise(gpointer user); void collect_spins(GtkWidget* w, std::vector* out); gboolean defaults_probe(gpointer user); gboolean keys_probe(gpointer user); diff --git a/src/ui_probe.cpp b/src/ui_probe.cpp index 93a4e05..8602d46 100644 --- a/src/ui_probe.cpp +++ b/src/ui_probe.cpp @@ -326,11 +326,28 @@ gboolean math_probe(gpointer user) { lp.output = 1; channels.push_back(lp); + // A third channel so the editor's new form row is built and synced on a baseline run. Assigned field + // by field: a partial brace-initialiser would add a fifth build warning. + MathChannel dn; + dn.name = "math denoise"; + dn.enabled = true; + MathNode dn_src; + dn_src.op = MathOp::Source; + dn_src.source = name; + MathNode dn_op; + dn_op.op = MathOp::DeNoise; + dn_op.in_a = 0; + dn_op.med_n = 5; + dn_op.fc = 500.0; + dn.nodes = {dn_src, dn_op}; + dn.output = 1; + channels.push_back(dn); + // Through the app's list, which is the pane's source of truth: a probe that wrote the model // directly left the two disagreeing, and the next pane edit then silently dropped its channels. a->math = std::move(channels); apply_math_channels(*a); - std::fprintf(stderr, "[math-probe] 2 math channels on '%s' and '%s'\n", name.c_str(), + std::fprintf(stderr, "[math-probe] 3 math channels on '%s' and '%s'\n", name.c_str(), late_name.c_str()); // And open the editor on the first one: as much a construction crash test as a state check, // since a modal window is presented and its canvas, form and gestures are all built here. @@ -3503,6 +3520,49 @@ int noise_channel(App* a, const char* name) { } // namespace +// Stage one of three. `apply_math_channels()` is a structural rebuild that drops the buffer's +// history, and every reading in `noise_probe` is taken over retained history -- so the rebuild +// happens here, five seconds before the first leg runs, and the buffer refills in between. The +// de-noise channel also needs that time: a math channel starts producing when it is applied. +// +// The channel is installed **disabled**, and `noise_probe_math` enables it two and a half seconds +// before its own reading. An acquired channel is a share of the ring, so a fifth one shortens every +// channel's retained span -- and that moves every figure above it: the six search legs' counts scale +// with the retained buffer, and `pkpk bare` is a peak-to-peak *of that window*. Those readings are +// the older feature's proof, so the new channel stays out of the acquisition plan until they have +// all run, and only the de-noise comparison is taken with it in. +// +// Fields assigned rather than brace-initialised: a partial brace-initialiser on `MathNode` is +// what the two `-Wmissing-field-initializers` in this file already are, and the build baseline +// is exactly four warnings. +gboolean noise_probe_setup(gpointer user) { + App* a = static_cast(user); + if (a->window == nullptr) return G_SOURCE_REMOVE; + MathChannel dn; + dn.name = "math denoise"; + dn.enabled = false; // enabled by `noise_probe_math`, so it costs no ring while the legs run + MathNode src_node; + src_node.op = MathOp::Source; + src_node.source = "sine"; + MathNode dn_node; + dn_node.op = MathOp::DeNoise; + dn_node.in_a = 0; + dn_node.med_n = 5; + // The median alone: a 2-sample impulse is shorter than half a 5-sample window, so it is rejected + // outright and the pole has nothing left to do. A corner here would attenuate the 5 Hz sine as well + // and make the peak-to-peak reading a function of two things. + dn_node.fc = 0.0; + dn.nodes = {src_node, dn_node}; + dn.output = 1; + a->math.push_back(dn); + apply_math_channels(*a); + std::fprintf(stderr, "[noise-probe] setup channels=%zu\n", a->math.size()); + std::fflush(stderr); + // 3300 + 4700 = 8000 ms, where Tasks 4 and 8 measured the legs. + g_timeout_add(5000, noise_probe, a); + return G_SOURCE_REMOVE; +} + gboolean noise_probe(gpointer user) { static int phase = 0; App* a = static_cast(user); @@ -3621,6 +3681,48 @@ gboolean noise_probe(gpointer user) { std::fflush(stderr); a->search.clear(); update_search_panel(*a); + g_timeout_add(2500, noise_probe_math, a); + return G_SOURCE_REMOVE; +} + +gboolean noise_probe_math(gpointer user) { + App* a = static_cast(user); + if (a->window == nullptr) return G_SOURCE_REMOVE; + { + // Enabled here for the reason `noise_probe_setup` gives: until this moment the channel must not + // be a share of the ring. Enabled *by name*, through the same switch the panel drives, so the + // model and the panel agree. + const std::vector& signals = a->model->signals(); + for (std::size_t i = 0; i < signals.size(); ++i) { + if (signals[i].name == "math denoise") a->model->set_signal_enabled(i, true); + } + } + // A fill before the reading: the channel's first sample is the first one after this enable, and a + // peak-to-peak needs a window with a sine trough *and* a crest in it. Two and a half seconds is + // twelve cycles of the 5 Hz source. + g_timeout_add(2500, noise_probe_denoise, a); + return G_SOURCE_REMOVE; +} + +gboolean noise_probe_denoise(gpointer user) { + App* a = static_cast(user); + if (a->window == nullptr) return G_SOURCE_REMOVE; + const std::size_t base = a->measures.size(); + // Both unfiltered and at no band: the only difference between the two readings is the node. + MeasureOptions options; + a->measures.push_back({std::string("sine"), MeasureType::PeakToPeak, options}); + a->measures.push_back({std::string("math denoise"), MeasureType::PeakToPeak, options}); + rebuild_measures_pane(*a); + update_measures(*a, /*force=*/true, /*accumulate=*/true); + const MeasureResult raw = + base < a->measure_results.size() ? a->measure_results[base] : MeasureResult{}; + const MeasureResult den = + base + 1 < a->measure_results.size() ? a->measure_results[base + 1] : MeasureResult{}; + std::fprintf(stderr, + "[noise-probe] denoise raw=%.6f/%d denoised=%.6f/%d ratio=%.3f channels=%zu\n", + raw.primary, int(raw.valid), den.primary, int(den.valid), + raw.primary > 0.0 ? den.primary / raw.primary : 0.0, a->math.size()); + std::fflush(stderr); return G_SOURCE_REMOVE; }