Skip to content

Commit e08bcb6

Browse files
Drop Averager from FormulaEngine
Signed-off-by: Christian Parpart <[email protected]>
1 parent 1e4c9ea commit e08bcb6

File tree

6 files changed

+4
-158
lines changed

6 files changed

+4
-158
lines changed

RELEASE_NOTES.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@
3232

3333
- The CI now runs cross-arch tests on `arm64` architectures.
3434

35+
- Drop `Averager` from `FormulaEngine`.
36+
3537
## Bug Fixes
3638

3739
<!-- Here goes notable bug fixes that are worth a special mention or explanation -->

src/frequenz/sdk/timeseries/_formula_engine/_formula_engine.py

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
from ._formula_formatter import format_formula
2424
from ._formula_steps import (
2525
Adder,
26-
Averager,
2726
Clipper,
2827
ConstantValue,
2928
Divider,
@@ -589,23 +588,6 @@ def push_clipper(self, min_value: float | None, max_value: float | None) -> None
589588
"""
590589
self._steps.append(Clipper(min_value, max_value))
591590

592-
def push_average(
593-
self, metrics: list[tuple[str, Receiver[Sample[QuantityT]], bool]]
594-
) -> None:
595-
"""Push an average calculator into the engine.
596-
597-
Args:
598-
metrics: list of arguments to pass to each `MetricFetcher`.
599-
"""
600-
fetchers: list[MetricFetcher[QuantityT]] = []
601-
for metric in metrics:
602-
fetcher = self._metric_fetchers.setdefault(
603-
metric[0],
604-
MetricFetcher(metric[0], metric[1], nones_are_zeros=metric[2]),
605-
)
606-
fetchers.append(fetcher)
607-
self._steps.append(Averager(fetchers))
608-
609591
@property
610592
def name(self) -> str:
611593
"""Return the name of the formula being built.

src/frequenz/sdk/timeseries/_formula_engine/_formula_formatter.py

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99

1010
from ._formula_steps import (
1111
Adder,
12-
Averager,
1312
Clipper,
1413
ConstantValue,
1514
Divider,
@@ -200,12 +199,6 @@ def format(self, postfix_expr: list[FormulaStep]) -> str:
200199
self._format_binary(Operator.MULTIPLICATION)
201200
case Divider():
202201
self._format_binary(Operator.DIVISION)
203-
case Averager():
204-
value = (
205-
# pylint: disable=protected-access
206-
f"avg({', '.join(self.format([f]) for f in step.fetchers)})"
207-
)
208-
self._stack.append(StackItem(value, OperatorPrecedence.PRIMARY, 1))
209202
case Clipper():
210203
the_value = self._stack.pop()
211204
min_value = step.min_value if step.min_value is not None else "-inf"

src/frequenz/sdk/timeseries/_formula_engine/_formula_steps.py

Lines changed: 1 addition & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,12 @@
77

88
import math
99
from abc import ABC, abstractmethod
10-
from typing import Generic, Sequence
10+
from typing import Generic
1111

1212
from frequenz.channels import Receiver
1313

1414
from .. import Sample
1515
from .._quantities import QuantityT
16-
from ._exceptions import FormulaEngineError
1716

1817

1918
class FormulaStep(ABC):
@@ -196,63 +195,6 @@ def apply(self, _: list[float]) -> None:
196195
"""No-op."""
197196

198197

199-
class Averager(Generic[QuantityT], FormulaStep):
200-
"""A formula step for calculating average."""
201-
202-
def __init__(self, fetchers: list[MetricFetcher[QuantityT]]) -> None:
203-
"""Create an `Averager` instance.
204-
205-
Args:
206-
fetchers: MetricFetchers for the metrics that need to be averaged.
207-
"""
208-
self._fetchers: list[MetricFetcher[QuantityT]] = fetchers
209-
210-
@property
211-
def fetchers(self) -> Sequence[MetricFetcher[QuantityT]]:
212-
"""Return the metric fetchers.
213-
214-
Returns:
215-
The metric fetchers.
216-
"""
217-
return self._fetchers
218-
219-
def __repr__(self) -> str:
220-
"""Return a string representation of the step.
221-
222-
Returns:
223-
A string representation of the step.
224-
"""
225-
return f"avg({', '.join(repr(f) for f in self._fetchers)})"
226-
227-
def apply(self, eval_stack: list[float]) -> None:
228-
"""Calculate average of given metrics, push the average to the eval_stack.
229-
230-
Args:
231-
eval_stack: An evaluation stack, to append the calculated average to.
232-
233-
Raises:
234-
FormulaEngineError: when metric fetchers are unable to fetch values.
235-
"""
236-
value_count = 0
237-
total = 0.0
238-
for fetcher in self._fetchers:
239-
next_val = fetcher.value
240-
if next_val is None:
241-
raise FormulaEngineError(
242-
"Unable to fetch a value from the resampling actor."
243-
)
244-
if next_val.value is None:
245-
continue
246-
value_count += 1
247-
total += next_val.value.base_value
248-
if value_count == 0:
249-
avg = 0.0
250-
else:
251-
avg = total / value_count
252-
253-
eval_stack.append(avg)
254-
255-
256198
class ConstantValue(FormulaStep):
257199
"""A formula step for inserting a constant value."""
258200

tests/timeseries/test_formula_engine.py

Lines changed: 0 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -571,73 +571,6 @@ async def test_nones_are_not_zeros(self) -> None:
571571
)
572572

573573

574-
class TestFormulaAverager:
575-
"""Tests for the formula step for calculating average."""
576-
577-
async def run_test(
578-
self,
579-
components: list[str],
580-
io_pairs: list[tuple[list[float | None], float | None]],
581-
) -> None:
582-
"""Run a formula test."""
583-
channels: dict[str, Broadcast[Sample[Quantity]]] = {}
584-
streams: list[tuple[str, Receiver[Sample[Quantity]], bool]] = []
585-
builder = FormulaBuilder("test_averager", create_method=Quantity)
586-
for comp_id in components:
587-
if comp_id not in channels:
588-
channels[comp_id] = Broadcast(comp_id)
589-
streams.append((f"{comp_id}", channels[comp_id].new_receiver(), False))
590-
591-
builder.push_average(streams)
592-
engine = builder.build()
593-
results_rx = engine.new_receiver()
594-
now = datetime.now()
595-
tests_passed = 0
596-
for io_pair in io_pairs:
597-
io_input, io_output = io_pair
598-
await asyncio.gather(
599-
*[
600-
chan.new_sender().send(
601-
Sample(now, None if not value else Quantity(value))
602-
)
603-
for chan, value in zip(channels.values(), io_input)
604-
]
605-
)
606-
next_val = await results_rx.receive()
607-
if io_output is None:
608-
assert next_val.value is None
609-
else:
610-
assert next_val.value and next_val.value.base_value == io_output
611-
tests_passed += 1
612-
await engine._stop() # pylint: disable=protected-access
613-
assert tests_passed == len(io_pairs)
614-
615-
async def test_simple(self) -> None:
616-
"""Test simple formulas."""
617-
await self.run_test(
618-
["#2", "#4", "#5"],
619-
[
620-
([10.0, 12.0, 14.0], 12.0),
621-
([15.0, 17.0, 19.0], 17.0),
622-
([11.1, 11.1, 11.1], 11.1),
623-
],
624-
)
625-
626-
async def test_nones_are_skipped(self) -> None:
627-
"""Test that `None`s are skipped for computing the average."""
628-
await self.run_test(
629-
["#2", "#4", "#5"],
630-
[
631-
([11.0, 13.0, 15.0], 13.0),
632-
([None, 13.0, 19.0], 16.0),
633-
([12.2, None, 22.2], 17.2),
634-
([16.5, 19.5, None], 18.0),
635-
([None, 13.0, None], 13.0),
636-
([None, None, None], 0.0),
637-
],
638-
)
639-
640-
641574
class TestConstantValue:
642575
"""Tests for the constant value step."""
643576

tests/timeseries/test_formula_formatter.py

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,15 +12,14 @@
1212
from frequenz.sdk.timeseries._formula_engine._formula_engine import FormulaBuilder
1313
from frequenz.sdk.timeseries._formula_engine._formula_formatter import format_formula
1414
from frequenz.sdk.timeseries._formula_engine._formula_steps import (
15-
Averager,
1615
Clipper,
1716
ConstantValue,
1817
FormulaStep,
1918
Maximizer,
2019
Minimizer,
2120
)
2221
from frequenz.sdk.timeseries._formula_engine._tokenizer import Tokenizer, TokenType
23-
from frequenz.sdk.timeseries._quantities import Percentage, Quantity
22+
from frequenz.sdk.timeseries._quantities import Quantity
2423
from tests.timeseries.mock_microgrid import MockMicrogrid
2524

2625

@@ -111,11 +110,6 @@ def test_functions(self) -> None:
111110
# flake8: enable
112111
# fmt: on
113112

114-
def test_function_avg(self) -> None:
115-
"""Test that the avg function is formatted correctly."""
116-
# This tests the special case of the avg function with no arguments.
117-
assert format_formula([Averager[Percentage]([])]) == "avg()"
118-
119113
async def test_higher_order_formula(self, mocker: MockerFixture) -> None:
120114
"""Test that the formula is formatted correctly for a higher-order formula."""
121115
mockgrid = MockMicrogrid(grid_meter=False)

0 commit comments

Comments
 (0)