test(bpsk): pin that uncoded BPSK takes the UNCANCELLED arm, and fix a false comment (#1429) - #1431
Merged
Merged
Conversation
…a false comment (#1429) 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0188ATCj6DZ9aRVQ2vSirua6
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1429 as far as measurement and evidence go. No behaviour change — no arm was switched, and
that is deliberate.
The finding
ModemEngine::receive_from_samplesprefersdemodulate_softwhenever the plugin advertises one, andBpskPlugin::supports_soft_demodreturnstrueunconditionally. So every uncoded decode(
FecMode::None,receive(),decode_burst_phase1) hard-decides the SOFT arm's LLRs — and BPSK'ssoft arm deliberately skips
cancel_crossfade_isi(#832, to protect the LLR calibration HARQcombining relies on). Every coded decode takes the cancelled arm.
Measured on #821's own fixture — same 180-byte payload, same σ = 0.9, same deterministic LCG
noise, 8 seeds:
< 0.02bardemodulate(cancelled)demodulate_softhard-decidedSo
crossfade_cancellation_lowers_awgn_ber— an uncoded BER test — guards a demodulator that nouncoded production decode executes, and the arm that ships would not pass it.
What this PR does
the_uncoded_production_path_takes_the_uncancelled_arm, on fix(bpsk): audit #1 — cancel crossfade ISI on the differential demod #821'sfixture by construction — a different fixture would not be comparable to the bar being cited.
Each of its three assertions names what its own failure would mean:
from this divergence;
been resolved and the test should be deleted rather than loosened.
engine.rsjustified preferring soft partly on the grounds that ahard retry "can't succeed where the soft pass failed — both share the same acquisition front
end". That is false for the one plugin whose arms differ, and they differ by exactly the
transform under discussion. The comment now carries the measurement, names the pinning test, and
states the choice is open.
What it deliberately does NOT do
Decide which arm uncoded traffic should use. That is a real trade, not an oversight to reverse on
sight:
96 on a delayed-dominant fade.
(An OTA-enabled daemon cannot receive any uncoded frame — filexfer, handshake, QSY and relay traffic are undecodable whenever
ota_enabled = true#1123) — frames that live on fading channels.So today's split hands uncoded traffic the fade-favourable arm and coded traffic the
AWGN-favourable one, which may well be right for what each actually carries. What was not
defensible is that it arose from a capability flag that exists for a different purpose, was defended
by a wrong comment, and that nothing tested the shipping path. Three candidate closures are on the
issue; the question may also dissolve if #1428's gated canceller lands and both arms can take it
(#1361's scope).
Test results
All 13 steps
ok. 2579 tests againstmain's 2578 — the one new pin.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
Review: none — the defect and its shape were identified in the #1428 design review, which
recommended "fix or document the
receive_from_samplesarm choice"; this documents and pins it andchanges no behaviour. The product decision it leaves open is recorded on #1429 for the maintainer.
🤖 Generated with Claude Code
https://claude.ai/code/session_0188ATCj6DZ9aRVQ2vSirua6