Fix Hilbert filter: quadrature branch used the in-phase (I) taps#3
Open
spinkham wants to merge 1 commit into
Open
Fix Hilbert filter: quadrature branch used the in-phase (I) taps#3spinkham wants to merge 1 commit into
spinkham wants to merge 1 commit into
Conversation
Hilbert::filter_block formed both the real and imaginary parts of the analytic output from the in-phase coefficient table (IHilbertBPFirCoef). The quadrature pointer QKptr was initialised but never used: the queue stores the real input duplicated into both cmplx components, and the loop multiplied the whole cmplx sample by the scalar I coefficient, so I and Q came out identical (a 45-degree-rotated real signal) instead of a 90-degree Hilbert pair. Because the "analytic" input to the fading stage was not truly analytic, the Rayleigh fading it produced was not circular. The faded envelope of a CW probe measured mean/rms ~= 0.78 (Hoyt/Nakagami-like, roughly double the deep-fade probability) rather than the Rayleigh value 0.886. Apply the I taps to the real accumulation and the Q taps to the imaginary accumulation. After the fix a CW tone's analytic envelope is essentially constant (|z| std/mean 0.52 -> 0.02) and the 2-path CCIR-Poor faded envelope is Rayleigh (mean/rms 0.90). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
rafael2k
added a commit
to Rhizomatica/pathsim
that referenced
this pull request
Jul 16, 2026
… Rayleigh) The analytic-signal FIR applied the in-phase tap table (IKptr) to BOTH output components; the quadrature table pointer (QKptr) was initialised but never dereferenced. So I == Q, the 'analytic' signal was not analytic, and the fading stage produced a non-circular Rayleigh: a CW probe's faded envelope measured mean/rms ~0.78 vs the correct 0.886 — roughly double the deep-fade probability. Build the imaginary part from the Q (Hilbert) taps, walking both coefficient tables in lockstep. Bug found and reported by Steve Pinkham (N4FPV); cf. bubnikv#3. Co-Authored-By: Claude Opus 4.8 <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
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.
Summary
Hilbert::filter_block(Delay.cpp) builds both the real and imaginary partsof the analytic output from the in-phase coefficient table (
IHilbertBPFirCoef).The quadrature pointer
QKptris initialised but never used: the queue stores thereal input duplicated into both
cmplxlanes, and the inner loop multiplies thewhole
cmplxsample by the scalar I coefficient, so I and Q come out identical —a 45°-rotated real signal rather than a 90° Hilbert pair.
Because the "analytic" input to the fading stage is not truly analytic, the
Rayleigh fading it produces is not circular. For a CW probe faded through a 2-path
CCIR-Poor channel the output envelope is Hoyt/Nakagami-like (mean/rms ≈ 0.78,
roughly double the deep-fade probability of Rayleigh) instead of the Rayleigh
value 0.886.
Fix
Apply the I taps to the real accumulation and the Q taps (the already-computed
QKptr) to the imaginary accumulation:cmplx acc{0., 0.}; for (int j = 0; j < HILBPFIR_LENGTH; ++ j, ++ Firptr) { acc.r += Firptr->r * (*IKptr++); // in-phase: band-pass (I) taps acc.i += Firptr->i * (*QKptr++); // quadrature: Hilbert (Q) taps } pOut[i] = acc;Verification
1500 Hz CW probe faded through
--spread 1.0 --delay2 2 --spread2 1.0:|z|std/mean (0 = ideal)After the fix the faded-envelope distribution matches Rayleigh across the band.
Note: this was verified on a byte-identical
Delay.cpp(the Rhizomatica tree),not a fresh build of this repo —
mastercurrently fails to build on GCC-13 dueto a missing
<cstring>include, which is unrelated to this change.