Skip to content

Commit

Permalink
chore: Cosmetic fixes in MovHistogram
Browse files Browse the repository at this point in the history
  • Loading branch information
@gavv
gavv committed Aug 1, 2024
1 parent b102bb7 commit 2314fa4
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 92 deletions.
36 changes: 19 additions & 17 deletions src/internal_modules/roc_core/mov_histogram.h
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2023 Roc Streaming authors
* Copyright (c) 2024 Roc Streaming authors
*
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
Expand All @@ -20,23 +20,24 @@
namespace roc {
namespace core {

//! @brief A class that implements a rolling window moving histogram.
//! Rolling window histogram.
//!
//! The MovHistogram class maintains a histogram of values within a specified window
//! length. It divides the range of values into a specified number of bins and updates the
//! histogram as new values are added and old values are removed from the window.
//!
//! @tparam T The type of values to be histogrammed.

template <typename T> class MovHistogram {
public:
//! @brief Constructs a moving histogram.
//! Constructs a moving histogram.
//! @param arena Memory arena for dynamic allocations.
//! @param value_range_min The minimum value of the range to be histogrammed.
//! @param value_range_max The maximum value of the range to be histogrammed.
//! (values outside of the range are clamped to the range boundaries).
//! @param num_bins The number of bins in the histogram. Each bin represents a
//! subrange of the value range.
//! @param window_length The length of the moving window. Only values within this
//! window are considered in the histogram.

MovHistogram(IArena& arena,
T value_range_min,
T value_range_max,
Expand All @@ -49,12 +50,13 @@ template <typename T> class MovHistogram {
, ring_buffer_(arena, window_length)
, bins_(arena)
, valid_(false) {
if (num_bins == 0 || window_length == 0 || value_range_min >= value_range_max) {
roc_panic("mov histogram: number of bins and window length must be greater "
"than 0 and value_range_min must be less than value_range_max");
}
roc_panic_if_msg(window_length == 0, "mov histogram: window_length must be > 0");
roc_panic_if_msg(num_bins == 0, "mov histogram: num_bins must be > 0");
roc_panic_if_msg(
value_range_min >= value_range_max,
"mov histogram: value_range_min must be less than value_range_max");

bin_width_ = (value_range_max - value_range_min) / static_cast<T>(num_bins);
bin_width_ = (value_range_max - value_range_min) / T(num_bins);

if (!ring_buffer_.is_valid() || !bins_.resize(num_bins)) {
return;
Expand All @@ -68,8 +70,13 @@ template <typename T> class MovHistogram {
return valid_;
}

//! Get the number of values in the given bin.
size_t mov_counter(size_t bin_index) const {
return bins_[bin_index];
}

//! Add a value to the histogram.
void add_value(const T& value) {
void add(const T& value) {
T clamped_value = value;

if (clamped_value < value_range_min_) {
Expand All @@ -92,19 +99,14 @@ template <typename T> class MovHistogram {
}
}

//! Get the number of values in the given bin.
size_t get_bin_counter(size_t bin_index) const {
return bins_[bin_index];
}

private:
//! Get the bin index for the given value.
size_t get_bin_index_(const T& value) const {
if (value == value_range_max_) {
return num_bins_ - 1;
}

return static_cast<size_t>((value - value_range_min_) / bin_width_);
return size_t((value - value_range_min_) / bin_width_);
}

T value_range_min_;
Expand Down
122 changes: 47 additions & 75 deletions src/tests/roc_core/test_mov_histogram.cpp
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2023 Roc Streaming authors
* Copyright (c) 2024 Roc Streaming authors
*
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
Expand All @@ -14,39 +14,11 @@
namespace roc {
namespace core {

namespace {

enum { NumObjects = 10, EmbeddedCap = 5 };

struct Object {
static long n_objects;

size_t value;

Object(size_t v = 0)
: value(v) {
n_objects++;
}

Object(const Object& other)
: value(other.value) {
n_objects++;
}

~Object() {
n_objects--;
}
};

long Object::n_objects = 0;

} // namespace

TEST_GROUP(movhistogram) {
TEST_GROUP(mov_histogram) {
HeapArena arena;
};

TEST(movhistogram, single_pass) {
TEST(mov_histogram, single_pass) {
const size_t value_range_min = 0;
const size_t value_range_max = 100;
const size_t num_bins = 10;
Expand All @@ -57,15 +29,15 @@ TEST(movhistogram, single_pass) {
CHECK(hist.is_valid());

for (size_t i = 0; i < win_length; i++) {
hist.add_value(i * num_bins);
hist.add(i * num_bins);
}

for (size_t i = 0; i < num_bins; ++i) {
LONGS_EQUAL(1, hist.get_bin_counter(i));
LONGS_EQUAL(1, hist.mov_counter(i));
}
}

TEST(movhistogram, rolling_window) {
TEST(mov_histogram, rolling_window) {
const size_t value_range_min = 0;
const size_t value_range_max = 100;
const size_t num_bins = 10;
Expand All @@ -76,15 +48,15 @@ TEST(movhistogram, rolling_window) {
CHECK(hist.is_valid());

for (size_t i = 0; i < win_length * 2; i++) {
hist.add_value(i * (value_range_max / num_bins));
hist.add(i * (value_range_max / num_bins));
}

for (size_t i = 0; i < num_bins; ++i) {
LONGS_EQUAL(i < win_length ? 0 : 1, hist.get_bin_counter(i));
LONGS_EQUAL(i < win_length ? 0 : 1, hist.mov_counter(i));
}
}

TEST(movhistogram, value_equal_to_value_range_max) {
TEST(mov_histogram, value_equal_to_value_range_max) {
const size_t value_range_min = 0;
const size_t value_range_max = 100;
const size_t num_bins = 10;
Expand All @@ -95,13 +67,13 @@ TEST(movhistogram, value_equal_to_value_range_max) {
CHECK(hist.is_valid());

size_t test_value = value_range_max;
hist.add_value(test_value);
hist.add(test_value);

size_t expected_bin_index = num_bins - 1;
LONGS_EQUAL(1, hist.get_bin_counter(expected_bin_index));
LONGS_EQUAL(1, hist.mov_counter(expected_bin_index));
}

TEST(movhistogram, value_is_float) {
TEST(mov_histogram, value_is_float) {
const float value_range_min = 0.0f;
const float value_range_max = 100.0f;
const size_t num_bins = 10;
Expand All @@ -112,15 +84,15 @@ TEST(movhistogram, value_is_float) {
CHECK(hist.is_valid());

for (size_t i = 0; i < win_length; i++) {
hist.add_value(i * num_bins * 1.0f);
hist.add(i * num_bins * 1.0f);
}

for (size_t i = 0; i < num_bins; ++i) {
LONGS_EQUAL(1, hist.get_bin_counter(i));
LONGS_EQUAL(1, hist.mov_counter(i));
}
}

TEST(movhistogram, min_max_negative) {
TEST(mov_histogram, min_max_negative) {
const int value_range_min = -150;
const int value_range_max = -50;
const size_t num_bins = 10;
Expand All @@ -133,15 +105,15 @@ TEST(movhistogram, min_max_negative) {

for (size_t i = 0; i < win_length; i++) {
int value = value_range_min + (int)i * bin_width;
hist.add_value(value);
hist.add(value);
}

for (size_t i = 0; i < num_bins; ++i) {
LONGS_EQUAL(1, hist.get_bin_counter(i));
LONGS_EQUAL(1, hist.mov_counter(i));
}
}

TEST(movhistogram, win_length_equal_one) {
TEST(mov_histogram, win_length_equal_one) {
const size_t value_range_min = 0;
const size_t value_range_max = 100;
const size_t num_bins = 10;
Expand All @@ -151,16 +123,16 @@ TEST(movhistogram, win_length_equal_one) {
win_length);
CHECK(hist.is_valid());

hist.add_value(0);
hist.add_value(10);
hist.add_value(20);
hist.add(0);
hist.add(10);
hist.add(20);

LONGS_EQUAL(0, hist.get_bin_counter(0));
LONGS_EQUAL(0, hist.get_bin_counter(1));
LONGS_EQUAL(1, hist.get_bin_counter(2));
LONGS_EQUAL(0, hist.mov_counter(0));
LONGS_EQUAL(0, hist.mov_counter(1));
LONGS_EQUAL(1, hist.mov_counter(2));
}

TEST(movhistogram, multiple_values_in_bins) {
TEST(mov_histogram, multiple_values_in_bins) {
const size_t value_range_min = 0;
const size_t value_range_max = 100;
const size_t num_bins = 10;
Expand All @@ -175,15 +147,15 @@ TEST(movhistogram, multiple_values_in_bins) {

for (size_t i = 0; i < total_values; ++i) {
size_t value = (i / values_per_bin) * (value_range_max / num_bins);
hist.add_value(value);
hist.add(value);
}

for (size_t i = 0; i < num_bins; ++i) {
LONGS_EQUAL(values_per_bin, hist.get_bin_counter(i));
LONGS_EQUAL(values_per_bin, hist.mov_counter(i));
}
}

TEST(movhistogram, rolling_window_bin_changes) {
TEST(mov_histogram, rolling_window_bin_changes) {
const size_t value_range_min = 0;
const size_t value_range_max = 100;
const size_t num_bins = 10;
Expand All @@ -194,27 +166,27 @@ TEST(movhistogram, rolling_window_bin_changes) {
CHECK(hist.is_valid());

for (size_t i = 0; i < win_length; i++) {
hist.add_value(i * (value_range_max / num_bins));
hist.add(i * (value_range_max / num_bins));
}

for (size_t i = 0; i < num_bins; i++) {
LONGS_EQUAL(i < win_length ? 1 : 0, hist.get_bin_counter(i));
LONGS_EQUAL(i < win_length ? 1 : 0, hist.mov_counter(i));
}

hist.add_value(win_length * (value_range_max / num_bins));
hist.add(win_length * (value_range_max / num_bins));

for (size_t i = 0; i < num_bins; i++) {
if (i < 1) {
LONGS_EQUAL(0, hist.get_bin_counter(i));
LONGS_EQUAL(0, hist.mov_counter(i));
} else if (i <= win_length) {
LONGS_EQUAL(1, hist.get_bin_counter(i));
LONGS_EQUAL(1, hist.mov_counter(i));
} else {
LONGS_EQUAL(0, hist.get_bin_counter(i));
LONGS_EQUAL(0, hist.mov_counter(i));
}
}
}

TEST(movhistogram, clamping_values) {
TEST(mov_histogram, clamping_values) {
const size_t value_range_min = 50;
const size_t value_range_max = 150;
const size_t num_bins = 10;
Expand All @@ -224,21 +196,21 @@ TEST(movhistogram, clamping_values) {
win_length);
CHECK(hist.is_valid());

hist.add_value(static_cast<size_t>(20));
hist.add_value(static_cast<size_t>(5));
hist.add_value(static_cast<size_t>(10));
hist.add(size_t(20));
hist.add(size_t(5));
hist.add(size_t(10));

hist.add_value(static_cast<size_t>(60));
hist.add_value(static_cast<size_t>(80));
hist.add(size_t(60));
hist.add(size_t(80));

hist.add_value(static_cast<size_t>(160));
hist.add_value(static_cast<size_t>(170));
hist.add_value(static_cast<size_t>(180));
hist.add(size_t(160));
hist.add(size_t(170));
hist.add(size_t(180));

LONGS_EQUAL(3, hist.get_bin_counter(0));
LONGS_EQUAL(1, hist.get_bin_counter(1));
LONGS_EQUAL(1, hist.get_bin_counter(3));
LONGS_EQUAL(3, hist.get_bin_counter(9));
LONGS_EQUAL(3, hist.mov_counter(0));
LONGS_EQUAL(1, hist.mov_counter(1));
LONGS_EQUAL(1, hist.mov_counter(3));
LONGS_EQUAL(3, hist.mov_counter(9));
}

} // namespace core
Expand Down

0 comments on commit 2314fa4

Please sign in to comment.