From 8bc19bc7cbe3385742d845c98a3542effa05284b Mon Sep 17 00:00:00 2001 From: Matthias Wende Date: Thu, 10 Aug 2023 11:19:31 +0200 Subject: [PATCH 1/4] Fix updating gaps code When cleaning up gaps the case that a gaps start date can become older than the oldest stored value in the buffer wasn't covered. Furthermore the cases for cleaning up a single gap didn't run for the last gaps, since the loop was stopping too early. Signed-off-by: Matthias Wende --- src/frequenz/sdk/timeseries/_ringbuffer/buffer.py | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/frequenz/sdk/timeseries/_ringbuffer/buffer.py b/src/frequenz/sdk/timeseries/_ringbuffer/buffer.py index 5a794d309..a1616d616 100644 --- a/src/frequenz/sdk/timeseries/_ringbuffer/buffer.py +++ b/src/frequenz/sdk/timeseries/_ringbuffer/buffer.py @@ -328,17 +328,24 @@ def _cleanup_gaps(self) -> None: self._gaps = sorted(self._gaps, key=lambda x: x.start.timestamp()) i = 0 - while i < len(self._gaps) - 1: + while i < len(self._gaps): w_1 = self._gaps[i] - w_2 = self._gaps[i + 1] + if i < len(self._gaps) - 1: + w_2 = self._gaps[i + 1] + else: + w_2 = None + # Delete out-of-date gaps if w_1.end <= self._datetime_oldest: del self._gaps[i] + # Update start of gap if it's rolled out of the buffer + elif w_1.start < self._datetime_oldest: + self._gaps[i].start = self._datetime_oldest # If w2 is a subset of w1 we can delete it - elif w_1.start <= w_2.start and w_1.end >= w_2.end: + elif w_2 and w_1.start <= w_2.start and w_1.end >= w_2.end: del self._gaps[i + 1] # If the gaps are direct neighbors, merge them - elif w_1.end >= w_2.start: + elif w_2 and w_1.end >= w_2.start: w_1.end = w_2.end del self._gaps[i + 1] else: From 79c373f455dd3defd954ecb993035df32b7a538f Mon Sep 17 00:00:00 2001 From: Matthias Wende Date: Thu, 10 Aug 2023 11:21:53 +0200 Subject: [PATCH 2/4] Add test for gap update code in fix_ringbuffer Adds a test case for updating the start time of gaps when it's older than the oldest stored element in the buffer. Signed-off-by: Matthias Wende --- tests/timeseries/test_ringbuffer.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/timeseries/test_ringbuffer.py b/tests/timeseries/test_ringbuffer.py index 958a62cea..d47ed18d5 100644 --- a/tests/timeseries/test_ringbuffer.py +++ b/tests/timeseries/test_ringbuffer.py @@ -369,3 +369,24 @@ def test_off_by_one_gap_logic_bug() -> None: assert buffer.is_missing(times[0]) is False assert buffer.is_missing(times[1]) is False + + +def test_cleanup_oldest_gap_timestamp() -> None: + """Test that gaps are updated such that they are fully contained in the buffer.""" + buffer = OrderedRingBuffer( + np.empty(shape=15, dtype=float), + sampling_period=timedelta(seconds=1), + align_to=datetime(1, 1, 1, tzinfo=timezone.utc), + ) + + for i in range(10): + buffer.update( + Sample(datetime.fromtimestamp(200 + i, tz=timezone.utc), Quantity(i)) + ) + + gap = Gap( + datetime.fromtimestamp(195, tz=timezone.utc), + datetime.fromtimestamp(200, tz=timezone.utc), + ) + + assert gap == buffer.gaps[0] From 38ac7fb7e85dad97d20733f6acec6e38cbfeec1f Mon Sep 17 00:00:00 2001 From: Matthias Wende Date: Thu, 10 Aug 2023 12:51:16 +0200 Subject: [PATCH 3/4] Add test to remove single gap if outdated Signed-off-by: Matthias Wende --- tests/timeseries/test_ringbuffer.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/timeseries/test_ringbuffer.py b/tests/timeseries/test_ringbuffer.py index d47ed18d5..f1b4e6708 100644 --- a/tests/timeseries/test_ringbuffer.py +++ b/tests/timeseries/test_ringbuffer.py @@ -390,3 +390,25 @@ def test_cleanup_oldest_gap_timestamp() -> None: ) assert gap == buffer.gaps[0] + + +def test_delete_oudated_gap() -> None: + """ + Update the buffer such that the gap is no longer valid. + We introduce two gaps and check that the oldest is removed. + """ + buffer = OrderedRingBuffer( + np.empty(shape=3, dtype=float), + sampling_period=timedelta(seconds=1), + align_to=datetime(1, 1, 1, tzinfo=timezone.utc), + ) + + for i in range(2): + buffer.update( + Sample(datetime.fromtimestamp(200 + i, tz=timezone.utc), Quantity(i)) + ) + assert len(buffer.gaps) == 1 + + buffer.update(Sample(datetime.fromtimestamp(202, tz=timezone.utc), Quantity(2))) + + assert len(buffer.gaps) == 0 From d8eaaebd1f50e11c72b3df4db3c3721fcf2572fc Mon Sep 17 00:00:00 2001 From: Matthias Wende Date: Thu, 10 Aug 2023 11:27:28 +0200 Subject: [PATCH 4/4] Update release notes Signed-off-by: Matthias Wende --- RELEASE_NOTES.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index de5831070..e17617d7f 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -16,3 +16,7 @@ Now the microgrid API v0.15.x is being used. The SDK can only connect to microgr - The `ConfigManagingActor` constructor now can accept a `pathlib.Path` as `config_path` too (before it accepted only a `str`). - The `PowerDistributingActor` now considers exclusion bounds, when finding an optimal distribution for power between batteries. + +## Bug Fixes + +- Fixes a bug in the ring buffer updating the end timestamp of gaps when they are outdated.