From 8faab9ce58560ef7179b484f402d5dbce460b1c3 Mon Sep 17 00:00:00 2001 From: Milosz Lagan Date: Tue, 19 Nov 2024 17:15:18 +0000 Subject: [PATCH 1/3] Rework moving average logic --- .../husarion_ugv_utils/moving_average.hpp | 20 ++++++++-------- .../test/test_moving_average.cpp | 23 +++++++++++++++++-- 2 files changed, 31 insertions(+), 12 deletions(-) diff --git a/husarion_ugv_utils/include/husarion_ugv_utils/moving_average.hpp b/husarion_ugv_utils/include/husarion_ugv_utils/moving_average.hpp index 0a6c7636..03fe6b07 100644 --- a/husarion_ugv_utils/include/husarion_ugv_utils/moving_average.hpp +++ b/husarion_ugv_utils/include/husarion_ugv_utils/moving_average.hpp @@ -25,40 +25,40 @@ class MovingAverage { public: MovingAverage(const std::size_t window_size = 5, const T initial_value = T(0)) - : window_size_(window_size), initial_value_(initial_value), sum_(T(0)) + : window_size_(window_size), initial_value_(initial_value) { } void Roll(const T value) { values_.push_back(value); - sum_ += value; if (values_.size() > window_size_) { - sum_ -= values_.front(); values_.pop_front(); } } - void Reset() - { - values_.erase(values_.begin(), values_.end()); - sum_ = T(0); - } + void Reset() { values_.erase(values_.begin(), values_.end()); } T GetAverage() const { if (values_.size() == 0) { return initial_value_; } - return sum_ / static_cast(values_.size()); + + T sum = T(0); + + for (const auto & value : values_) { + sum += value / static_cast(values_.size()); + } + + return sum; } private: const std::size_t window_size_; std::deque values_; const T initial_value_; - T sum_; }; } // namespace husarion_ugv_utils diff --git a/husarion_ugv_utils/test/test_moving_average.cpp b/husarion_ugv_utils/test/test_moving_average.cpp index 653483ce..e520bc80 100644 --- a/husarion_ugv_utils/test/test_moving_average.cpp +++ b/husarion_ugv_utils/test/test_moving_average.cpp @@ -61,12 +61,12 @@ TEST(TestMovingAverage, TestHighOverload) double sum = 0.0; for (std::size_t i = 1; i <= window_len * 10; i++) { - sum += double(i); + sum += double(i) / double(window_len); ma.Roll(double(i)); // test every 1000 rolls expected average if (i % window_len == 0) { - EXPECT_EQ(sum / double(window_len), ma.GetAverage()); + EXPECT_LT(sum - ma.GetAverage(), std::numeric_limits::epsilon()); sum = 0.0; } } @@ -107,6 +107,25 @@ TEST(TestMovingAverage, TestResetToInitialValue) EXPECT_EQ(7.0, ma.GetAverage()); } +TEST(TestMovingAverage, TestInfInjectionHandling) +{ + husarion_ugv_utils::MovingAverage ma(4); + ma.Roll(1.0); + ma.Roll(2.0); + ma.Roll(3.0); + ma.Roll(4.0); + EXPECT_EQ(2.5, ma.GetAverage()); + + ma.Roll(std::numeric_limits::infinity()); + EXPECT_EQ(std::numeric_limits::infinity(), ma.GetAverage()); + + ma.Roll(1.0); + ma.Roll(2.0); + ma.Roll(3.0); + ma.Roll(4.0); + EXPECT_EQ(2.5, ma.GetAverage()); +} + int main(int argc, char ** argv) { testing::InitGoogleTest(&argc, argv); From b891a894b1999a8650b9bfdf2f307c726e05d54d Mon Sep 17 00:00:00 2001 From: Milosz Lagan Date: Fri, 22 Nov 2024 09:05:06 +0000 Subject: [PATCH 2/3] Review changes --- .../husarion_ugv_utils/moving_average.hpp | 19 +++++++++++-------- .../test/test_moving_average.cpp | 8 ++++---- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/husarion_ugv_utils/include/husarion_ugv_utils/moving_average.hpp b/husarion_ugv_utils/include/husarion_ugv_utils/moving_average.hpp index 03fe6b07..311f214d 100644 --- a/husarion_ugv_utils/include/husarion_ugv_utils/moving_average.hpp +++ b/husarion_ugv_utils/include/husarion_ugv_utils/moving_average.hpp @@ -27,29 +27,32 @@ class MovingAverage MovingAverage(const std::size_t window_size = 5, const T initial_value = T(0)) : window_size_(window_size), initial_value_(initial_value) { + if (window_size_ == 0) { + throw std::invalid_argument("Window size must be greater than 0"); + } } void Roll(const T value) { - values_.push_back(value); + buffer_.push_back(value); - if (values_.size() > window_size_) { - values_.pop_front(); + if (buffer_.size() > window_size_) { + buffer_.pop_front(); } } - void Reset() { values_.erase(values_.begin(), values_.end()); } + void Reset() { buffer_.erase(buffer_.begin(), buffer_.end()); } T GetAverage() const { - if (values_.size() == 0) { + if (buffer_.size() == 0) { return initial_value_; } T sum = T(0); - for (const auto & value : values_) { - sum += value / static_cast(values_.size()); + for (const auto & value : buffer_) { + sum += value / static_cast(buffer_.size()); } return sum; @@ -57,7 +60,7 @@ class MovingAverage private: const std::size_t window_size_; - std::deque values_; + std::deque buffer_; const T initial_value_; }; diff --git a/husarion_ugv_utils/test/test_moving_average.cpp b/husarion_ugv_utils/test/test_moving_average.cpp index e520bc80..9c759980 100644 --- a/husarion_ugv_utils/test/test_moving_average.cpp +++ b/husarion_ugv_utils/test/test_moving_average.cpp @@ -59,15 +59,15 @@ TEST(TestMovingAverage, TestHighOverload) const std::size_t window_len = 1000; husarion_ugv_utils::MovingAverage ma(window_len); - double sum = 0.0; + double avg = 0.0; for (std::size_t i = 1; i <= window_len * 10; i++) { - sum += double(i) / double(window_len); + avg += double(i) / double(window_len); ma.Roll(double(i)); // test every 1000 rolls expected average if (i % window_len == 0) { - EXPECT_LT(sum - ma.GetAverage(), std::numeric_limits::epsilon()); - sum = 0.0; + ASSERT_NEAR(avg, ma.GetAverage(), std::numeric_limits::epsilon()); + avg = 0.0; } } } From 72554b836e2d914a25ea8d6fc5e9b83dbb4e72b7 Mon Sep 17 00:00:00 2001 From: Milosz Lagan Date: Fri, 22 Nov 2024 10:43:13 +0000 Subject: [PATCH 3/3] Use normal accumulation and division for MA calculation --- .../include/husarion_ugv_utils/moving_average.hpp | 10 ++++------ husarion_ugv_utils/test/test_moving_average.cpp | 9 +++++---- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/husarion_ugv_utils/include/husarion_ugv_utils/moving_average.hpp b/husarion_ugv_utils/include/husarion_ugv_utils/moving_average.hpp index 311f214d..fc1d6b4f 100644 --- a/husarion_ugv_utils/include/husarion_ugv_utils/moving_average.hpp +++ b/husarion_ugv_utils/include/husarion_ugv_utils/moving_average.hpp @@ -16,6 +16,7 @@ #define HUSARION_UGV_UTILS_MOVING_AVERAGE_HPP_ #include +#include namespace husarion_ugv_utils { @@ -49,13 +50,10 @@ class MovingAverage return initial_value_; } - T sum = T(0); + T sum = std::accumulate(buffer_.begin(), buffer_.end(), T(0)); + T average = sum / static_cast(buffer_.size()); - for (const auto & value : buffer_) { - sum += value / static_cast(buffer_.size()); - } - - return sum; + return average; } private: diff --git a/husarion_ugv_utils/test/test_moving_average.cpp b/husarion_ugv_utils/test/test_moving_average.cpp index 9c759980..2934f28e 100644 --- a/husarion_ugv_utils/test/test_moving_average.cpp +++ b/husarion_ugv_utils/test/test_moving_average.cpp @@ -59,15 +59,16 @@ TEST(TestMovingAverage, TestHighOverload) const std::size_t window_len = 1000; husarion_ugv_utils::MovingAverage ma(window_len); - double avg = 0.0; + double sum = 0.0; for (std::size_t i = 1; i <= window_len * 10; i++) { - avg += double(i) / double(window_len); + sum += double(i); ma.Roll(double(i)); // test every 1000 rolls expected average if (i % window_len == 0) { - ASSERT_NEAR(avg, ma.GetAverage(), std::numeric_limits::epsilon()); - avg = 0.0; + ASSERT_NEAR( + sum / double(window_len), ma.GetAverage(), std::numeric_limits::epsilon()); + sum = 0.0; } } }