From 29a5dff0c9ec5fb342e9862110ae66285a98b46e Mon Sep 17 00:00:00 2001 From: Simon Keimer Date: Tue, 22 Sep 2026 14:11:32 +0200 Subject: [PATCH] test(bpsk): pin that uncoded BPSK takes the UNCANCELLED arm, and fix a false comment (#1429) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit No behaviour change, deliberately — no arm was switched. `receive_from_samples` prefers `demodulate_soft` whenever the plugin advertises one, and `BpskPlugin::supports_soft_demod` returns `true` UNCONDITIONALLY. So every uncoded decode (`FecMode::None`, `receive()`, `decode_burst_phase1`) hard-decides the soft arm's LLRs, and BPSK's soft arm deliberately skips `cancel_crossfade_isi` (#832). Every coded decode takes the cancelled arm. MEASURED on #821's own fixture — same payload, same sigma 0.9, same LCG noise, 8 seeds: demodulate (cancelled) -> mean BER 0.0127 passes its < 0.02 bar soft hard-decided (SHIPPED) -> mean BER 0.0336 fails on every seed (0.0243-0.0444) So `crossfade_cancellation_lowers_awgn_ber`, an UNCODED BER test, guards a demodulator that no uncoded production decode runs — and the arm that ships would not pass it. WHICH ARM UNCODED TRAFFIC SHOULD USE IS NOT DECIDED HERE, and that is on purpose: #1363 measures the cancellation as a win on AWGN and pure Doppler and a loss of 8 frames in 96 on a delayed-dominant fade, while the uncoded traffic on this path — §97.119 station ID, handshake, QSY, relay (#1123) — lives on fading channels. The current split may be right for the traffic each arm carries. What was indefensible is that it came from a capability flag that exists for another purpose, was justified by a comment that is false for the one plugin whose arms differ, and that nothing tested the shipping path. The new test is a CHARACTERISATION test on #821's fixture by construction. Each of its three assertions names what its own failure would mean — including that if the production arm ever MEETS the bar, #1429 is resolved and the test should be deleted rather than loosened. engine.rs's comment claiming a hard retry "can't succeed where the soft pass failed — both share the same acquisition front end" is corrected: false for BPSK, whose arms differ by exactly the transform under discussion. Refactors: CAP-12 Verification-objective: the test guarding a transform must exercise the path that ships, and a justification comment must not assert a property the code does not have Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0188ATCj6DZ9aRVQ2vSirua6 --- crates/openpulse-modem/src/engine.rs | 18 ++++- docs/dev/project/traceability.md | 39 ++++++++++ plugins/bpsk/src/lib.rs | 103 +++++++++++++++++++++++++++ 3 files changed, 158 insertions(+), 2 deletions(-) diff --git a/crates/openpulse-modem/src/engine.rs b/crates/openpulse-modem/src/engine.rs index e40d034b..1a26134f 100644 --- a/crates/openpulse-modem/src/engine.rs +++ b/crates/openpulse-modem/src/engine.rs @@ -4478,8 +4478,22 @@ impl ModemEngine { // and hard bits (via sign decision), avoiding a redundant demodulate() call. // Only plugins that declare soft support take this path; for them a soft // error is a genuine demodulation failure, not a cue to re-demodulate hard - // (which would double the per-attempt cost and can't succeed where the - // soft pass failed — both share the same acquisition front end). + // (which would double the per-attempt cost). + // + // **"can't succeed where the soft pass failed — both share the same acquisition front + // end" WAS ALSO CLAIMED HERE, AND IS FALSE FOR BPSK (#1429).** Its two arms differ by + // exactly `cancel_crossfade_isi`: `demodulate` applies it, `demodulate_soft` + // deliberately does not (#832 — the recursion breaks the LLR calibration HARQ combining + // relies on). `BpskPlugin::supports_soft_demod` returns `true` unconditionally, so every + // UNCODED decode that reaches here takes the uncancelled arm while every coded decode + // takes the cancelled one. Measured on #821's own fixture, 8 seeds: cancelled mean BER + // 0.0127 against its `< 0.02` bar, uncancelled 0.0336 — above the bar on every seed. + // Pinned by `the_uncoded_production_path_takes_the_uncancelled_arm` in `bpsk-plugin`. + // + // Which arm uncoded traffic SHOULD take is open (#1429) and is a real trade, not an + // oversight to reverse on sight: #1363 measures the cancellation as a win on AWGN and + // pure Doppler and a loss of 8 frames in 96 on a delayed-dominant fade, and the uncoded + // traffic here — §97.119 station ID, handshake, QSY, relay — lives on fading channels. // Absolute RX SNR for rate adaptation: the mode's calibrated symbol-domain estimate // (M2M4 fallback inside `rx_snr_db`). The old mean-|LLR| proxy reads ≈ −2 dB on a // clean path (only a relative confidence indicator) and can't drive the SNR-hint diff --git a/docs/dev/project/traceability.md b/docs/dev/project/traceability.md index d724aa99..04259fb9 100644 --- a/docs/dev/project/traceability.md +++ b/docs/dev/project/traceability.md @@ -15,6 +15,45 @@ and the actually-observed results per change. --- +## 2026-09-22 — the uncoded BPSK path misses #821's own bar by 1.7×; #1429 + +**Change.** A characterisation test and a corrected comment; **no behaviour change**, deliberately. + +**The finding.** `receive_from_samples` prefers `demodulate_soft` whenever the plugin advertises one, +and `BpskPlugin::supports_soft_demod` returns `true` unconditionally — so every **uncoded** decode +(`FecMode::None`, `receive()`, `decode_burst_phase1`) hard-decides the SOFT arm's LLRs, and BPSK's +soft arm deliberately skips `cancel_crossfade_isi` (#832). Every **coded** decode takes the cancelled +arm. Measured on #821's own fixture — same payload, same σ = 0.9, same LCG noise, 8 seeds: the +cancelled arm means **0.0127** against its `< 0.02` bar; the uncancelled arm means **0.0336** and +exceeds the bar on **every seed**. So `crossfade_cancellation_lowers_awgn_ber`, an *uncoded* BER +test, guards an arm no uncoded production decode runs — and the arm that ships would fail it. + +**What was NOT done, on purpose: no arm was switched.** #1363 measures the cancellation as a win on +AWGN and pure Doppler and a loss of 8 frames in 96 on a delayed-dominant fade, and the uncoded +traffic here — §97.119 station ID, handshake, QSY, relay (#1123) — lives on fading channels. So the +current split gives uncoded traffic the fade-favourable arm and coded traffic the AWGN-favourable +one, which may be right for the traffic each carries. What was indefensible is that it arose from an +unconditional capability flag, was justified by a comment that is false for the one plugin whose arms +differ, and that nothing tested the shipping path. + +**Implementation.** `plugins/bpsk/src/lib.rs` gains +`the_uncoded_production_path_takes_the_uncancelled_arm`, on #821's fixture by construction (a +different fixture would not be comparable to the bar being cited). Its three assertions each name +what their own failure would mean — including that if the production arm ever *meets* the bar, the +reader should delete the test because #1429 is resolved, rather than loosen it. +`crates/openpulse-modem/src/engine.rs`'s justification comment is corrected: the claim that a hard +retry "can't succeed where the soft pass failed — both share the same acquisition front end" is false +for BPSK, whose arms differ by exactly the transform in question. + +**Tests → results.** `the_uncoded_production_path_takes_the_uncancelled_arm` passes; full gate on +PR #1431. + +**Open, and the maintainer's.** Which arm uncoded traffic should use. Three candidate closures are on +the issue; the cheapest is to keep the split and record the reason, and the question may dissolve +entirely if #1428's gated canceller lands and both arms can take it (#1361's scope). + +--- + ## 2026-09-22 — the #1363 gate arms, and an estimator that was slandering its own candidates **Change.** Test-only; no production code. `mod carrier_dip_tiebreak` gains the (lock × dominance) diff --git a/plugins/bpsk/src/lib.rs b/plugins/bpsk/src/lib.rs index 9f501088..551cd52c 100644 --- a/plugins/bpsk/src/lib.rs +++ b/plugins/bpsk/src/lib.rs @@ -280,6 +280,109 @@ mod tests { assert_eq!(&recovered[..payload.len()], payload); } + /// The UNCODED production path takes the uncancelled arm, and does NOT meet the bar the test + /// below holds the cancelled arm to (#1429). + /// + /// `ModemEngine::receive_from_samples` prefers `demodulate_soft` whenever the plugin advertises + /// one, and `BpskPlugin::supports_soft_demod` returns `true` unconditionally — so every uncoded + /// decode (`FecMode::None`, `receive()`, `decode_burst_phase1`) hard-decides the SOFT arm's + /// LLRs, and BPSK's soft arm deliberately skips `cancel_crossfade_isi` (#832, to protect the LLR + /// calibration HARQ combining relies on). Coded decodes take the cancelled arm instead. + /// + /// So `crossfade_cancellation_lowers_awgn_ber` below — an **uncoded** BER test — guards a + /// demodulator that **no uncoded production decode uses**. Measured on its own fixture, 8 seeds: + /// the cancelled arm means **0.0127** against its `< 0.02` bar, the production arm means + /// **0.0336** and exceeds that bar on *every* seed. + /// + /// This is a CHARACTERISATION test, not an endorsement. It pins the divergence so it cannot + /// change silently, and so that whoever settles #1429 does so deliberately: the current split + /// hands uncoded traffic the fade-favourable arm and coded traffic the AWGN-favourable one, + /// which #1363 measured and which may well be the right answer for §97.119 station ID, + /// handshake, QSY and relay frames that live on a fading channel. What is not defensible is + /// that nothing tested it. + #[test] + fn the_uncoded_production_path_takes_the_uncancelled_arm() { + fn add_noise(samples: &mut [f32], sigma: f32, mut seed: u64) { + let mut next = || { + seed = seed + .wrapping_mul(6364136223846793005) + .wrapping_add(1442695040888963407); + ((seed >> 11) as f64 / (1u64 << 53) as f64) as f32 + }; + for s in samples.iter_mut() { + let u1 = next().max(1e-7); + let u2 = next(); + let g = (-2.0 * u1.ln()).sqrt() * (std::f32::consts::TAU * u2).cos(); + *s += sigma * g; + } + } + let plugin = BpskPlugin::new(); + let cfg = ModulationConfig { + mode: "BPSK250".to_string(), + ..ModulationConfig::default() + }; + // The SAME payload and noise level as `crossfade_cancellation_lowers_awgn_ber`, by + // construction: a different fixture would not be comparable to the bar being cited. + let payload: Vec = (0..180u32) + .map(|i| (i.wrapping_mul(37).wrapping_add(11) & 0xff) as u8) + .collect(); + let clean = plugin.modulate(&payload, &cfg).expect("modulate"); + let ber = |bytes: &[u8]| { + let n = payload.len().min(bytes.len()); + let e: u32 = (0..n).map(|i| (payload[i] ^ bytes[i]).count_ones()).sum(); + e as f32 / (n * 8) as f32 + }; + + let (mut cancelled, mut production) = (0.0f32, 0.0f32); + const SEEDS: u64 = 8; + for k in 0..SEEDS { + let mut noisy = clean.clone(); + add_noise( + &mut noisy, + 0.9, + 0x1234_5678u64.wrapping_add(k.wrapping_mul(0x9E37_79B9)), + ); + cancelled += ber(&plugin.demodulate(&noisy, &cfg).expect("demodulate")); + // Exactly what `receive_from_samples` does on the uncoded path: soft LLRs, hard-decided. + let llrs = plugin + .demodulate_soft(&noisy, &cfg) + .expect("demodulate_soft"); + let bytes: Vec = llrs + .iter() + .map(|l| *l < 0.0) + .collect::>() + .chunks(8) + .map(|c| { + c.iter() + .enumerate() + .fold(0u8, |b, (i, &v)| b | ((v as u8) << i)) + }) + .collect(); + production += ber(&bytes); + } + cancelled /= SEEDS as f32; + production /= SEEDS as f32; + + assert!( + cancelled < 0.02, + "the CANCELLED arm ({cancelled:.4}) no longer meets #821's bar — that is a regression \ + in the arm `crossfade_cancellation_lowers_awgn_ber` tests, not this divergence" + ); + assert!( + production > cancelled * 1.5, + "the production (uncancelled) arm {production:.4} is no longer materially worse than \ + the cancelled arm {cancelled:.4} on #821's own fixture. If the two paths have been \ + reconciled, #1429 is resolved — delete this test and say which arm won, rather than \ + loosening it" + ); + assert!( + production > 0.02, + "the production arm {production:.4} now MEETS #821's bar. Either the uncoded path was \ + switched to the cancelled arm or the soft arm gained the cancellation — either way \ + #1429 is resolved and this test should be replaced by one asserting the bar directly" + ); + } + /// Crossfade-ISI cancellation must lower the BPSK AWGN bit-error rate: the uncancelled `+β` bias in /// the differential dot product costs several dB of flip-bit margin. Deterministic (fixed-seed noise). #[test]