Conversation
…Python receivers check_snr.py: use re.search(r'state:\s+sync') instead of literal 'state: sync' to match both Python radae_rxe.py (single space) and C radae_rx (padded spaces) formats. CMakeLists.txt: replace hardcoded --snr3k targets with the Measured SNR3k parsed dynamically from inference.sh stdout. The hardcoded values were Target SNR3k which is ~0.8 dB higher than Measured due to PAPR under --peak normalisation. Tighten tolerance from 3.0 to 2.0 dB now that the correct reference is used (worst observed error 1.43 dB). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Three new ctests exercising the modem equalization code via QPSK BER: - radae_nopy_ber: noiseless check (BER must be 0.0000) - radae_nopy_ber_awgn: AWGN at EbNodB=0, BER < theoretical QPSK target with 2 dB implementation loss allowance - radae_nopy_ber_mpp: MPP two-path Rayleigh at EbNodB=0, BER < theoretical two-path target with 2 dB implementation loss allowance Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This reverts commit 023359d.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The LS pseudo-inverse for pilot channel estimation was using regular transpose A^T (torch.transpose) instead of conjugate transpose A^H (torch.conj(torch.transpose)) for the complex A matrix. The Octave reference implementation (equaliser.m) correctly uses A' which is A^H in Octave. Python's torch.transpose is A^T, not A^H. The bug was invisible in practice because: - EQ quality is unaffected (dominant G1 term estimated identically) - SNR calibration coefficients were fitted against the biased estimator With A^H the phase estimator noise alpha is reduced, so S2 more accurately estimates N/2, giving a more accurate SNR estimate. Recalibrate m and c by running est_snr.py --eq_ls for AWGN, MPG, MPP and averaging the polyfit slopes and intercepts: AWGN: m=0.8872 c=3.8355 MPG: m=0.7627 c=4.6551 MPP: m=0.6452 c=3.9124 mean: m=0.7650 c=4.1343 Also fix missing model.Pend arg in est_snr.py receiver_one() call. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…s no complete breakdown
tmiw
reviewed
Apr 29, 2026
tmiw
left a comment
Collaborator
There was a problem hiding this comment.
Just a few minor comments. See below.
…r ch
ch is built as part of radae itself, so use ${CMAKE_BINARY_DIR}/src/ch
in radae_nopy_eoo_data_mpp, eliminating the CODEC2_DEV_BUILD_DIR dependency.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…IR guard Wrap the entire RADAE_NOPY C port test block in if(RADAE_NOPY_BUILD_DIR). Tests are only added when -DRADAE_NOPY_BUILD_DIR=... is passed to cmake, making radae_nopy an optional dependency with no impact on the default build. README.md: add section documenting how to build and run the optional radae_nopy ctests. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Closed
radae_nopy tests are now optional (if(RADAE_NOPY_BUILD_DIR) in CMakeLists.txt) so CI does not need to clone/build radae_nopy. Run all tests without the -R radae_nopy filter, matching the main branch workflow. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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
2 participants
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.
V1 pilot EQ & SNR
While using the latest Python reference
mainto test the C port https://github.com/peterbmarks/radae_nopy, a SNR estimator ctest fail lead Claude on a merry chase that found a bug in the Python equalisation code. The C code implementation is correct. The effects of the bug were surprisingly subtle, mainly a difference in the Python and C SNR estimator outputs. This PR is a fix to the Python code, including re-calibration of the SNR estimator. We will also need to re-calibrate the C SNR estimator.Thoughts:
Merge nopy tests
Merge NoPy ctest functionality from #66, as discussed in peterbmarks/radae_nopy#13