bugfix,sched: avoid instability in TPCs around PHR~0

lib/include/srsran/adt/accumulators.h

@@ -30,6 +30,11 @@ struct rolling_average {
   }
   T        value() const { return count_ == 0 ? 0 : avg_; }
   uint32_t count() const { return count_; }
+  void     reset()
+  {
+    avg_   = 0;
+    count_ = 0;
+  }
 
 private:
   T        avg_   = 0;

srsenb/hdr/stack/mac/sched_ue_ctrl/tpc.h

@@ -77,7 +77,7 @@ public:
   {
     last_phr = phr_;
     for (auto& ch_snr : snr_estim_list) {
-      ch_snr.phr_flag = false;
+      ch_snr.acc_tpc_phr_values = 0;
     }
 
     // compute and cache the max nof UL PRBs that avoids overflowing PHR
@@ -109,6 +109,7 @@ public:
       if (ch_snr.pending_snr == null_snr) {
         ch_snr.last_snr_sample_count++;
         ch_snr.acc_tpc_values += ch_snr.win_tpc_values.oldest();
+        ch_snr.acc_tpc_phr_values += ch_snr.win_tpc_values.oldest();
       } else {
         ch_snr.acc_tpc_values = 0;
         ch_snr.snr_avg.push(ch_snr.pending_snr, ch_snr.last_snr_sample_count);
@@ -170,40 +171,43 @@ private:
       // undefined target SINR case
       return encode_tpc_delta(0);
     }
-    if ((tti_count - ch_snr.last_tpc_tti_count) < min_tpc_tti_interval) {
+
-      // more time required before sending next TPC
+    // limitation of TPC based on PHR
-      return encode_tpc_delta(0);
+    int max_delta = 3;
-    }
+    int eff_phr   = last_phr;
-    if (cc == PUSCH_CODE and last_phr < 0 and not ch_snr.phr_flag) {
+    if (cc == PUSCH_CODE and last_phr != undefined_phr) {
-      // if negative PHR and PUSCH
+      eff_phr -= ch_snr.win_tpc_values.value() + ch_snr.acc_tpc_phr_values;
-      logger.info("TPC: rnti=0x%x, PUSCH command=0 due to PHR=%d<0", rnti, last_phr);
+      max_delta = std::min(max_delta, eff_phr);
-      ch_snr.phr_flag      = true;
-      ch_snr.pending_delta = -1;
-      return encode_tpc_delta(ch_snr.pending_delta);
     }
 
-    // target SINR is finite and there is power headroom
+    float snr = ch_snr.last_snr_sample;
-    float diff = target_snr_dB - ch_snr.last_snr_sample;
+    // In case of periodicity of TPCs is > 1 tti, use average SNR to compute SNR diff
-    diff -= ch_snr.win_tpc_values.value() + ch_snr.acc_tpc_values;
+    if (min_tpc_tti_interval > 1) {
-    if (diff >= 1) {
+      ch_snr.tpc_snr_avg.push(snr);
-      ch_snr.pending_delta = diff > 3 ? 3 : 1;
+      if ((tti_count - ch_snr.last_tpc_tti_count) < min_tpc_tti_interval) {
-      if (cc == PUSCH_CODE and static_cast<int>(ch_snr.pending_delta) > last_phr) {
+        // more time required before sending next TPC
-        // cap PUSCH TPC when PHR is low
+        return encode_tpc_delta(0);
-        ch_snr.pending_delta = last_phr > 1 ? 1 : 0;
       }
-      ch_snr.last_tpc_tti_count = tti_count;
+      snr = ch_snr.tpc_snr_avg.value();
-    } else if (diff <= -1) {
-      ch_snr.pending_delta      = -1;
-      ch_snr.last_tpc_tti_count = tti_count;
     }
+
+    float diff = target_snr_dB - snr;
+    diff -= ch_snr.win_tpc_values.value() + ch_snr.acc_tpc_values;
+    ch_snr.pending_delta = std::max(std::min((int)floorf(diff), max_delta), -1);
+    ch_snr.pending_delta = (ch_snr.pending_delta == 2) ? 1 : ch_snr.pending_delta;
     if (ch_snr.pending_delta != 0) {
-      logger.debug("TPC: rnti=0x%x, %s command=%d, last SNR=%d, SNR average=%f, diff_acc=%f",
+      if (min_tpc_tti_interval > 1) {
+        ch_snr.last_tpc_tti_count = tti_count;
+        ch_snr.tpc_snr_avg.reset();
+      }
+      logger.debug("TPC: rnti=0x%x, %s command=%d, last SNR=%d, SNR average=%f, diff_acc=%f, eff_phr=%d",
                    rnti,
                    cc == PUSCH_CODE ? "PUSCH" : "PUCCH",
                    encode_tpc_delta(ch_snr.pending_delta),
                    ch_snr.last_snr_sample,
                    ch_snr.snr_avg.value(),
-                   diff);
+                   diff,
+                   eff_phr);
     }
     return encode_tpc_delta(ch_snr.pending_delta);
   }
@@ -225,8 +229,6 @@ private:
 
   // SNR estimation
   struct ul_ch_snr_estim {
-    // flag used in undefined target SINR case
-    bool phr_flag = false;
     // pending new snr sample
     float pending_snr = srsran::null_sliding_average<float>::null_value();
     // SNR average estimation with irregular sample spacing
@@ -234,10 +236,12 @@ private:
     srsran::exp_average_irreg_sampling<float> snr_avg;
     int                                       last_snr_sample;
     // Accumulation of past TPC commands
-    srsran::sliding_sum<int> win_tpc_values;
+    srsran::sliding_sum<int>       win_tpc_values;
-    int8_t                   pending_delta      = 0;
+    int                            acc_tpc_values     = 0;
-    int                      acc_tpc_values     = 0;
+    int                            acc_tpc_phr_values = 0;
-    uint32_t                 last_tpc_tti_count = 0;
+    int8_t                         pending_delta      = 0;
+    uint32_t                       last_tpc_tti_count = 0;
+    srsran::rolling_average<float> tpc_snr_avg; // average of SNRs since last TPC != 1
 
     explicit ul_ch_snr_estim(float exp_avg_alpha, int initial_snr) :
       snr_avg(exp_avg_alpha, initial_snr),

srsenb/test/mac/sched_tpc_test.cc

@@ -72,23 +72,40 @@ int test_finite_target_snr()
   }
 
   // TEST: PHR is negative. Checks:
-  // - one TPC should be sent to decrease power. No more TPCs != 0 should be sent until the next PHR
+  // - TPCs sent should be negative or zero
-  snr_diff = -10;
+  // - The accumulation of TPCs should lead to next PHR being zero.
+  snr_diff     = -10;
+  int next_phr = -2;
   tpcfsm.set_snr(target_snr + snr_diff, tpc::PUSCH_CODE);
   tpcfsm.set_snr(target_snr + snr_diff, tpc::PUCCH_CODE);
+  sum_pucch = 0;
   for (uint32_t i = 0; i < 3; ++i) {
-    tpcfsm.set_phr(-2, 1);
+    tpcfsm.set_phr(next_phr, 1);
-    tpcfsm.new_tti();
-    TESTASSERT(decode_tpc(tpcfsm.encode_pusch_tpc()) == -1);
-    TESTASSERT(decode_tpc(tpcfsm.encode_pucch_tpc()) == 3); // PUCCH doesnt get affected by neg PHR
     for (uint32_t j = 0; j < 100; ++j) {
       tpcfsm.new_tti();
-      TESTASSERT(decode_tpc(tpcfsm.encode_pusch_tpc()) == 0);
+      int tpc_pusch = decode_tpc(tpcfsm.encode_pusch_tpc());
+      TESTASSERT(tpc_pusch <= 0);
+      next_phr -= tpc_pusch;
+      sum_pucch += decode_tpc(tpcfsm.encode_pucch_tpc());
     }
+    TESTASSERT(next_phr == 0);
+  }
+  TESTASSERT(sum_pucch == -snr_diff); // PUCCH doesnt get affected by neg PHR
+
+  // TEST: PHR is positive and SINR < target SINR. Checks:
+  // - accumulation of TPCs should not make next PHR negative
+  // - TPCs should be positive or zero
+  next_phr = 5;
+  snr_diff = -10;
+  tpcfsm.set_phr(next_phr, 1);
+  tpcfsm.set_snr(target_snr + snr_diff, tpc::PUSCH_CODE);
+  for (uint32_t j = 0; j < 100; ++j) {
+    tpcfsm.new_tti();
+    int tpc_pusch = decode_tpc(tpcfsm.encode_pusch_tpc());
+    next_phr -= tpc_pusch;
+    TESTASSERT(tpc_pusch >= 0);
   }
-  tpcfsm.set_phr(20, 1);
+  TESTASSERT(next_phr == 0);
-  tpcfsm.new_tti();
-  TESTASSERT(decode_tpc(tpcfsm.encode_pusch_tpc()) == 3);
 
   return SRSRAN_SUCCESS;
 }
@@ -164,11 +181,49 @@ int test_undefined_target_snr()
   return SRSRAN_SUCCESS;
 }
 
+void test_finite_target_snr_tpc_period_above_1()
+{
+  const uint32_t nof_prbs   = 50;
+  const int      target_snr = 15;
+
+  tpc tpcfsm(0x46, nof_prbs, 15, 15, true, 0, 5);
+
+  // TEST: While UL SNR ~ target, no TPC commands are sent
+  for (uint32_t i = 0; i < 100 and tpcfsm.get_ul_snr_estim(0) < 14; ++i) {
+    tpcfsm.set_snr(15, 0);
+    tpcfsm.set_snr(15, 1);
+    tpcfsm.new_tti();
+  }
+  for (uint32_t i = 0; i < 100; ++i) {
+    tpcfsm.new_tti();
+    TESTASSERT(decode_tpc(tpcfsm.encode_pucch_tpc()) == 0);
+    TESTASSERT(decode_tpc(tpcfsm.encode_pusch_tpc()) == 0);
+  }
+
+  // TEST: current SNR above target SNR. Checks:
+  // - TPC commands should be sent to decrease power
+  // - The sum power of TPC commands should not exceed the difference between current and target SNRs
+  int snr_diff = 10;
+  tpcfsm.set_snr(target_snr + snr_diff, tpc::PUSCH_CODE);
+  tpcfsm.set_snr(target_snr + snr_diff, tpc::PUCCH_CODE);
+  int sum_pusch = 0, sum_pucch = 0;
+  for (uint32_t i = 0; i < 100; ++i) {
+    tpcfsm.new_tti();
+    int tpc = decode_tpc(tpcfsm.encode_pusch_tpc());
+    TESTASSERT(tpc <= 0);
+    sum_pusch += tpc;
+    sum_pucch += decode_tpc(tpcfsm.encode_pucch_tpc());
+    TESTASSERT(sum_pucch < 0 and sum_pucch >= -snr_diff);
+  }
+  TESTASSERT(sum_pusch == -snr_diff);
+}
+
 } // namespace srsenb
 
 int main()
 {
   TESTASSERT(srsenb::test_finite_target_snr() == 0);
   TESTASSERT(srsenb::test_undefined_target_snr() == 0);
+  srsenb::test_finite_target_snr_tpc_period_above_1();
   printf("Success\n");
 }