Skip to content

Commit b16931c

Browse files
rafael2kclaude
andcommitted
watterson: run the Doppler filter in double, it diverges at high sample rates
The fading tap is a 2nd-order Butterworth driven by white noise. As fc/fs shrinks its poles crowd the unit circle: at 48 kHz with 1 Hz Doppler the ratio is 2e-5 and the poles sit within ~1e-4 of the circle, where float's ~6e-8 of relative resolution is enough to place one OUTSIDE it. The filter then diverges instead of fading, and the "measured signal power" is the blow-up: fs= 8000 2 paths 1 Hz -> SNR3k -8.37 dB sane fs=48000 1 path 0 Hz -> SNR3k -8.24 dB sane (no Doppler filter) fs=48000 2 paths 1 Hz -> SNR3k +65.76 dB diverged Coefficients and filter state are now double. After the change the same case reads -6.43 dB, and 8 kHz is unmoved (-8.37 -> -8.32, inside the scatter of a 2 s window of slow fading). Mercury runs at 8 kHz, which is why this sat unnoticed: 8 kHz is 6x further from the cliff and stayed marginally stable. It surfaced only when driving a 48 kHz modem through the same channel, and it matters now because wider bandwidth means higher sample rates -- this model is the instrument behind every sensitivity number in the project, so it has to hold up above 8 kHz before any of those numbers can be trusted there. No behaviour change at 8 kHz: the MFSK AWGN cliff is unchanged (11/20 at -11.1 dB, 19/20 at -10.1 dB) and the 1 Hz fading point moved 0.20 -> 0.25 FER over 60 trials, which is 3 frames and a necessarily different RNG sequence. The test pins two properties and was verified to fail on the old code: - fading stays finite and in range over 8/16/24/48/96 kHz x 0.1..2 Hz. The band is wide on purpose; it is there to catch divergence, not pin a value. - a SINGLE static path is rate-invariant to 0.5 dB (measured: 0.04 dB). That is the property that lets an 8 kHz result be compared with a 48 kHz one. Deliberately not two paths -- a delayed copy adds a multipath interference term that really does vary with rate (~1.5 dB), which would turn this into a test of interference rather than of the model's rate handling. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent ba1dce9 commit b16931c

4 files changed

Lines changed: 129 additions & 26 deletions

File tree

common/watterson.c

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -47,23 +47,23 @@ static float gaussian()
4747
*
4848
* Result: H(z) = (b0 + b1·z⁻¹ + b2·z⁻²) / (1 + a1·z⁻¹ + a2·z⁻²)
4949
*/
50-
static void butterworth_2nd_lp(float fc, float fs,
51-
float *b0, float *b1, float *b2,
52-
float *a1, float *a2)
50+
static void butterworth_2nd_lp(double fc, double fs,
51+
double *b0, double *b1, double *b2,
52+
double *a1, double *a2)
5353
{
5454
/* Pre-warp cutoff frequency */
55-
float c = tanf(M_PI * fc / fs);
56-
float c2 = c * c;
57-
float sqrt2c = 1.41421356237f * c; /* √2 */
55+
double c = tan(M_PI * fc / fs);
56+
double c2 = c * c;
57+
double sqrt2c = 1.41421356237309505 * c; /* sqrt(2) */
5858

59-
float den0 = 1.0f + sqrt2c + c2;
59+
double den0 = 1.0 + sqrt2c + c2;
6060

6161
*b0 = c2 / den0;
62-
*b1 = 2.0f * c2 / den0;
62+
*b1 = 2.0 * c2 / den0;
6363
*b2 = c2 / den0;
6464

65-
*a1 = 2.0f * (c2 - 1.0f) / den0;
66-
*a2 = (1.0f - sqrt2c + c2) / den0;
65+
*a1 = 2.0 * (c2 - 1.0) / den0;
66+
*a2 = (1.0 - sqrt2c + c2) / den0;
6767
}
6868

