Change in osmo-bts[master]: MS/BS Power Control Loop: Fix downscaling averaging bug

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

pespin gerrit-no-reply at lists.osmocom.org
Mon Sep 6 10:06:43 UTC 2021


pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/25329 )

Change subject: MS/BS Power Control Loop: Fix downscaling averaging bug
......................................................................

MS/BS Power Control Loop: Fix downscaling averaging bug

The bug showed up in previous commit and is fixed in this commit. It can
be seen how rounding error is carried over time in the average
measurement, and affects final values.

Change-Id: I680d1c94bd4bae179b14b26662a819fa1462a5c8
---
M src/common/power_control.c
M tests/power/ms_power_loop_test.c
M tests/power/ms_power_loop_test.err
M tests/power/ms_power_loop_test.ok
4 files changed, 11 insertions(+), 9 deletions(-)

Approvals:
  Jenkins Builder: Verified
  laforge: Looks good to me, but someone else must approve
  pespin: Looks good to me, approved
  fixeria: Looks good to me, but someone else must approve



diff --git a/src/common/power_control.c b/src/common/power_control.c
index b3066cd..0362675 100644
--- a/src/common/power_control.c
+++ b/src/common/power_control.c
@@ -36,6 +36,8 @@
 
 /* We don't want to deal with floating point, so we scale up */
 #define EWMA_SCALE_FACTOR 100
+/* EWMA_SCALE_FACTOR/2 = +50: Round to nearest value when downscaling, otherwise floor() is applied. */
+#define EWMA_ROUND_FACTOR (EWMA_SCALE_FACTOR / 2)
 
 /* Base Low-Pass Single-Pole IIR Filter (EWMA) formula:
  *
@@ -84,8 +86,8 @@
 		return Val;
 	}
 
-	*Avg100 += A * (Val - *Avg100 / EWMA_SCALE_FACTOR);
-	return *Avg100 / EWMA_SCALE_FACTOR;
+	*Avg100 += A * (Val - (*Avg100 + EWMA_ROUND_FACTOR) / EWMA_SCALE_FACTOR);
+	return (*Avg100 + EWMA_ROUND_FACTOR) / EWMA_SCALE_FACTOR;
 }
 
 /* Calculate target RxLev value from lower/upper thresholds */
diff --git a/tests/power/ms_power_loop_test.c b/tests/power/ms_power_loop_test.c
index b9d6867..5f83329 100644
--- a/tests/power/ms_power_loop_test.c
+++ b/tests/power/ms_power_loop_test.c
@@ -204,11 +204,11 @@
 	/* Avg[t] = (0.2 * 40) + (0.8 * 29.60) = RXLEV 31.68 (-78.32 dBm),
 	 * but due to up-/down-scaling artefacts we get the following:
 	 *   Avg100[t] = Avg100[t - 1] + A * (Pwr - Avg[t] / 100)
-	 *   Avg100[t] = 2960 + 20 * (40 - (2960 / 100))
-	 *   Avg100[t] = 2960 + 20 * (40 - 29)
-	 *   Avg[t] = 3180 / 100 = 31.80 */
+	 *   Avg100[t] = 2960 + 20 * (40 - ((2960+50) / 100)) <- HERE we lose 0.1: (2960+50) / 100) = 30.1
+	 *   Avg100[t] = 2960 + 20 * (40 - 30) <- HERE we lose 20*0.1 = 2.0! (upscaled, hence we lose finally 2.0/100=0.2)
+	 *   Avg[t] = (3160) / 100 = 31.60*/
 	apply_power_test(lchan, -70, good_lqual, 1, 9); /* RXLEV 40 */
-	CHECK_RXLEV_AVG100(31.80);
+	CHECK_RXLEV_AVG100(31.60);
 
 	mp->ewma.alpha = 70; /* 30% smoothing */
 	lchan->ms_power_ctrl.current = 15;
