fix(bpsk): the GPU demodulator never cancelled the crossfade ISI - #1434
Merged
Merged
Conversation
`BpskPlugin::demodulate` dispatches to the GPU when a context exists (lib.rs:112), and `bpsk_demodulate_with_gpu`'s non-RRC branch went GPU timing search -> `bpsk_iq_demod_gpu` -> slice -> `differential_decode` with NO `cancel_crossfade_isi`. The CPU arm cancels at demodulate.rs:114. The GPU path landed 2026-05-04 (664122b); #821 added the cancellation 2026-07-13 to `symbol_stream_with_expected` only. THIS SHIPS. The daemon is `default = ["gpu"]` and registers `BpskPlugin::with_gpu(ctx)` whenever an adapter is present, so the coded BPSK receive took the uncancelled arm -- and `demodulate_soft` has no GPU path and already skips cancellation by design (#832), so on a GPU daemon BOTH arms were uncancelled. Measured, 200 B BPSK250, 16 seeds, total-power SNR: SNR CPU BER GPU BER before ratio CPU ok GPU before GPU after 0 dB 0.00023 0.00328 14.0x 11/16 0/16 11/16 2 dB 0.00000 0.00047 -- 16/16 9/16 16/16 4 dB 0.00000 0.00000 -- 16/16 16/16 16/16 Cancellation runs on the whole symbol stream before the preamble/tail slice, mirroring the CPU ordering: it is a backward substitution, so running it on a slice changes the boundary symbol. The RRC branch is untouched -- it returns earlier and does not crossfade. WHY NOTHING CAUGHT IT. `gpu_and_cpu_agree_under_noise` swept 4-20 dB on a 27-byte payload; BPSK250 at 8 kHz has ~15 dB of processing gain, and 4 dB is measured above as the FIRST SNR at which the difference vanishes. The fixture's easiest cell was exactly the boundary. The new test runs 0 and 2 dB, asserts the mechanism (BER ratio <= 2x) and the outcome (decode counts), and refuses to run in a cell where the CPU cannot decode or makes no errors, so it cannot go vacuous. Corrects #1080's record: `gpu_and_cpu_agree_under_a_carrier_offset` printed "GPU decoded 6 of 30 frames the CPU did not ... the two searches still disagree off-frequency. See #1080." It now prints 0. That divergence was this defect, not the timing search. Twins swept with reasons: `psk8_demodulate_gpu` returns None for non-RRC modes and RRC does not crossfade; qpsk's `demodulate` never dispatches to the GPU; 64qam has no crossfade canceller. EVIDENCE TIER, stated because it is lower than usual: these tests are `#![cfg(feature = "gpu")]` and the workspace gate runs `--no-default-features`, so the gate CANNOT run them; `gate.sh`'s `--all-features` pass is compile+lint only and CI's gpu job was removed in #1380. Run by hand on a host with a working adapter: 5 passed, 0 failed; the new test fails before the fix (0/16 vs 11/16) with the other four passing. Closes #1433 Refactors: CAP-12 Verification-objective: a GPU-accelerated arm must agree with its CPU twin in a cell where the property under test decides the frame, not only where both arms succeed Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0188ATCj6DZ9aRVQ2vSirua6
The defect was found by adversarial review of the #1428 union design, not by reading the GPU code — the review asked what would have to be true for that design to work on the shipped binary. Records the finding's provenance, the independent verification of each structural claim before acting on it, and the transferable lesson: an equivalence test between an accelerated path and its reference must run in a cell where the property under test decides the outcome. Measured, 4 dB is the first SNR at which the two arms stop differing, and the existing fixture's easiest cell sat exactly there. Refactors: CAP-12 Verification-objective: a GPU-accelerated arm must agree with its CPU twin in a cell where the property under test decides the frame, not only where both arms succeed 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.
BpskPlugin::demodulatedispatches to the GPU when a context exists (plugins/bpsk/src/lib.rs:112),and
bpsk_demodulate_with_gpu's non-RRC branch went GPU timing search →bpsk_iq_demod_gpu→ slice→
differential_decodewith nocancel_crossfade_isi. The CPU arm cancels atdemodulate.rs:114.The GPU path landed 2026-05-04 (
664122b9, Phase 3.3). #821 added the cancellation 2026-07-13 andtouched
symbol_stream_with_expectedonly. So the two arms have disagreed for four months.This ships
crates/openpulse-daemon/Cargo.toml:28isdefault = ["gpu"], andserver.rs:142registersBpskPlugin::with_gpu(ctx)wheneverGpuContext::init()finds an adapter. On such a host the codedBPSK receive took the uncancelled arm — and
demodulate_softhas no GPU path and already skips thecancellation by design (#832), so both arms were uncancelled there.
hpx_hf's SL2–SL5 are allBPSK, and SL2 is the rung every session starts on.
Measured — 200 B BPSK250, 16 seeds, total-power SNR
After the fix the arms agree bit-for-bit (BER 0.00023 both at 0 dB).
Why nothing caught it — and why the new cell is not just "one more cell"
gpu_and_cpu_agree_under_noisesweeps 4 / 8 / 12 / 20 dB total-power SNR on a 27-byte payload.BPSK250 at 8 kHz is 32 samples/symbol (~15 dB of processing gain), so its lowest cell sits near
19 dB Eb/N0. The measurement above shows the sharper version of that argument: 4 dB is the first
SNR at which the difference vanishes — 16/16 both ways. The existing fixture's easiest cell sat
exactly on the boundary of the property it was named after.
gpu_and_cpu_agree_where_the_cancellation_decides_the_frameruns 0 and 2 dB and asserts both:It carries its own anti-vacuity guard: the cell must be one where the CPU both decodes some frames
and makes some errors, or the ratio is meaningless and the counts uninformative. A cell that drifted
back up to "both arms succeed" now fails on
cpu_ok > 0's sibling condition rather than passingquietly.
Failure-first, in order: the new test fails on the unfixed code (0/16 against 11/16) while the
other four pass; it passes after the fix. That pair is what makes it discriminating rather than
merely added.
The fix
Cancel on the whole symbol stream immediately after
bpsk_iq_demod_gpuand before thepreamble/tail slice, mirroring the CPU ordering — the cancellation is a backward substitution, so
running it on a slice changes the boundary symbol. The RRC branch is untouched: it returns earlier
and RRC does not crossfade.
A correction to #1080's record
gpu_and_cpu_agree_under_a_carrier_offsetprinted "GPU decoded 6 of 30 frames the CPU did not. Nota regression; the two searches still disagree off-frequency. See #1080." After this fix it prints 0.
That divergence was this defect, not the timing search it was attributed to.
Twins — swept with reasons, not absence
psk8_demodulate_gpureturnsNonefor non-RRC modes (plugins/psk8/src/demodulate.rs:377), andRRC does not crossfade — so its GPU path never reaches a crossfade pulse.
qpsk'sdemodulatenever dispatches to the GPU at all (plugins/qpsk/src/lib.rs:102).64qamhas no crossfade canceller.BPSK was the only affected plugin.
Evidence tier — lower than usual, and stated because of it
These tests are
#![cfg(feature = "gpu")]. The workspace gate runs--no-default-features, so itcannot run them;
gate.sh's--all-featurespass is compile + lint only; and CI'sgpujob wasremoved in #1380 as subsumed by that pass. So nothing runs them automatically, and that absence is
why this survived four months. The numbers above were produced by hand on a host with a working
adapter —
the_adapter_is_available_or_this_file_proves_nothingpasses here, so the suite is notsilently skipping.
cargo test -p bpsk-plugin --features gpu --test gpu_cpu_equivalence→ 5 passed, 0 failed.Workspace gate:
GATE: PASS 88a5649a clean, suites=339 tests_passed=2579 tests_failed=0.I have not proposed a CI job for this: GitHub runners have no adapter, and a job that skips is
the vacuous-gate shape this repo bans. Closing that gap honestly needs a self-hosted runner or an
accepted manual tier, which is a maintainer decision rather than something to slip into this PR.
Closes #1433. Prerequisite for #1428's union, which is degenerate on a GPU daemon until this lands.
Review: docs/dev/reviews/review-1433-gpu-crossfade.md
Refactors: CAP-12
Verification-objective: a GPU-accelerated arm must agree with its CPU twin in a cell where the property under test decides the frame, not only where both arms succeed
🤖 Generated with Claude Code
https://claude.ai/code/session_0188ATCj6DZ9aRVQ2vSirua6