Skip to content

Audit the whole library: 37 verified defects found and fixed - #43

Merged
vincentchoqueuse merged 2 commits into
mainfrom
claude/comnumpy-v1-architecture-5dtrz3
Aug 14, 2026
Merged

vincentchoqueuse merged 2 commits into
mainfrom
claude/comnumpy-v1-architecture-5dtrz3

Conversation

@vincentchoqueuse

Copy link
Copy Markdown
Owner

Five independent reviews swept the whole library — core signal path, compensators/metrics/utils, optical, MIMO+FEC, and the chain machinery with OFDM — hunting for wrong axes, dtype loss, RNG misuse, numerical blow-ups and validation gaps. Every suspicion had to be verified by running a minimal reproduction before being counted: 37 confirmed defects, 37 fixed, each locked by a test in tests/test_audit_regressions.py (36 tests).

The numerical cores came back clean against independent references: closed-form SER curves cross-checked by Monte-Carlo and numeric fading integration (~1e-10), Viterbi against exhaustive ML (600 trials, metric-exact), ML against the sphere decoder bit-for-bit, LDPC min-sum against brute force, CD/DBP round trips to 1e-14, EDFA/Raman budgets against their closed forms, the (133,171) distance spectrum against the published tables. No block mutates its input.

Real mis-computations

  • Sequential.seed() never reached blocks inside a nested Sequential or a wrapper chain (the OFDM chains) — a seeded composite chain was not reproducible; it now recurses, with the flat-chain child seeds unchanged.
  • The AMP detector evaluated its posterior-variance term G at variance tau^2 instead of sigma^2 (1 + tau^2) (Jeon et al., Algorithm 2): at high SNR the state went NaN and was silently hard-projected to symbol 0. soft_projector also gains a log-sum-exp guard, which fixes the same divergence in OAMP.
  • ErrorCounter raveled before truncating, counting phantom errors on every batch row after the first under a filter delay.
  • CarrierAllocator silently zeroed complex pilots on real data; OFDMReceiver read the period of a 2-D scattered mask as N_fft.
  • compute_acpr counted a bin sitting exactly on a band edge in both bands.
  • SRRCFilter(rho=0) — the documented sinc case — crashed; a logarithmic split-step schedule at alpha_dB=0 was all-NaN (now the uniform spacing it is the limit of); the FIR CD compensator applied a sqrt(1/(4*pi*K)) gain when its design collapsed to one tap (now the identity it tends to); the optical 90° hybrid and the MZM crashed on any (2, N) input.
  • Sequential.summary ran the chain without feeding wiring or recording taps.

State that lied

FrameField froze the caller's array in place; Deframer recorded views of the input buffer; tap() served signals from a previous run or a previous taps= configuration; DataExtractor.forward overwrote its declared selector; BWFilter silently turned real signals complex.

Contract mismatches

A nested chain exported to JSON but failed to import; int-keyed callbacks were documented but never fired; ViterbiDecoder broke exactly at the empty message; the distribution matcher pair died on a zero-length stream; print_data died on degenerate tables.

Validation gaps closed with clear errors

Negative AWGN variance, cross-QAM orders in the square-QAM closed form, Serial2Parallel method, Complex2Real part, Upsampler phase, SampleRemover windows, CyclicPrefixer(N_cp > N), 1-D Parallel2Serial, short-signal SRRCFilter(method='fft'), numpy-integer sizes in the generators, empty compute_ser.

DataAidedMixin becomes a plain empty-slots class so the slotted compensators stay expressible under pyright's new disjoint-bases check.

Verification

  • 1011 tests + 1293 subtests green (including the new regression file), examples smoke tests green
  • ruff check . clean, pyright 0 errors, sphinx -W clean

🤖 Generated with Claude Code

https://claude.ai/code/session_01Pn2F8X4HZhS8fEJcL5enzw


Generated by Claude Code

claude added 2 commits August 14, 2026 04:56
The displayed sweep dict is `curves` everywhere -- the OFDM page's
`measured` and the AWGN page's `measured_ber` included -- its
closed-form companion is `theory` (the PAPR page's `reference` renamed,
since `reference` is the library's word for the known signal a
data-aided block compares against), and the sweep axis carries its
unit: the OFDM `snr_list` is `snr_dB_list`. Domain names survive only
where they say strictly more: `ccdf_curves` for the PAPR distribution
pairs, `snr_dB` in the DBP study, `snr_measured` for the per-span
check. The canon is stated in the tutorial skill.

The optical pages were audited against the doctrine on the way:
one_shot_pdm is already inline and loop-free, and the two DBP scripts
keep the one sanctioned chain split and their per-span progress table
-- six heterogeneous metrics over a slow loop, which print_data's
x-plus-curves contract does not describe.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Five independent reviews swept core, optical, MIMO, FEC, OFDM and the
chain machinery, verifying every suspicion by running a minimal
reproduction. The numerical cores came back clean against independent
references (closed forms vs Monte-Carlo, Viterbi vs exhaustive ML,
ML vs sphere decoder bit-for-bit, CD/DBP round trips, EDFA/Raman
budgets, the (133,171) distance spectrum). The confirmed defects are
fixed and locked by tests/test_audit_regressions.py:

- Sequential.seed() now reaches nested chains and wrapper blocks
- AMP posterior variance G at sigma2*(1+tau2) per Jeon et al.;
  soft_projector gains the log-sum-exp guard (fixes OAMP NaN too)
- ErrorCounter truncates along the last axis before pooling
- CarrierAllocator keeps complex pilots on real data
- OFDMReceiver reads N_fft off a 2-D scattered mask, not its period
- compute_acpr counts a band-edge bin once
- SRRCFilter(rho=0) is the sinc pulse it documents
- logarithmic split-step spacing degrades to uniform at alpha=0
- the FIR CD compensator degrades to identity below one tap of memory
- Sequential.summary feeds wiring and records taps; int-keyed
  callbacks fire; tapped_ is cleared at each pass
- FrameField freezes a copy, Deframer records copies,
  DataExtractor keeps its declared selector
- BWFilter keeps a real signal real (its mask is symmetric)
- nested Sequential round-trips through JSON
- ViterbiDecoder accepts the tail-only codeword (empty message)
- the distribution matcher pair handles an empty stream
- the optical LO draws along the sample axis (batched hybrid/MZM)
- validation gaps closed with clear errors: negative AWGN variance,
  cross-QAM closed form, S2P method, Complex2Real part, Upsampler
  phase, SampleRemover window, CyclicPrefixer N_cp > N, 1-D
  Parallel2Serial, short-signal SRRC fft, numpy-integer sizes,
  empty compute_ser, degenerate print_data tables

DataAidedMixin becomes a plain empty-slots class so the slotted
concrete compensators stay expressible under pyright's disjoint-bases
check.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@vincentchoqueuse
vincentchoqueuse merged commit b596fa2 into main Aug 14, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants