Skip to content

Commit add3ff5

Browse files
committed
RingBuffer: Fix off-by-one error in gap calculation
Signed-off-by: Mathias L. Baumann <[email protected]>
1 parent 6c8a794 commit add3ff5

File tree

3 files changed

+24
-5
lines changed

3 files changed

+24
-5
lines changed

src/frequenz/sdk/timeseries/_ringbuffer/buffer.py

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ def __init__(
8282
self._gaps: list[Gap] = []
8383
self._datetime_newest: datetime = self._DATETIME_MIN
8484
self._datetime_oldest: datetime = self._DATETIME_MAX
85-
self._time_range: timedelta = (len(self._buffer) - 1) * sampling_period
85+
self._full_time_range: timedelta = len(self._buffer) * self._sampling_period
8686

8787
@property
8888
def sampling_period(self) -> timedelta:
@@ -145,7 +145,9 @@ def update(self, sample: Sample[QuantityT]) -> None:
145145
# Update timestamps
146146
prev_newest = self._datetime_newest
147147
self._datetime_newest = max(self._datetime_newest, timestamp)
148-
self._datetime_oldest = self._datetime_newest - self._time_range
148+
self._datetime_oldest = self._datetime_newest - (
149+
self._full_time_range - self._sampling_period
150+
)
149151

150152
# Update data
151153
value: float = np.nan if sample.value is None else sample.value.base_value
@@ -293,7 +295,7 @@ def _update_gaps(
293295

294296
if not new_missing:
295297
# Replace all gaps with one if we went far into then future
296-
if self._datetime_newest - newest >= self._time_range:
298+
if self._datetime_newest - newest >= self._full_time_range:
297299
self._gaps = [
298300
Gap(start=self._datetime_oldest, end=self._datetime_newest)
299301
]

tests/timeseries/test_ringbuffer.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -335,3 +335,22 @@ def test_ringbuffer_empty_buffer() -> None:
335335
sampling_period=timedelta(seconds=1),
336336
align_to=datetime(1, 1, 1),
337337
)
338+
339+
340+
def test_off_by_one_gap_logic_bug() -> None:
341+
"""Test off by one bug in the gap calculation."""
342+
buffer = OrderedRingBuffer(
343+
np.empty(shape=2, dtype=float),
344+
sampling_period=timedelta(seconds=1),
345+
align_to=datetime(1, 1, 1, tzinfo=timezone.utc),
346+
)
347+
348+
base_time = datetime(2023, 1, 1, tzinfo=timezone.utc)
349+
350+
times = [base_time, base_time + timedelta(seconds=1)]
351+
352+
buffer.update(Sample(times[0], Quantity(1.0)))
353+
buffer.update(Sample(times[1], Quantity(2.0)))
354+
355+
assert buffer.is_missing(times[0]) is False
356+
assert buffer.is_missing(times[1]) is False

tests/timeseries/test_ringbuffer_serialization.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,6 @@ def load_dump_test(dumped: rb.OrderedRingBuffer[Any], path: str) -> None:
6161
# pylint: disable=protected-access
6262
assert dumped._gaps == loaded._gaps
6363
# pylint: disable=protected-access
64-
assert dumped._time_range == loaded._time_range
65-
# pylint: disable=protected-access
6664
assert dumped._sampling_period == loaded._sampling_period
6765
# pylint: disable=protected-access
6866
assert dumped._time_index_alignment == loaded._time_index_alignment

0 commit comments

Comments
 (0)