diff --git a/tests/power/ms_power_loop_test.err b/tests/power/ms_power_loop_test.err
index 8ab6419..8f58882 100644
--- a/tests/power/ms_power_loop_test.err
+++ b/tests/power/ms_power_loop_test.err
@@ -23,8 +23,8 @@
 (bts=0,trx=0,ts=0,ss=0) Lowering MS power control level 14 (2 dBm) => 15 (0 dBm): ms-pwr-lvl[curr 14, max 0], RSSI[curr -40, avg -47, thresh -75..-75] dBm, C/I[curr 14, avg 14, thresh 12..16] dB
 (bts=0,trx=0,ts=0,ss=0) Keeping MS power at control level 15 (0 dBm): ms-pwr-lvl[curr 15, max 2], RSSI[curr -75, avg -75, thresh -75..-75] dBm, C/I[curr 14, avg 14, thresh 12..16] dB
 (bts=0,trx=0,ts=0,ss=0) Raising MS power control level 15 (0 dBm) => 13 (3 dBm): ms-pwr-lvl[curr 15, max 2], RSSI[curr -90, avg -78, thresh -75..-75] dBm, C/I[curr 14, avg 14, thresh 12..16] dB
-(bts=0,trx=0,ts=0,ss=0) Raising MS power control level 13 (4 dBm) => 11 (8 dBm): ms-pwr-lvl[curr 13, max 2], RSSI[curr -90, avg -81, thresh -75..-75] dBm, C/I[curr 14, avg 14, thresh 12..16] dB
-(bts=0,trx=0,ts=0,ss=0) Raising MS power control level 11 (8 dBm) => 9 (12 dBm): ms-pwr-lvl[curr 11, max 2], RSSI[curr -70, avg -79, thresh -75..-75] dBm, C/I[curr 14, avg 14, thresh 12..16] dB
+(bts=0,trx=0,ts=0,ss=0) Raising MS power control level 13 (4 dBm) => 11 (8 dBm): ms-pwr-lvl[curr 13, max 2], RSSI[curr -90, avg -80, thresh -75..-75] dBm, C/I[curr 14, avg 14, thresh 12..16] dB
+(bts=0,trx=0,ts=0,ss=0) Raising MS power control level 11 (8 dBm) => 9 (11 dBm): ms-pwr-lvl[curr 11, max 2], RSSI[curr -70, avg -78, thresh -75..-75] dBm, C/I[curr 14, avg 14, thresh 12..16] dB
 (bts=0,trx=0,ts=0,ss=0) Keeping MS power at control level 15 (0 dBm): ms-pwr-lvl[curr 15, max 2], RSSI[curr -50, avg -50, thresh -75..-75] dBm, C/I[curr 14, avg 14, thresh 12..16] dB
 (bts=0,trx=0,ts=0,ss=0) Keeping MS power at control level 15 (0 dBm): ms-pwr-lvl[curr 15, max 2], RSSI[curr -50, avg -50, thresh -75..-75] dBm, C/I[curr 14, avg 14, thresh 12..16] dB
 (bts=0,trx=0,ts=0,ss=0) Raising MS power control level 15 (0 dBm) => 13 (4 dBm): ms-pwr-lvl[curr 15, max 2], RSSI[curr -110, avg -92, thresh -75..-75] dBm, C/I[curr 14, avg 14, thresh 12..16] dB
diff --git a/tests/power/ms_power_loop_test.ok b/tests/power/ms_power_loop_test.ok
index 8c92b03..27992b1 100644
--- a/tests/power/ms_power_loop_test.ok
+++ b/tests/power/ms_power_loop_test.ok
@@ -62,7 +62,7 @@
 	Avg[t] is RxLev 29.60 (expected 29.60)
 lchan_ms_pwr_ctrl(RxLvl=-70 dBm) returns 1 (expected 1)
 	MS current power 11 -> 9 (expected 9)
-	Avg[t] is RxLev 31.80 (expected 31.80)
+	Avg[t] is RxLev 31.60 (expected 31.60)
 lchan_ms_pwr_ctrl(RxLvl=-50 dBm) returns 0 (expected 0)
 	MS current power 15 -> 15 (expected 15)
 	Avg[t] is RxLev 60.00 (expected 60.00)

-- 
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/25329
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I680d1c94bd4bae179b14b26662a819fa1462a5c8
Gerrit-Change-Number: 25329
Gerrit-PatchSet: 3
Gerrit-Owner: pespin <pespin at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy at sysmocom.de>
Gerrit-Reviewer: laforge <laforge at osmocom.org>
Gerrit-Reviewer: pespin <pespin at sysmocom.de>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20210906/b8700a0b/attachment.htm>


More information about the gerrit-log mailing list