6969
/* Apply a 2nd-order IIR filter to one scalar sample.
@@ -73,9 +73,9 @@ static void butterworth_2nd_lp(float fc, float fs,
7373
*
7474
* The x[3] and y[3] arrays hold {x[n-2], x[n-1], x[n]}.
7575
*/
76-
static float iir_tick(float xn, float x[3], float y[3],
77-
float b0, float b1, float b2,
78-
float a1, float a2)
76+
static double iir_tick(double xn, double x[3], double y[3],
77+
double b0, double b1, double b2,
78+
double a1, double a2)
7979
{
8080
/* shift input history */
8181
x[0] = x[1];
@@ -178,9 +178,9 @@ int watterson_add_path(watterson_t *w, float delay_ms, float doppler_hz,
178178
/* The filter bandwidth is matched to the Doppler spread.
179179
* A Butterworth LPF with fc = σ provides a reasonable
180180
* approximation of the Gaussian spectrum. */
181-
float fc = p->doppler_hz;
181+
double fc = p->doppler_hz;
182182

183-
butterworth_2nd_lp(fc, (float)w->sample_rate,
183+
butterworth_2nd_lp(fc, (double)w->sample_rate,
184184
&p->b0, &p->b1, &p->b2,
185185
&p->a1, &p->a2);
186186

@@ -196,15 +196,15 @@ int watterson_add_path(watterson_t *w, float delay_ms, float doppler_hz,
196196
* narrow Doppler: the fade is ~constant over any short window, so it
197197
* would normalise to a single fade realisation, not the ensemble.) */
198198
{
199-
float xh[3] = {0,0,0}, yh[3] = {0,0,0};
199+
double xh[3] = {0,0,0}, yh[3] = {0,0,0};
200200
double e_h = 0.0;
201-
float in = 1.0f; /* unit impulse at n=0, zero thereafter */
201+
double in = 1.0; /* unit impulse at n=0, zero thereafter */
202202
int n;
203203
for (n = 0; n < 2000000; n++)
204204
{
205-
float y = iir_tick(in, xh, yh, p->b0, p->b1, p->b2, p->a1, p->a2);
206-
in = 0.0f;
207-
e_h += (double)y * y;
205+
double y = iir_tick(in, xh, yh, p->b0, p->b1, p->b2, p->a1, p->a2);
206+
in = 0.0;
207+
e_h += y * y;
208208
if (n > 2000 && (double)y * y < e_h * 1e-13)
209209
break; /* impulse-response tail is negligible */
210210
}

common/watterson.h

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -44,14 +44,21 @@ typedef struct watterson_path
4444

4545
/* IIR filter state for Doppler shaping (2nd-order Butterworth LPF)
4646
* Separate filters are used for the I and Q components */
47-
float x_i[3]; /* Input history, I component */
48-
float y_i[3]; /* Output history, I component */
49-
float x_q[3]; /* Input history, Q component */
50-
float y_q[3]; /* Output history, Q component */
47+
/* double, not float: at a high sample rate with a slow Doppler the
48+
* cutoff ratio fc/fs becomes tiny and the Butterworth poles sit within
49+
* ~1e-4 of the unit circle. float carries only ~6e-8 of relative
50+
* resolution there, which is enough to push a pole OUTSIDE the circle:
51+
* the filter then diverges instead of fading. Measured before the change,
52+
* 2 paths at 1 Hz: 8 kHz gave a sane SNR3k of -8.37 dB but 48 kHz gave
53+
* +65.76 dB, i.e. the "signal" was the filter blowing up. */
54+
double x_i[3]; /* Input history, I component */
55+
double y_i[3]; /* Output history, I component */
56+
double x_q[3]; /* Input history, Q component */
57+
double y_q[3]; /* Output history, Q component */
5158

5259
/* IIR filter coefficients (a0 is normalised to 1.0) */
53-
float b0, b1, b2; /* Numerator */
54-
float a1, a2; /* Denominator (a0 = 1) */
60+
double b0, b1, b2; /* Numerator (double: see the history arrays) */
61+
double a1, a2; /* Denominator (a0 = 1) */
5562

5663
/* Frequency offset state */
5764
float phase; /* Current phase accumulator for rotation */

tests/Makefile

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ ARQ_STUBS = datalink_arq/arq_test_stubs.c
4545
TEST_BINS = test_ring_buffer test_arq_protocol test_arq_timing test_arq_fsm \
4646
test_tcp_interfaces test_resampler test_arq_ladder test_arq_sim \
4747
test_arq_conn test_cfg_utils test_arq_tnc test_channel_busy test_mfsk \
48+
test_watterson \
4849
test_mfsk_modem test_pattern_ack_detection test_freedv_harq \
4950
test_pcm24 test_sock_wire test_virtual_clock
5051

@@ -121,6 +122,10 @@ test_arq_conn: datalink_arq/test_arq_conn.c $(UNITY_SRC) \
121122
datalink_arq/arq_conn_accessors.c
122123
$(CC) $(CFLAGS) -o $@ $^ $(LDFLAGS)
123124

125+
# -I.. : watterson.h includes "modem/freedv/comp.h" relative to the repo root
126+
test_watterson: common/test_watterson.c $(UNITY_SRC) ../common/watterson.c
127+
$(CC) $(CFLAGS) -I.. -o $@ $^ $(LDFLAGS)
128+
124129
test_cfg_utils: common/test_cfg_utils.c $(UNITY_SRC) \
125130
../common/cfg_utils.c \
126131
../common/iniparser/iniparser.c ../common/iniparser/dictionary.c

tests/common/test_watterson.c

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
/* Watterson channel model — numerical sanity across sample rates.
2+
*
3+
* The model is the instrument behind every sensitivity number in this project,
4+
* so it needs a guard of its own. The Doppler tap is a 2nd-order Butterworth
5+
* driven by white noise; as fc/fs shrinks its poles crowd the unit circle, and
6+
* in single precision they can land OUTSIDE it. The filter then diverges
7+
* instead of fading, and the "measured SNR" becomes the blow-up rather than
8+
* the signal: at 48 kHz with 1 Hz Doppler this read +65.76 dB where 8 kHz read
9+
* -8.37 dB. Mercury runs at 8 kHz today, which is why it went unnoticed --
10+
* it surfaced only when driving a 48 kHz modem through the same channel.
11+
*/
12+
#include "unity.h"
13+
#include "watterson.h"
14+
#include <math.h>
15+
#include <stdlib.h>
16+
17+
void setUp(void) {}
18+
void tearDown(void) {}
19+
20+
static float measure(int fs, float doppler_hz, int two_path)
21+
{
22+
watterson_t w;
23+
TEST_ASSERT_EQUAL_INT(0, watterson_init(&w, fs));
24+
watterson_add_path(&w, 0.0f, doppler_hz, 0.0f, 1.0f);
25+
if (two_path) watterson_add_path(&w, 2.0f, doppler_hz, 0.0f, 1.0f);
26+
watterson_set_noise(&w, -20.0f);
27+
watterson_reset_meas(&w);
28+
29+
int n = fs * 2;
30+
COMP *s = malloc(sizeof(COMP) * (size_t)n);
31+
TEST_ASSERT_NOT_NULL(s);
32+
for (int i = 0; i < n; i++) {
33+
s[i].real = 3000.0f * sinf(2.0f * (float)M_PI * 1500.0f * i / fs);
34+
s[i].imag = 0.0f;
35+
}
36+
watterson_process(&w, s, n);
37+
38+
for (int i = 0; i < n; i++)
39+
TEST_ASSERT_TRUE_MESSAGE(isfinite(s[i].real) && isfinite(s[i].imag),
40+
"channel produced a non-finite sample");
41+
float snr = watterson_measured_snr3k(&w);
42+
free(s);
43+
watterson_dispose(&w);
44+
return snr;
45+
}
46+
47+
void test_fading_is_stable_across_sample_rates(void)
48+
{
49+
/* Same physical channel at every rate, so the measured SNR3k must land in
50+
* the same ballpark. The band is wide because a 2 s window of slow fading
51+
* samples only a few fades and scatters by several dB -- it is here to
52+
* catch divergence, not to pin a value. */
53+
const int rates[] = { 8000, 16000, 24000, 48000, 96000 };
54+
const float dopplers[] = { 0.1f, 0.5f, 1.0f, 2.0f };
55+
56+
for (unsigned r = 0; r < sizeof(rates)/sizeof(rates[0]); r++) {
57+
for (unsigned d = 0; d < sizeof(dopplers)/sizeof(dopplers[0]); d++) {
58+
float snr = measure(rates[r], dopplers[d], 1);
59+
char msg[128];
60+
snprintf(msg, sizeof msg,
61+
"fs=%d doppler=%.1f Hz gave SNR3k=%.2f dB (filter diverged?)",
62+
rates[r], dopplers[d], snr);
63+
TEST_ASSERT_TRUE_MESSAGE(isfinite(snr), msg);
64+
TEST_ASSERT_TRUE_MESSAGE(snr > -40.0f && snr < 5.0f, msg);
65+
}
66+
}
67+
}
68+
69+
void test_single_static_path_is_rate_invariant(void)
70+
{
71+
/* One static path is the clean cross-rate invariant: the tap is a
72+
* constant, so an identical channel must measure identically whatever the
73+
* sample rate. This is what makes an 8 kHz result comparable with a 48 kHz
74+
* one, so it is worth pinning tightly -- measured agreement is 0.04 dB.
75+
*
76+
* Deliberately NOT two paths: summing a delayed copy adds a multipath
77+
* interference term that genuinely does depend on the rate (~1.5 dB here),
78+
* which would make this a test of interference rather than of the model's
79+
* rate handling. */
80+
float a = measure(8000, 0.0f, 0), b = measure(48000, 0.0f, 0);
81+
TEST_ASSERT_TRUE(isfinite(a) && isfinite(b));
82+
TEST_ASSERT_FLOAT_WITHIN(0.5f, a, b);
83+
}
84+
85+
int main(void)
86+
{
87+
UNITY_BEGIN();
88+
RUN_TEST(test_fading_is_stable_across_sample_rates);
89+
RUN_TEST(test_single_static_path_is_rate_invariant);
90+
return UNITY_END();
91+
}

0 commit comments

Comments
 (0)