From 5afc64def3683abff15ac82c760b01018e316333 Mon Sep 17 00:00:00 2001 From: Christian Parpart Date: Fri, 7 Jul 2023 10:27:30 +0200 Subject: [PATCH 01/46] Add `Temperature` quantity Signed-off-by: Christian Parpart --- src/frequenz/sdk/timeseries/__init__.py | 11 +++++- src/frequenz/sdk/timeseries/_quantities.py | 41 +++++++++++++++++++++- 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/src/frequenz/sdk/timeseries/__init__.py b/src/frequenz/sdk/timeseries/__init__.py index b1fcdd90e..515d06366 100644 --- a/src/frequenz/sdk/timeseries/__init__.py +++ b/src/frequenz/sdk/timeseries/__init__.py @@ -38,7 +38,15 @@ from ._base_types import UNIX_EPOCH, Sample, Sample3Phase from ._moving_window import MovingWindow from ._periodic_feature_extractor import PeriodicFeatureExtractor -from ._quantities import Current, Energy, Percentage, Power, Quantity, Voltage +from ._quantities import ( + Current, + Energy, + Percentage, + Power, + Quantity, + Temperature, + Voltage, +) from ._resampling import ResamplerConfig __all__ = [ @@ -55,6 +63,7 @@ "Current", "Energy", "Power", + "Temperature", "Voltage", "Percentage", ] diff --git a/src/frequenz/sdk/timeseries/_quantities.py b/src/frequenz/sdk/timeseries/_quantities.py index 0b2b99eca..654495cc8 100644 --- a/src/frequenz/sdk/timeseries/_quantities.py +++ b/src/frequenz/sdk/timeseries/_quantities.py @@ -10,7 +10,14 @@ from typing import Any, NoReturn, Self, TypeVar, overload QuantityT = TypeVar( - "QuantityT", "Quantity", "Power", "Current", "Voltage", "Energy", "Percentage" + "QuantityT", + "Current", + "Energy", + "Percentage", + "Power", + "Quantity", + "Temperature", + "Voltage", ) @@ -301,6 +308,38 @@ def __call__(cls, *_args: Any, **_kwargs: Any) -> NoReturn: ) +class Temperature( + Quantity, + metaclass=_NoDefaultConstructible, + exponent_unit_map={ + 0: "°C", + }, +): + """A temperature quantity (in degrees Celsius).""" + + @classmethod + def from_celsius(cls, value: float) -> Self: + """Initialize a new temperature quantity. + + Args: + value: The temperature in degrees Celsius. + + Returns: + A new temperature quantity. + """ + power = cls.__new__(cls) + power._base_value = value + return power + + def as_celsius(self) -> float: + """Return the temperature in degrees Celsius. + + Returns: + The temperature in degrees Celsius. + """ + return self._base_value + + class Power( Quantity, metaclass=_NoDefaultConstructible, From 0e5efeddb43e1ca930a11253655a46a86f102e9f Mon Sep 17 00:00:00 2001 From: Christian Parpart Date: Fri, 7 Jul 2023 10:29:09 +0200 Subject: [PATCH 02/46] Add ComponentMetricId.TEMPERATURE_MAX and the ability to retrieve this metric Signed-off-by: Christian Parpart --- src/frequenz/sdk/actor/_data_sourcing/microgrid_api_source.py | 1 + src/frequenz/sdk/microgrid/component/_component.py | 2 ++ 2 files changed, 3 insertions(+) diff --git a/src/frequenz/sdk/actor/_data_sourcing/microgrid_api_source.py b/src/frequenz/sdk/actor/_data_sourcing/microgrid_api_source.py index b1aacc5f4..5e131a623 100644 --- a/src/frequenz/sdk/actor/_data_sourcing/microgrid_api_source.py +++ b/src/frequenz/sdk/actor/_data_sourcing/microgrid_api_source.py @@ -84,6 +84,7 @@ def get_channel_name(self) -> str: ComponentMetricId.CAPACITY: lambda msg: msg.capacity, ComponentMetricId.POWER_LOWER_BOUND: lambda msg: msg.power_lower_bound, ComponentMetricId.POWER_UPPER_BOUND: lambda msg: msg.power_upper_bound, + ComponentMetricId.TEMPERATURE_MAX: lambda msg: msg.temperature_max, } _InverterDataMethods: Dict[ComponentMetricId, Callable[[InverterData], float]] = { diff --git a/src/frequenz/sdk/microgrid/component/_component.py b/src/frequenz/sdk/microgrid/component/_component.py index 131f25010..da4a3d1e8 100644 --- a/src/frequenz/sdk/microgrid/component/_component.py +++ b/src/frequenz/sdk/microgrid/component/_component.py @@ -139,3 +139,5 @@ class ComponentMetricId(Enum): ACTIVE_POWER_LOWER_BOUND = "active_power_lower_bound" ACTIVE_POWER_UPPER_BOUND = "active_power_upper_bound" + + TEMPERATURE_MAX = "temperature_max" From 09932bdbfb4bce141abf40aecf3fe8e0403e6779 Mon Sep 17 00:00:00 2001 From: Christian Parpart Date: Fri, 7 Jul 2023 10:37:15 +0200 Subject: [PATCH 03/46] Add TemperatureMetrics to timeseries.battery_pool Signed-off-by: Christian Parpart --- .../sdk/timeseries/battery_pool/__init__.py | 3 ++- .../battery_pool/_metric_calculator.py | 2 +- .../timeseries/battery_pool/_result_types.py | 19 +++++++++++++++++++ 3 files changed, 22 insertions(+), 2 deletions(-) diff --git a/src/frequenz/sdk/timeseries/battery_pool/__init__.py b/src/frequenz/sdk/timeseries/battery_pool/__init__.py index 56f07eda2..1d6ce5c77 100644 --- a/src/frequenz/sdk/timeseries/battery_pool/__init__.py +++ b/src/frequenz/sdk/timeseries/battery_pool/__init__.py @@ -3,11 +3,12 @@ """Manage a pool of batteries.""" -from ._result_types import Bound, PowerMetrics +from ._result_types import Bound, PowerMetrics, TemperatureMetrics from .battery_pool import BatteryPool __all__ = [ "BatteryPool", "PowerMetrics", + "TemperatureMetrics", "Bound", ] diff --git a/src/frequenz/sdk/timeseries/battery_pool/_metric_calculator.py b/src/frequenz/sdk/timeseries/battery_pool/_metric_calculator.py index f0da2c95e..f25f43e01 100644 --- a/src/frequenz/sdk/timeseries/battery_pool/_metric_calculator.py +++ b/src/frequenz/sdk/timeseries/battery_pool/_metric_calculator.py @@ -12,7 +12,7 @@ from ...microgrid import connection_manager from ...microgrid.component import ComponentCategory, ComponentMetricId, InverterType -from ...timeseries import Energy, Percentage, Sample +from ...timeseries import Energy, Percentage, Sample, Temperature from ._component_metrics import ComponentMetricsData from ._result_types import Bound, PowerMetrics diff --git a/src/frequenz/sdk/timeseries/battery_pool/_result_types.py b/src/frequenz/sdk/timeseries/battery_pool/_result_types.py index 728d68a5e..89031f071 100644 --- a/src/frequenz/sdk/timeseries/battery_pool/_result_types.py +++ b/src/frequenz/sdk/timeseries/battery_pool/_result_types.py @@ -6,6 +6,8 @@ from dataclasses import dataclass, field from datetime import datetime +from ...timeseries import Temperature + @dataclass class Bound: @@ -61,3 +63,20 @@ class PowerMetrics: ) ``` """ + + +@dataclass +class TemperatureMetrics: + """Container for temperature metrics.""" + + timestamp: datetime + """Timestamp of the metrics.""" + + max: Temperature + """Maximum temperature of the collection of temperatures.""" + + min: Temperature + """Minimum temperature of the collection of temperatures.""" + + avg: Temperature + """Average temperature of the collection of temperatures.""" From 9dfb5a6788bf9b6f3bbfe9f6057945900f7e470e Mon Sep 17 00:00:00 2001 From: Christian Parpart Date: Fri, 7 Jul 2023 10:37:51 +0200 Subject: [PATCH 04/46] Add BatteryPool.temperature metrics receiver Signed-off-by: Christian Parpart --- .../battery_pool/_metric_calculator.py | 97 ++++++++++++++++++- .../timeseries/battery_pool/battery_pool.py | 27 +++++- 2 files changed, 120 insertions(+), 4 deletions(-) diff --git a/src/frequenz/sdk/timeseries/battery_pool/_metric_calculator.py b/src/frequenz/sdk/timeseries/battery_pool/_metric_calculator.py index f25f43e01..2e2ff5647 100644 --- a/src/frequenz/sdk/timeseries/battery_pool/_metric_calculator.py +++ b/src/frequenz/sdk/timeseries/battery_pool/_metric_calculator.py @@ -14,7 +14,7 @@ from ...microgrid.component import ComponentCategory, ComponentMetricId, InverterType from ...timeseries import Energy, Percentage, Sample, Temperature from ._component_metrics import ComponentMetricsData -from ._result_types import Bound, PowerMetrics +from ._result_types import Bound, PowerMetrics, TemperatureMetrics _logger = logging.getLogger(__name__) _MIN_TIMESTAMP = datetime.min.replace(tzinfo=timezone.utc) @@ -59,7 +59,7 @@ def battery_inverter_mapping(batteries: Iterable[int]) -> dict[int, int]: # Formula output types class have no common interface # Print all possible types here. -T = TypeVar("T", Sample[Percentage], Sample[Energy], PowerMetrics) +T = TypeVar("T", Sample[Percentage], Sample[Energy], PowerMetrics, TemperatureMetrics) class MetricCalculator(ABC, Generic[T]): @@ -234,6 +234,99 @@ def calculate( ) +class TemperatureCalculator(MetricCalculator[TemperatureMetrics]): + """Define how to calculate temperature metrics.""" + + def __init__(self, batteries: Set[int]) -> None: + """Create class instance. + + Args: + batteries: What batteries should be used for calculation. + """ + super().__init__(batteries) + + self._metrics = [ + ComponentMetricId.TEMPERATURE_MAX, + ] + + @classmethod + def name(cls) -> str: + """Return name of the calculator. + + Returns: + Name of the calculator + """ + return "temperature" + + @property + def battery_metrics(self) -> Mapping[int, list[ComponentMetricId]]: + """Return what metrics are needed for each battery. + + Returns: + Map between battery id and set of required metrics id. + """ + return {bid: self._metrics for bid in self._batteries} + + @property + def inverter_metrics(self) -> Mapping[int, list[ComponentMetricId]]: + """Return what metrics are needed for each inverter. + + Returns: + Map between inverter id and set of required metrics id. + """ + return {} + + def calculate( + self, + metrics_data: dict[int, ComponentMetricsData], + working_batteries: set[int], + ) -> TemperatureMetrics | None: + """Aggregate the metrics_data and calculate high level metric for temperature. + + Missing components will be ignored. Formula will be calculated for all + working batteries that are in metrics_data. + + Args: + metrics_data: Components metrics data, that should be used to calculate the + result. + working_batteries: working batteries. These batteries will be used + to calculate the result. It should be subset of the batteries given in a + constructor. + + Returns: + High level metric calculated from the given metrics. + Return None if there are no component metrics. + """ + timestamp = _MIN_TIMESTAMP + temperature_sum: float = 0 + temperature_min: float = float("inf") + temperature_max: float = float("-inf") + temperature_count: int = 0 + for battery_id in working_batteries: + if battery_id not in metrics_data: + continue + metrics = metrics_data[battery_id] + temperature = metrics.get(ComponentMetricId.TEMPERATURE_MAX) + if temperature is None: + continue + timestamp = max(timestamp, metrics.timestamp) + temperature_sum += temperature + temperature_min = min(temperature_min, temperature) + temperature_max = max(temperature_max, temperature) + temperature_count += 1 + if timestamp == _MIN_TIMESTAMP: + return None + + temperature_avg = temperature_sum / temperature_count + + return TemperatureMetrics( + timestamp=timestamp, + min=Temperature.from_celsius(value=temperature_min), + avg=Temperature.from_celsius(value=temperature_avg), + max=Temperature.from_celsius(value=temperature_max), + ) + + class SoCCalculator(MetricCalculator[Sample[Percentage]]): """Define how to calculate SoC metrics.""" diff --git a/src/frequenz/sdk/timeseries/battery_pool/battery_pool.py b/src/frequenz/sdk/timeseries/battery_pool/battery_pool.py index eb33fc682..09f073dc4 100644 --- a/src/frequenz/sdk/timeseries/battery_pool/battery_pool.py +++ b/src/frequenz/sdk/timeseries/battery_pool/battery_pool.py @@ -29,8 +29,13 @@ ) from .._quantities import Energy, Percentage, Power from ._methods import MetricAggregator, SendOnUpdate -from ._metric_calculator import CapacityCalculator, PowerBoundsCalculator, SoCCalculator -from ._result_types import PowerMetrics +from ._metric_calculator import ( + CapacityCalculator, + PowerBoundsCalculator, + SoCCalculator, + TemperatureCalculator, +) +from ._result_types import PowerMetrics, TemperatureMetrics class BatteryPool: @@ -373,6 +378,24 @@ def soc(self) -> MetricAggregator[Sample[Percentage]]: return self._active_methods[method_name] + @property + def temperature(self) -> MetricAggregator[TemperatureMetrics]: + """Fetch the maximum temperature of the batteries in the pool. + + Returns: + A MetricAggregator that will calculate and stream the maximum temperature + of all batteries in the pool. + """ + method_name = SendOnUpdate.name() + "_" + TemperatureCalculator.name() + if method_name not in self._active_methods: + calculator = TemperatureCalculator(self._batteries) + self._active_methods[method_name] = SendOnUpdate( + metric_calculator=calculator, + working_batteries=self._working_batteries, + min_update_interval=self._min_update_interval, + ) + return self._active_methods[method_name] + @property def capacity(self) -> MetricAggregator[Sample[Energy]]: """Get receiver to receive new capacity metrics when they change. From eca91fb4a160030ae3192e21a26930c593e9f1c5 Mon Sep 17 00:00:00 2001 From: Jack Date: Mon, 17 Jul 2023 16:48:38 +0200 Subject: [PATCH 05/46] Nice error message for empty buffer and new test case Signed-off-by: Jack --- src/frequenz/sdk/timeseries/_moving_window.py | 3 +++ tests/timeseries/test_moving_window.py | 11 +++++++++++ 2 files changed, 14 insertions(+) diff --git a/src/frequenz/sdk/timeseries/_moving_window.py b/src/frequenz/sdk/timeseries/_moving_window.py index b2d254ee3..42e02392c 100644 --- a/src/frequenz/sdk/timeseries/_moving_window.py +++ b/src/frequenz/sdk/timeseries/_moving_window.py @@ -305,6 +305,8 @@ def __getitem__(self, key: SupportsIndex | datetime | slice) -> float | ArrayLik A float if the key is a number or a timestamp. an numpy array if the key is a slice. """ + if len(self._buffer) == 0: + raise IndexError("The buffer is empty.") if isinstance(key, slice): if isinstance(key.start, int) or isinstance(key.stop, int): if key.start is None or key.stop is None: @@ -327,6 +329,7 @@ def __getitem__(self, key: SupportsIndex | datetime | slice) -> float | ArrayLik _logger.debug("Returning value at time %s ", key) return self._buffer[self._buffer.datetime_to_index(key)] elif isinstance(key, SupportsIndex): + _logger.debug("Returning value at index %s ", key) return self._buffer[key] raise TypeError( diff --git a/tests/timeseries/test_moving_window.py b/tests/timeseries/test_moving_window.py index 032075c31..42c196809 100644 --- a/tests/timeseries/test_moving_window.py +++ b/tests/timeseries/test_moving_window.py @@ -111,6 +111,17 @@ async def test_access_window_by_ts_slice() -> None: assert np.array_equal(window[time_start:time_end], np.array([3.0, 4.0])) # type: ignore +async def test_access_empty_window() -> None: + """Test accessing an empty window, should throw IndexError""" + window, _ = init_moving_window(timedelta(seconds=5)) + try: + window[42] + except IndexError as index_error: + assert str(index_error) == "The buffer is empty." + else: + assert False + + async def test_window_size() -> None: """Test the size of the window.""" window, sender = init_moving_window(timedelta(seconds=5)) From b9ea7f12fe0020ede3b59914a3a69299ef1fdc28 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 19 Jul 2023 07:07:00 +0000 Subject: [PATCH 06/46] Bump mkdocs-material from 9.1.18 to 9.1.19 Bumps [mkdocs-material](https://github.com/squidfunk/mkdocs-material) from 9.1.18 to 9.1.19. - [Release notes](https://github.com/squidfunk/mkdocs-material/releases) - [Changelog](https://github.com/squidfunk/mkdocs-material/blob/master/CHANGELOG) - [Commits](https://github.com/squidfunk/mkdocs-material/compare/9.1.18...9.1.19) --- updated-dependencies: - dependency-name: mkdocs-material dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 82a46f1c2..9cbdfe484 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -60,7 +60,7 @@ dev-mkdocs = [ "mike == 1.1.2", "mkdocs-gen-files == 0.5.0", "mkdocs-literate-nav == 0.6.0", - "mkdocs-material == 9.1.18", + "mkdocs-material == 9.1.19", "mkdocs-section-index == 0.3.5", "mkdocstrings[python] == 0.22.0", "frequenz-repo-config[lib] == 0.3.0", From c5af86b22be7e5dec1e05fc7b6d2c597fdf01067 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 21 Jul 2023 08:12:18 +0000 Subject: [PATCH 07/46] Bump polars from 0.18.7 to 0.18.8 Bumps [polars](https://github.com/pola-rs/polars) from 0.18.7 to 0.18.8. - [Release notes](https://github.com/pola-rs/polars/releases) - [Commits](https://github.com/pola-rs/polars/compare/py-0.18.7...py-0.18.8) --- updated-dependencies: - dependency-name: polars dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 9cbdfe484..121605fd5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -54,7 +54,7 @@ dev-docstrings = [ "darglint == 1.8.1", "tomli == 2.0.1", # Needed by pydocstyle to read pyproject.toml ] -dev-examples = ["polars == 0.18.7"] +dev-examples = ["polars == 0.18.8"] dev-formatting = ["black == 23.7.0", "isort == 5.12.0"] dev-mkdocs = [ "mike == 1.1.2", From da6a5ce90e89d968032bc03e44c705e27e65351a Mon Sep 17 00:00:00 2001 From: "Mathias L. Baumann" Date: Thu, 13 Jul 2023 16:11:38 +0200 Subject: [PATCH 08/46] Add capability to multiply quantities with percentage Signed-off-by: Mathias L. Baumann --- RELEASE_NOTES.md | 3 +- src/frequenz/sdk/timeseries/_quantities.py | 116 ++++++++++++++++++--- tests/timeseries/test_quantities.py | 44 ++++++++ 3 files changed, 148 insertions(+), 15 deletions(-) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 43c161352..e9e777455 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -12,8 +12,9 @@ ## New Features -- Add quantity class `Frequency` for frequency values. - Add `abs()` support for quantities. +* Add quantity class `Frequency` for frequency values. +* Quantities can now be multiplied with `Percentage` types. ## Bug Fixes diff --git a/src/frequenz/sdk/timeseries/_quantities.py b/src/frequenz/sdk/timeseries/_quantities.py index 1e896a050..bc086bf41 100644 --- a/src/frequenz/sdk/timeseries/_quantities.py +++ b/src/frequenz/sdk/timeseries/_quantities.py @@ -221,6 +221,22 @@ def __sub__(self, other: Self) -> Self: difference._base_value = self._base_value - other._base_value return difference + def __mul__(self, percent: Percentage) -> Self: + """Return the product of this quantity and a percentage. + + Args: + percent: The percentage. + + Returns: + The product of this quantity and a percentage. + """ + if not isinstance(percent, Percentage): + return NotImplemented + + product = type(self).__new__(type(self)) + product._base_value = self._base_value * percent.as_fraction() + return product + def __gt__(self, other: Self) -> bool: """Return whether this quantity is greater than another. @@ -423,18 +439,42 @@ def as_megawatts(self) -> float: """ return self._base_value / 1e6 - def __mul__(self, duration: timedelta) -> Energy: + @overload # type: ignore + def __mul__(self, other: Percentage) -> Self: + """Return a power from multiplying this power by the given percentage. + + Args: + other: The percentage to multiply by. + """ + + @overload + def __mul__(self, other: timedelta) -> Energy: """Return an energy from multiplying this power by the given duration. Args: - duration: The duration to multiply by. + other: The duration to multiply by. + """ + + def __mul__(self, other: Percentage | timedelta) -> Self | Energy: + """Return a power or energy from multiplying this power by the given value. + + Args: + other: The percentage or duration to multiply by. Returns: - An energy from multiplying this power by the given duration. + A power or energy. + + Raises: + TypeError: If the given value is not a percentage or duration. """ - return Energy.from_watt_hours( - self._base_value * duration.total_seconds() / 3600.0 - ) + if isinstance(other, Percentage): + return super().__mul__(other) + if isinstance(other, timedelta): + return Energy.from_watt_hours( + self._base_value * other.total_seconds() / 3600.0 + ) + + return NotImplemented @overload def __truediv__(self, other: Current) -> Voltage: @@ -527,16 +567,40 @@ def as_milliamperes(self) -> float: """ return self._base_value * 1e3 - def __mul__(self, voltage: Voltage) -> Power: + @overload # type: ignore + def __mul__(self, other: Percentage) -> Self: + """Return a power from multiplying this power by the given percentage. + + Args: + other: The percentage to multiply by. + """ + + @overload + def __mul__(self, other: Voltage) -> Power: """Multiply the current by a voltage to get a power. Args: - voltage: The voltage. + other: The voltage. + """ + + def __mul__(self, other: Percentage | Voltage) -> Self | Power: + """Return a current or power from multiplying this current by the given value. + + Args: + other: The percentage or voltage to multiply by. Returns: - The power. + A current or power. + + Raises: + TypeError: If the given value is not a percentage or voltage. """ - return Power.from_watts(self._base_value * voltage._base_value) + if isinstance(other, Percentage): + return super().__mul__(other) + if isinstance(other, Voltage): + return Power.from_watts(self._base_value * other._base_value) + + return NotImplemented class Voltage( @@ -612,16 +676,40 @@ def as_kilovolts(self) -> float: """ return self._base_value / 1e3 - def __mul__(self, current: Current) -> Power: + @overload # type: ignore + def __mul__(self, other: Percentage) -> Self: + """Return a power from multiplying this power by the given percentage. + + Args: + other: The percentage to multiply by. + """ + + @overload + def __mul__(self, other: Current) -> Power: """Multiply the voltage by the current to get the power. Args: - current: The current to multiply the voltage with. + other: The current to multiply the voltage with. + """ + + def __mul__(self, other: Percentage | Current) -> Self | Power: + """Return a voltage or power from multiplying this voltage by the given value. + + Args: + other: The percentage or current to multiply by. Returns: - The calculated power. + The calculated voltage or power. + + Raises: + TypeError: If the given value is not a percentage or current. """ - return Power.from_watts(self._base_value * current._base_value) + if isinstance(other, Percentage): + return super().__mul__(other) + if isinstance(other, Current): + return Power.from_watts(self._base_value * other._base_value) + + return NotImplemented class Energy( diff --git a/tests/timeseries/test_quantities.py b/tests/timeseries/test_quantities.py index 14b45eb7b..16c89b36c 100644 --- a/tests/timeseries/test_quantities.py +++ b/tests/timeseries/test_quantities.py @@ -377,3 +377,47 @@ def test_abs() -> None: pct = Percentage.from_fraction(30) assert abs(pct) == Percentage.from_fraction(30) assert abs(-pct) == Percentage.from_fraction(30) + + +def test_quantity_multiplied_with_precentage() -> None: + """Test the multiplication of all quantities with percentage.""" + percentage = Percentage.from_percent(50) + power = Power.from_watts(1000.0) + voltage = Voltage.from_volts(230.0) + current = Current.from_amperes(2) + energy = Energy.from_kilowatt_hours(12) + percentage_ = Percentage.from_percent(50) + + assert power * percentage == Power.from_watts(500.0) + assert voltage * percentage == Voltage.from_volts(115.0) + assert current * percentage == Current.from_amperes(1) + assert energy * percentage == Energy.from_kilowatt_hours(6) + assert percentage_ * percentage == Percentage.from_percent(25) + + +def test_invalid_multiplications() -> None: + """Test the multiplication of quantities with invalid quantities.""" + power = Power.from_watts(1000.0) + voltage = Voltage.from_volts(230.0) + current = Current.from_amperes(2) + energy = Energy.from_kilowatt_hours(12) + + for quantity in [power, voltage, current, energy]: + with pytest.raises(TypeError): + _ = power * quantity # type: ignore + + for quantity in [voltage, power, energy]: + with pytest.raises(TypeError): + _ = voltage * quantity # type: ignore + + for quantity in [current, power, energy]: + with pytest.raises(TypeError): + _ = current * quantity # type: ignore + + for quantity in [energy, power, voltage, current]: + with pytest.raises(TypeError): + _ = energy * quantity # type: ignore + + for quantity in [power, voltage, current, energy, Percentage.from_percent(50)]: + with pytest.raises(TypeError): + _ = quantity * 200.0 # type: ignore From 1043360af7b040674de553b7d663fa02404e5c19 Mon Sep 17 00:00:00 2001 From: Sahas Subramanian Date: Fri, 21 Jul 2023 17:01:48 +0200 Subject: [PATCH 09/46] Implement dunder iadd, isub, imul methods for Quantity imul is implemented only for `Percentage` because, all other multiplications that we have produce outputs of a new type, which is not possible with imul. With this usage like this becomes possible: ```python power = Power.from_watts(1000.0) power += Power.from_watts(3.2) ``` Signed-off-by: Sahas Subramanian --- src/frequenz/sdk/timeseries/_quantities.py | 44 ++++++++++++++++++++++ tests/timeseries/test_quantities.py | 30 +++++++++++++++ 2 files changed, 74 insertions(+) diff --git a/src/frequenz/sdk/timeseries/_quantities.py b/src/frequenz/sdk/timeseries/_quantities.py index bc086bf41..05e0f3223 100644 --- a/src/frequenz/sdk/timeseries/_quantities.py +++ b/src/frequenz/sdk/timeseries/_quantities.py @@ -3,6 +3,8 @@ """Types for holding quantities with units.""" +# pylint: disable=too-many-lines + from __future__ import annotations import math @@ -237,6 +239,48 @@ def __mul__(self, percent: Percentage) -> Self: product._base_value = self._base_value * percent.as_fraction() return product + def __iadd__(self, other: Self) -> Self: + """Add another quantity to this one. + + Args: + other: The other quantity. + + Returns: + This quantity. + """ + if not type(other) is type(self): + return NotImplemented + self._base_value += other._base_value + return self + + def __isub__(self, other: Self) -> Self: + """Subtract another quantity from this one. + + Args: + other: The other quantity. + + Returns: + This quantity. + """ + if not type(other) is type(self): + return NotImplemented + self._base_value -= other._base_value + return self + + def __imul__(self, percent: Percentage) -> Self: + """Multiply this quantity by a percentage. + + Args: + percent: The percentage. + + Returns: + This quantity. + """ + if not isinstance(percent, Percentage): + return NotImplemented + self._base_value *= percent.as_fraction() + return self + def __gt__(self, other: Self) -> bool: """Return whether this quantity is greater than another. diff --git a/tests/timeseries/test_quantities.py b/tests/timeseries/test_quantities.py index 16c89b36c..f6a4c161f 100644 --- a/tests/timeseries/test_quantities.py +++ b/tests/timeseries/test_quantities.py @@ -105,6 +105,15 @@ def test_addition_subtraction() -> None: assert Fz1(1) - Fz2(1) # type: ignore assert excinfo.value.args[0] == "unsupported operand type(s) for -: 'Fz1' and 'Fz2'" + fz1 = Fz1(1.0) + fz1 += Fz1(4.0) + assert fz1 == Fz1(5.0) + fz1 -= Fz1(9.0) + assert fz1 == Fz1(-4.0) + + with pytest.raises(TypeError) as excinfo: + fz1 += Fz2(1.0) # type: ignore + def test_comparison() -> None: """Test the comparison of the quantities.""" @@ -394,6 +403,17 @@ def test_quantity_multiplied_with_precentage() -> None: assert energy * percentage == Energy.from_kilowatt_hours(6) assert percentage_ * percentage == Percentage.from_percent(25) + power *= percentage + assert power == Power.from_watts(500.0) + voltage *= percentage + assert voltage == Voltage.from_volts(115.0) + current *= percentage + assert current == Current.from_amperes(1) + energy *= percentage + assert energy == Energy.from_kilowatt_hours(6) + percentage_ *= percentage + assert percentage_ == Percentage.from_percent(25) + def test_invalid_multiplications() -> None: """Test the multiplication of quantities with invalid quantities.""" @@ -405,19 +425,29 @@ def test_invalid_multiplications() -> None: for quantity in [power, voltage, current, energy]: with pytest.raises(TypeError): _ = power * quantity # type: ignore + with pytest.raises(TypeError): + power *= quantity # type: ignore for quantity in [voltage, power, energy]: with pytest.raises(TypeError): _ = voltage * quantity # type: ignore + with pytest.raises(TypeError): + voltage *= quantity # type: ignore for quantity in [current, power, energy]: with pytest.raises(TypeError): _ = current * quantity # type: ignore + with pytest.raises(TypeError): + current *= quantity # type: ignore for quantity in [energy, power, voltage, current]: with pytest.raises(TypeError): _ = energy * quantity # type: ignore + with pytest.raises(TypeError): + energy *= quantity # type: ignore for quantity in [power, voltage, current, energy, Percentage.from_percent(50)]: with pytest.raises(TypeError): _ = quantity * 200.0 # type: ignore + with pytest.raises(TypeError): + quantity *= 200.0 # type: ignore From 68a73606f26b331dbc008fa448e56729b5d48954 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 24 Jul 2023 07:40:48 +0000 Subject: [PATCH 10/46] Bump types-protobuf from 4.23.0.1 to 4.23.0.2 Bumps [types-protobuf](https://github.com/python/typeshed) from 4.23.0.1 to 4.23.0.2. - [Commits](https://github.com/python/typeshed/commits) --- updated-dependencies: - dependency-name: types-protobuf dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 121605fd5..4da74a47a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -68,7 +68,7 @@ dev-mkdocs = [ dev-mypy = [ "mypy == 1.4.1", "grpc-stubs == 1.24.12", # This dependency introduces breaking changes in patch releases - "types-protobuf == 4.23.0.1", + "types-protobuf == 4.23.0.2", # For checking the noxfile, docs/ script, and tests "frequenz-sdk[dev-mkdocs,dev-noxfile,dev-pytest]", ] From 207d96a826519bcaac228f9222d2e15b8b2e1c50 Mon Sep 17 00:00:00 2001 From: Matthias Wende Date: Tue, 18 Jul 2023 17:59:44 +0200 Subject: [PATCH 11/46] Add token type CONSTANT Signed-off-by: Matthias Wende --- src/frequenz/sdk/timeseries/_formula_engine/_tokenizer.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/frequenz/sdk/timeseries/_formula_engine/_tokenizer.py b/src/frequenz/sdk/timeseries/_formula_engine/_tokenizer.py index ce2be427a..956bf67b5 100644 --- a/src/frequenz/sdk/timeseries/_formula_engine/_tokenizer.py +++ b/src/frequenz/sdk/timeseries/_formula_engine/_tokenizer.py @@ -79,7 +79,8 @@ class TokenType(Enum): """Represents the types of tokens the Tokenizer can return.""" COMPONENT_METRIC = 0 - OPER = 1 + CONSTANT = 1 + OPER = 2 @dataclass From 7dc5283ae67523a873856577748644c4fcb757ab Mon Sep 17 00:00:00 2001 From: Matthias Wende Date: Tue, 18 Jul 2023 18:00:21 +0200 Subject: [PATCH 12/46] Support constants in arithmetic FormulaEngine operations Signed-off-by: Matthias Wende --- .../_formula_engine/_formula_engine.py | 54 +++++++++++++------ 1 file changed, 38 insertions(+), 16 deletions(-) diff --git a/src/frequenz/sdk/timeseries/_formula_engine/_formula_engine.py b/src/frequenz/sdk/timeseries/_formula_engine/_formula_engine.py index bc46c3726..906795a82 100644 --- a/src/frequenz/sdk/timeseries/_formula_engine/_formula_engine.py +++ b/src/frequenz/sdk/timeseries/_formula_engine/_formula_engine.py @@ -29,7 +29,7 @@ from ..._internal._asyncio import cancel_and_await from .. import Sample, Sample3Phase -from .._quantities import QuantityT +from .._quantities import Quantity, QuantityT from ._formula_steps import ( Adder, Averager, @@ -231,7 +231,7 @@ async def _stop(self) -> None: def __add__( self, - other: _GenericEngine | _GenericHigherOrderBuilder, + other: _GenericEngine | _GenericHigherOrderBuilder | QuantityT, ) -> _GenericHigherOrderBuilder: """Return a formula builder that adds (data in) `other` to `self`. @@ -246,7 +246,7 @@ def __add__( return self._higher_order_builder(self, self._create_method) + other # type: ignore def __sub__( - self, other: _GenericEngine | _GenericHigherOrderBuilder + self, other: _GenericEngine | _GenericHigherOrderBuilder | QuantityT ) -> _GenericHigherOrderBuilder: """Return a formula builder that subtracts (data in) `other` from `self`. @@ -261,7 +261,7 @@ def __sub__( return self._higher_order_builder(self, self._create_method) - other # type: ignore def __mul__( - self, other: _GenericEngine | _GenericHigherOrderBuilder + self, other: _GenericEngine | _GenericHigherOrderBuilder | float ) -> _GenericHigherOrderBuilder: """Return a formula builder that multiplies (data in) `self` with `other`. @@ -276,7 +276,7 @@ def __mul__( return self._higher_order_builder(self, self._create_method) * other # type: ignore def __truediv__( - self, other: _GenericEngine | _GenericHigherOrderBuilder + self, other: _GenericEngine | _GenericHigherOrderBuilder | float ) -> _GenericHigherOrderBuilder: """Return a formula builder that divides (data in) `self` by `other`. @@ -740,7 +740,11 @@ def __init__( self._steps: deque[ tuple[ TokenType, - FormulaEngine[QuantityT] | FormulaEngine3Phase[QuantityT] | str, + FormulaEngine[QuantityT] + | FormulaEngine3Phase[QuantityT] + | QuantityT + | float + | str, ] ] = deque() self._steps.append((TokenType.COMPONENT_METRIC, engine)) @@ -754,12 +758,12 @@ def _push( @overload def _push( - self, oper: str, other: _CompositionType3Phase + self, oper: str, other: _CompositionType3Phase | QuantityT | float ) -> HigherOrderFormulaBuilder3Phase[QuantityT]: ... def _push( - self, oper: str, other: _CompositionType + self, oper: str, other: _CompositionType | QuantityT | float ) -> ( HigherOrderFormulaBuilder[QuantityT] | HigherOrderFormulaBuilder3Phase[QuantityT] @@ -771,6 +775,19 @@ def _push( # pylint: disable=protected-access if isinstance(other, (FormulaEngine, FormulaEngine3Phase)): self._steps.append((TokenType.COMPONENT_METRIC, other)) + elif isinstance(other, (Quantity, float)): + match oper: + case "+" | "-": + if not isinstance(other, Quantity): + raise RuntimeError( + f"A Quantity must be provided for addition or subtraction to {other}" + ) + case "*" | "/": + if not isinstance(other, (float, int)): + raise RuntimeError( + f"A float must be provided for scalar multiplication to {other}" + ) + self._steps.append((TokenType.CONSTANT, other)) elif isinstance(other, _BaseHOFormulaBuilder): self._steps.append((TokenType.OPER, "(")) self._steps.extend(other._steps) @@ -791,12 +808,12 @@ def __add__( @overload def __add__( - self, other: _CompositionType3Phase + self, other: _CompositionType3Phase | QuantityT ) -> HigherOrderFormulaBuilder3Phase[QuantityT]: ... def __add__( - self, other: _CompositionType + self, other: _CompositionType | QuantityT ) -> ( HigherOrderFormulaBuilder[QuantityT] | HigherOrderFormulaBuilder3Phase[QuantityT] @@ -821,13 +838,13 @@ def __sub__( @overload def __sub__( - self, other: _CompositionType3Phase + self, other: _CompositionType3Phase | QuantityT ) -> HigherOrderFormulaBuilder3Phase[QuantityT]: ... def __sub__( self, - other: _CompositionType, + other: _CompositionType | QuantityT, ) -> ( HigherOrderFormulaBuilder[QuantityT] | HigherOrderFormulaBuilder3Phase[QuantityT] @@ -852,13 +869,13 @@ def __mul__( @overload def __mul__( - self, other: _CompositionType3Phase + self, other: _CompositionType3Phase | float ) -> HigherOrderFormulaBuilder3Phase[QuantityT]: ... def __mul__( self, - other: _CompositionType, + other: _CompositionType | float, ) -> ( HigherOrderFormulaBuilder[QuantityT] | HigherOrderFormulaBuilder3Phase[QuantityT] @@ -883,13 +900,13 @@ def __truediv__( @overload def __truediv__( - self, other: _CompositionType3Phase + self, other: _CompositionType3Phase | float ) -> HigherOrderFormulaBuilder3Phase[QuantityT]: ... def __truediv__( self, - other: _CompositionType, + other: _CompositionType | float, ) -> ( HigherOrderFormulaBuilder[QuantityT] | HigherOrderFormulaBuilder3Phase[QuantityT] @@ -935,6 +952,11 @@ def build( elif typ == TokenType.OPER: assert isinstance(value, str) builder.push_oper(value) + elif typ == TokenType.CONSTANT: + assert isinstance(value, (Quantity, float)) + builder.push_constant( + value.base_value if isinstance(value, Quantity) else value + ) return builder.build() From d47c657c4e7a65940e6294295d93d593805ac56b Mon Sep 17 00:00:00 2001 From: Matthias Wende Date: Thu, 20 Jul 2023 16:24:29 +0200 Subject: [PATCH 13/46] Add test for composing formulas with constants Signed-off-by: Matthias Wende --- .../test_formula_composition.py | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/tests/timeseries/_formula_engine/test_formula_composition.py b/tests/timeseries/_formula_engine/test_formula_composition.py index 48d10b967..23447839f 100644 --- a/tests/timeseries/_formula_engine/test_formula_composition.py +++ b/tests/timeseries/_formula_engine/test_formula_composition.py @@ -170,6 +170,62 @@ async def test_formula_composition_missing_bat(self, mocker: MockerFixture) -> N assert count == 10 + async def test_formula_composition_constant(self, mocker: MockerFixture) -> None: + """Test the composition of formulas with constant values.""" + mockgrid = MockMicrogrid(grid_side_meter=True) + await mockgrid.start(mocker) + + logical_meter = microgrid.logical_meter() + engine_add = (logical_meter.grid_power + Power.from_watts(50)).build( + "grid_power_addition" + ) + engine_sub = (logical_meter.grid_power - Power.from_watts(100)).build( + "grid_power_subtraction" + ) + engine_mul = (logical_meter.grid_power * 2.0).build("grid_power_multiplication") + engine_div = (logical_meter.grid_power / 2.0).build("grid_power_division") + + await mockgrid.mock_resampler.send_meter_power([100.0]) + + # Test addition + grid_power_addition = await engine_add.new_receiver().receive() + assert grid_power_addition.value is not None + assert math.isclose( + grid_power_addition.value.as_watts(), + 150.0, + ) + + # Test subtraction + grid_power_subtraction = await engine_sub.new_receiver().receive() + assert grid_power_subtraction.value is not None + assert math.isclose( + grid_power_subtraction.value.as_watts(), + 0.0, + ) + + # Test multiplication + grid_power_multiplication = await engine_mul.new_receiver().receive() + assert grid_power_multiplication.value is not None + assert math.isclose( + grid_power_multiplication.value.as_watts(), + 200.0, + ) + + # Test division + grid_power_division = await engine_div.new_receiver().receive() + assert grid_power_division.value is not None + assert math.isclose( + grid_power_division.value.as_watts(), + 50.0, + ) + + await engine_add._stop() # pylint: disable=protected-access + await engine_sub._stop() # pylint: disable=protected-access + await engine_mul._stop() # pylint: disable=protected-access + await engine_div._stop() # pylint: disable=protected-access + await mockgrid.cleanup() + await logical_meter.stop() + async def test_3_phase_formulas(self, mocker: MockerFixture) -> None: """Test 3 phase formulas current formulas and their composition.""" mockgrid = MockMicrogrid( From dcc4b30b9ae86423512c4778c06510ec863a500f Mon Sep 17 00:00:00 2001 From: Matthias Wende Date: Mon, 24 Jul 2023 16:33:45 +0200 Subject: [PATCH 14/46] Add failing tests for formula composition Signed-off-by: Matthias Wende --- .../_formula_engine/test_formula_composition.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/timeseries/_formula_engine/test_formula_composition.py b/tests/timeseries/_formula_engine/test_formula_composition.py index 23447839f..3c6082af6 100644 --- a/tests/timeseries/_formula_engine/test_formula_composition.py +++ b/tests/timeseries/_formula_engine/test_formula_composition.py @@ -6,6 +6,7 @@ import math +import pytest from pytest_mock import MockerFixture from frequenz.sdk import microgrid @@ -219,6 +220,20 @@ async def test_formula_composition_constant(self, mocker: MockerFixture) -> None 50.0, ) + # Test multiplication with a Quantity + with pytest.raises(RuntimeError): + engine_assert = ( + logical_meter.grid_power * Power.from_watts(2.0) # type: ignore + ).build("grid_power_multiplication") + await engine_assert.new_receiver().receive() + + # Test addition with a float + with pytest.raises(RuntimeError): + engine_assert = (logical_meter.grid_power + 2.0).build( # type: ignore + "grid_power_multiplication" + ) + await engine_assert.new_receiver().receive() + await engine_add._stop() # pylint: disable=protected-access await engine_sub._stop() # pylint: disable=protected-access await engine_mul._stop() # pylint: disable=protected-access From 674ac1a7f71a08a52b3246b3bf7e271184ec504a Mon Sep 17 00:00:00 2001 From: Matthias Wende Date: Mon, 24 Jul 2023 12:01:41 +0200 Subject: [PATCH 15/46] Move formula evaluator to separate file This patch is done in order to decrease the size of the `_formula_engine.py` file. Signed-off-by: Matthias Wende --- .../_formula_engine/_formula_engine.py | 124 +--------------- .../_formula_engine/_formula_evaluator.py | 133 ++++++++++++++++++ 2 files changed, 134 insertions(+), 123 deletions(-) create mode 100644 src/frequenz/sdk/timeseries/_formula_engine/_formula_evaluator.py diff --git a/src/frequenz/sdk/timeseries/_formula_engine/_formula_engine.py b/src/frequenz/sdk/timeseries/_formula_engine/_formula_engine.py index 906795a82..6dd989200 100644 --- a/src/frequenz/sdk/timeseries/_formula_engine/_formula_engine.py +++ b/src/frequenz/sdk/timeseries/_formula_engine/_formula_engine.py @@ -9,15 +9,12 @@ import logging from abc import ABC from collections import deque -from datetime import datetime -from math import isinf, isnan from typing import ( Callable, Dict, Generic, List, Optional, - Set, Tuple, Type, TypeVar, @@ -30,6 +27,7 @@ from ..._internal._asyncio import cancel_and_await from .. import Sample, Sample3Phase from .._quantities import Quantity, QuantityT +from ._formula_evaluator import FormulaEvaluator from ._formula_steps import ( Adder, Averager, @@ -56,126 +54,6 @@ } -class FormulaEvaluator(Generic[QuantityT]): - """A post-fix formula evaluator that operates on `Sample` receivers.""" - - def __init__( - self, - name: str, - steps: List[FormulaStep], - metric_fetchers: Dict[str, MetricFetcher[QuantityT]], - create_method: Callable[[float], QuantityT], - ) -> None: - """Create a `FormulaEngine` instance. - - Args: - name: A name for the formula. - steps: Steps for the engine to execute, in post-fix order. - metric_fetchers: Fetchers for each metric stream the formula depends on. - create_method: A method to generate the output `Sample` value with. If the - formula is for generating power values, this would be - `Power.from_watts`, for example. - """ - self._name = name - self._steps = steps - self._metric_fetchers: Dict[str, MetricFetcher[QuantityT]] = metric_fetchers - self._first_run = True - self._create_method: Callable[[float], QuantityT] = create_method - - async def _synchronize_metric_timestamps( - self, metrics: Set[asyncio.Task[Optional[Sample[QuantityT]]]] - ) -> datetime: - """Synchronize the metric streams. - - For synchronised streams like data from the `ComponentMetricsResamplingActor`, - this a call to this function is required only once, before the first set of - inputs are fetched. - - Args: - metrics: The finished tasks from the first `fetch_next` calls to all the - `MetricFetcher`s. - - Returns: - The timestamp of the latest metric value. - - Raises: - RuntimeError: when some streams have no value, or when the synchronization - of timestamps fails. - """ - metrics_by_ts: Dict[datetime, list[str]] = {} - for metric in metrics: - result = metric.result() - name = metric.get_name() - if result is None: - raise RuntimeError(f"Stream closed for component: {name}") - metrics_by_ts.setdefault(result.timestamp, []).append(name) - latest_ts = max(metrics_by_ts) - - # fetch the metrics with non-latest timestamps again until we have the values - # for the same ts for all metrics. - for metric_ts, names in metrics_by_ts.items(): - if metric_ts == latest_ts: - continue - while metric_ts < latest_ts: - for name in names: - fetcher = self._metric_fetchers[name] - next_val = await fetcher.fetch_next() - assert next_val is not None - metric_ts = next_val.timestamp - if metric_ts > latest_ts: - raise RuntimeError( - "Unable to synchronize resampled metric timestamps, " - f"for formula: {self._name}" - ) - self._first_run = False - return latest_ts - - async def apply(self) -> Sample[QuantityT]: - """Fetch the latest metrics, apply the formula once and return the result. - - Returns: - The result of the formula. - - Raises: - RuntimeError: if some samples didn't arrive, or if formula application - failed. - """ - eval_stack: List[float] = [] - ready_metrics, pending = await asyncio.wait( - [ - asyncio.create_task(fetcher.fetch_next(), name=name) - for name, fetcher in self._metric_fetchers.items() - ], - return_when=asyncio.ALL_COMPLETED, - ) - - if pending or any(res.result() is None for res in iter(ready_metrics)): - raise RuntimeError( - f"Some resampled metrics didn't arrive, for formula: {self._name}" - ) - - if self._first_run: - metric_ts = await self._synchronize_metric_timestamps(ready_metrics) - else: - sample = next(iter(ready_metrics)).result() - assert sample is not None - metric_ts = sample.timestamp - - for step in self._steps: - step.apply(eval_stack) - - # if all steps were applied and the formula was correct, there should only be a - # single value in the evaluation stack, and that would be the formula result. - if len(eval_stack) != 1: - raise RuntimeError(f"Formula application failed: {self._name}") - - res = eval_stack.pop() - if isnan(res) or isinf(res): - return Sample(metric_ts, None) - - return Sample(metric_ts, self._create_method(res)) - - _CompositionType = Union[ "FormulaEngine", "HigherOrderFormulaBuilder", diff --git a/src/frequenz/sdk/timeseries/_formula_engine/_formula_evaluator.py b/src/frequenz/sdk/timeseries/_formula_engine/_formula_evaluator.py new file mode 100644 index 000000000..6f2313858 --- /dev/null +++ b/src/frequenz/sdk/timeseries/_formula_engine/_formula_evaluator.py @@ -0,0 +1,133 @@ +# License: MIT +# Copyright © 2022 Frequenz Energy-as-a-Service GmbH + +"""A post-fix formula evaluator that operates on `Sample` receivers.""" + +import asyncio +from datetime import datetime +from math import isinf, isnan +from typing import Callable, Dict, Generic, List, Optional, Set + +from .. import Sample +from .._quantities import QuantityT +from ._formula_steps import FormulaStep, MetricFetcher + + +class FormulaEvaluator(Generic[QuantityT]): + """A post-fix formula evaluator that operates on `Sample` receivers.""" + + def __init__( + self, + name: str, + steps: List[FormulaStep], + metric_fetchers: Dict[str, MetricFetcher[QuantityT]], + create_method: Callable[[float], QuantityT], + ) -> None: + """Create a `FormulaEngine` instance. + + Args: + name: A name for the formula. + steps: Steps for the engine to execute, in post-fix order. + metric_fetchers: Fetchers for each metric stream the formula depends on. + create_method: A method to generate the output `Sample` value with. If the + formula is for generating power values, this would be + `Power.from_watts`, for example. + """ + self._name = name + self._steps = steps + self._metric_fetchers: Dict[str, MetricFetcher[QuantityT]] = metric_fetchers + self._first_run = True + self._create_method: Callable[[float], QuantityT] = create_method + + async def _synchronize_metric_timestamps( + self, metrics: Set[asyncio.Task[Optional[Sample[QuantityT]]]] + ) -> datetime: + """Synchronize the metric streams. + + For synchronised streams like data from the `ComponentMetricsResamplingActor`, + this a call to this function is required only once, before the first set of + inputs are fetched. + + Args: + metrics: The finished tasks from the first `fetch_next` calls to all the + `MetricFetcher`s. + + Returns: + The timestamp of the latest metric value. + + Raises: + RuntimeError: when some streams have no value, or when the synchronization + of timestamps fails. + """ + metrics_by_ts: Dict[datetime, list[str]] = {} + for metric in metrics: + result = metric.result() + name = metric.get_name() + if result is None: + raise RuntimeError(f"Stream closed for component: {name}") + metrics_by_ts.setdefault(result.timestamp, []).append(name) + latest_ts = max(metrics_by_ts) + + # fetch the metrics with non-latest timestamps again until we have the values + # for the same ts for all metrics. + for metric_ts, names in metrics_by_ts.items(): + if metric_ts == latest_ts: + continue + while metric_ts < latest_ts: + for name in names: + fetcher = self._metric_fetchers[name] + next_val = await fetcher.fetch_next() + assert next_val is not None + metric_ts = next_val.timestamp + if metric_ts > latest_ts: + raise RuntimeError( + "Unable to synchronize resampled metric timestamps, " + f"for formula: {self._name}" + ) + self._first_run = False + return latest_ts + + async def apply(self) -> Sample[QuantityT]: + """Fetch the latest metrics, apply the formula once and return the result. + + Returns: + The result of the formula. + + Raises: + RuntimeError: if some samples didn't arrive, or if formula application + failed. + """ + eval_stack: List[float] = [] + ready_metrics, pending = await asyncio.wait( + [ + asyncio.create_task(fetcher.fetch_next(), name=name) + for name, fetcher in self._metric_fetchers.items() + ], + return_when=asyncio.ALL_COMPLETED, + ) + + if pending or any(res.result() is None for res in iter(ready_metrics)): + raise RuntimeError( + f"Some resampled metrics didn't arrive, for formula: {self._name}" + ) + + if self._first_run: + metric_ts = await self._synchronize_metric_timestamps(ready_metrics) + else: + sample = next(iter(ready_metrics)).result() + assert sample is not None + metric_ts = sample.timestamp + + for step in self._steps: + step.apply(eval_stack) + + # if all steps were applied and the formula was correct, there should only be a + # single value in the evaluation stack, and that would be the formula result. + if len(eval_stack) != 1: + raise RuntimeError(f"Formula application failed: {self._name}") + + res = eval_stack.pop() + if isnan(res) or isinf(res): + return Sample(metric_ts, None) + + return Sample(metric_ts, self._create_method(res)) From fdb8a51907a50e58efdb4bcc9c53d30ae4a69005 Mon Sep 17 00:00:00 2001 From: Matthias Wende Date: Fri, 21 Jul 2023 09:55:09 +0200 Subject: [PATCH 16/46] Update release notes Signed-off-by: Matthias Wende --- RELEASE_NOTES.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index e9e777455..89c97ab6c 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -13,8 +13,9 @@ ## New Features - Add `abs()` support for quantities. -* Add quantity class `Frequency` for frequency values. -* Quantities can now be multiplied with `Percentage` types. +- Add quantity class `Frequency` for frequency values. +- Quantities can now be multiplied with `Percentage` types. +- `FormulaEngine` arithmetics now supports scalar multiplication with floats and addition with Quantities ## Bug Fixes From 4009eec5319ee25395fe0a4b23f9fb7baab806e3 Mon Sep 17 00:00:00 2001 From: Sahas Subramanian Date: Mon, 24 Jul 2023 17:20:42 +0200 Subject: [PATCH 17/46] Remove support for hashing of `Quantity` objects Closes #532 Signed-off-by: Sahas Subramanian --- src/frequenz/sdk/timeseries/_quantities.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/frequenz/sdk/timeseries/_quantities.py b/src/frequenz/sdk/timeseries/_quantities.py index 05e0f3223..2424c884f 100644 --- a/src/frequenz/sdk/timeseries/_quantities.py +++ b/src/frequenz/sdk/timeseries/_quantities.py @@ -100,14 +100,6 @@ def isinf(self) -> bool: """ return math.isinf(self._base_value) - def __hash__(self) -> int: - """Return a hash of this object. - - Returns: - A hash of this object. - """ - return hash((type(self), self._base_value)) - def __repr__(self) -> str: """Return a representation of this quantity. From 247c08186db1b069abf9776a2ef0292a131479bb Mon Sep 17 00:00:00 2001 From: Sahas Subramanian Date: Mon, 24 Jul 2023 17:22:00 +0200 Subject: [PATCH 18/46] Document internal representation of `Quantity` objects as `float`s Closes #531 Signed-off-by: Sahas Subramanian --- src/frequenz/sdk/timeseries/_quantities.py | 66 ++++++++++++++++++++-- 1 file changed, 60 insertions(+), 6 deletions(-) diff --git a/src/frequenz/sdk/timeseries/_quantities.py b/src/frequenz/sdk/timeseries/_quantities.py index 2424c884f..98468c92f 100644 --- a/src/frequenz/sdk/timeseries/_quantities.py +++ b/src/frequenz/sdk/timeseries/_quantities.py @@ -393,7 +393,16 @@ class Power( 6: "MW", }, ): - """A power quantity.""" + """A power quantity. + + Objects of this type are wrappers around `float` values. + + The constructors accept a single `float` value, the `as_*()` methods return a + `float` value, and each of the arithmetic operators supported by this type are + actually implemented using floating-point arithmetic. + + So all considerations about floating-point arithmetic apply to this type as well. + """ @classmethod def from_watts(cls, watts: float) -> Self: @@ -557,7 +566,16 @@ class Current( 0: "A", }, ): - """A current quantity.""" + """A current quantity. + + Objects of this type are wrappers around `float` values. + + The constructors accept a single `float` value, the `as_*()` methods return a + `float` value, and each of the arithmetic operators supported by this type are + actually implemented using floating-point arithmetic. + + So all considerations about floating-point arithmetic apply to this type as well. + """ @classmethod def from_amperes(cls, amperes: float) -> Self: @@ -644,7 +662,16 @@ class Voltage( metaclass=_NoDefaultConstructible, exponent_unit_map={0: "V", -3: "mV", 3: "kV"}, ): - """A voltage quantity.""" + """A voltage quantity. + + Objects of this type are wrappers around `float` values. + + The constructors accept a single `float` value, the `as_*()` methods return a + `float` value, and each of the arithmetic operators supported by this type are + actually implemented using floating-point arithmetic. + + So all considerations about floating-point arithmetic apply to this type as well. + """ @classmethod def from_volts(cls, volts: float) -> Self: @@ -757,7 +784,16 @@ class Energy( 6: "MWh", }, ): - """An energy quantity.""" + """An energy quantity. + + Objects of this type are wrappers around `float` values. + + The constructors accept a single `float` value, the `as_*()` methods return a + `float` value, and each of the arithmetic operators supported by this type are + actually implemented using floating-point arithmetic. + + So all considerations about floating-point arithmetic apply to this type as well. + """ @classmethod def from_watt_hours(cls, watt_hours: float) -> Self: @@ -867,7 +903,16 @@ class Frequency( metaclass=_NoDefaultConstructible, exponent_unit_map={0: "Hz", 3: "kHz", 6: "MHz", 9: "GHz"}, ): - """A frequency quantity.""" + """A frequency quantity. + + Objects of this type are wrappers around `float` values. + + The constructors accept a single `float` value, the `as_*()` methods return a + `float` value, and each of the arithmetic operators supported by this type are + actually implemented using floating-point arithmetic. + + So all considerations about floating-point arithmetic apply to this type as well. + """ @classmethod def from_hertz(cls, hertz: float) -> Self: @@ -971,7 +1016,16 @@ class Percentage( metaclass=_NoDefaultConstructible, exponent_unit_map={0: "%"}, ): - """A percentage quantity.""" + """A percentage quantity. + + Objects of this type are wrappers around `float` values. + + The constructors accept a single `float` value, the `as_*()` methods return a + `float` value, and each of the arithmetic operators supported by this type are + actually implemented using floating-point arithmetic. + + So all considerations about floating-point arithmetic apply to this type as well. + """ @classmethod def from_percent(cls, percent: float) -> Self: From ee43f4934448578f501de5f1ce1f0cae2a743afb Mon Sep 17 00:00:00 2001 From: Sahas Subramanian Date: Mon, 24 Jul 2023 17:36:17 +0200 Subject: [PATCH 19/46] Add `isclose()` method for `Quantity` objects This allows us to compare floating point values without having to use the equality operator. Signed-off-by: Sahas Subramanian --- src/frequenz/sdk/timeseries/_quantities.py | 18 ++++++++++++++++++ tests/timeseries/test_quantities.py | 6 ++++++ 2 files changed, 24 insertions(+) diff --git a/src/frequenz/sdk/timeseries/_quantities.py b/src/frequenz/sdk/timeseries/_quantities.py index 98468c92f..c7fff9b1f 100644 --- a/src/frequenz/sdk/timeseries/_quantities.py +++ b/src/frequenz/sdk/timeseries/_quantities.py @@ -100,6 +100,24 @@ def isinf(self) -> bool: """ return math.isinf(self._base_value) + def isclose(self, other: Self, rel_tol: float = 1e-9, abs_tol: float = 0.0) -> bool: + """Return whether this quantity is close to another. + + Args: + other: The quantity to compare to. + rel_tol: The relative tolerance. + abs_tol: The absolute tolerance. + + Returns: + Whether this quantity is close to another. + """ + return math.isclose( + self._base_value, + other._base_value, # pylint: disable=protected-access + rel_tol=rel_tol, + abs_tol=abs_tol, + ) + def __repr__(self) -> str: """Return a representation of this quantity. diff --git a/tests/timeseries/test_quantities.py b/tests/timeseries/test_quantities.py index f6a4c161f..a7b3d02bf 100644 --- a/tests/timeseries/test_quantities.py +++ b/tests/timeseries/test_quantities.py @@ -91,6 +91,12 @@ def test_string_representation() -> None: assert f"{Fz1(-20000)}" == "-20 kHz" +def test_isclose() -> None: + """Test the isclose method of the quantities.""" + assert Fz1(1.024445).isclose(Fz1(1.024445)) + assert not Fz1(1.024445).isclose(Fz1(1.0)) + + def test_addition_subtraction() -> None: """Test the addition and subtraction of the quantities.""" assert Quantity(1) + Quantity(1, exponent=0) == Quantity(2, exponent=0) From bc37e7cd7ae5f2dde5092f4a879cbcfe6ef969be Mon Sep 17 00:00:00 2001 From: Sahas Subramanian Date: Tue, 25 Jul 2023 15:42:58 +0200 Subject: [PATCH 20/46] Add RELEASE_NOTES entries Signed-off-by: Sahas Subramanian --- RELEASE_NOTES.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 89c97ab6c..a6595d094 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -9,13 +9,15 @@ - `Channels` has been upgraded to version 0.16.0, for information on how to upgrade visit https://github.com/frequenz-floss/frequenz-channels-python/releases/tag/v0.16.0 +- `Quantity` objects are no longer hashable. This is because of the pitfalls of hashing `float` values. ## New Features - Add `abs()` support for quantities. - Add quantity class `Frequency` for frequency values. - Quantities can now be multiplied with `Percentage` types. -- `FormulaEngine` arithmetics now supports scalar multiplication with floats and addition with Quantities +- `FormulaEngine` arithmetics now supports scalar multiplication with floats and addition with Quantities. +- Add a `isclose()` method on quantities to compare them to other values of the same type. Because `Quantity` types are just wrappers around `float`s, direct comparison might not always be desirable. ## Bug Fixes From 6d4f2ad56ea360b04471778d067fd69687539fdc Mon Sep 17 00:00:00 2001 From: Sahas Subramanian Date: Mon, 10 Jul 2023 16:03:48 +0200 Subject: [PATCH 21/46] Rewrite component data wrapper classes to not redefine data fields These classes were redefining all parameters, and that had the risk of going out of sync with the fields in the base class. This PR ties the wrappers with their base classes, such that it is easy to identify changes to the base classes, without updates to the wrappers. Signed-off-by: Sahas Subramanian --- tests/utils/component_data_wrapper.py | 167 ++++++++++++++++++++------ 1 file changed, 127 insertions(+), 40 deletions(-) diff --git a/tests/utils/component_data_wrapper.py b/tests/utils/component_data_wrapper.py index e83db3af1..6339fda88 100644 --- a/tests/utils/component_data_wrapper.py +++ b/tests/utils/component_data_wrapper.py @@ -13,7 +13,7 @@ from __future__ import annotations import math -from dataclasses import dataclass, field, replace +from dataclasses import dataclass, replace from datetime import datetime from typing import Tuple @@ -30,24 +30,47 @@ ) -@dataclass(frozen=True) class BatteryDataWrapper(BatteryData): """Wrapper for the BatteryData with default arguments.""" - soc: float = math.nan - soc_lower_bound: float = math.nan - soc_upper_bound: float = math.nan - capacity: float = math.nan - power_lower_bound: float = math.nan - power_upper_bound: float = math.nan - temperature_max: float = math.nan - _relay_state: battery_pb.RelayState.ValueType = ( - battery_pb.RelayState.RELAY_STATE_UNSPECIFIED - ) - _component_state: battery_pb.ComponentState.ValueType = ( - battery_pb.ComponentState.COMPONENT_STATE_UNSPECIFIED - ) - _errors: list[battery_pb.Error] = field(default_factory=list) + def __init__( # pylint: disable=too-many-arguments + self, + component_id: int, + timestamp: datetime, + soc: float = math.nan, + soc_lower_bound: float = math.nan, + soc_upper_bound: float = math.nan, + capacity: float = math.nan, + power_lower_bound: float = math.nan, + power_upper_bound: float = math.nan, + temperature_max: float = math.nan, + _relay_state: battery_pb.RelayState.ValueType = ( + battery_pb.RelayState.RELAY_STATE_UNSPECIFIED + ), + _component_state: battery_pb.ComponentState.ValueType = ( + battery_pb.ComponentState.COMPONENT_STATE_UNSPECIFIED + ), + _errors: list[battery_pb.Error] | None = None, + ) -> None: + """Initialize the BatteryDataWrapper. + + This is a wrapper for the BatteryData with default arguments. The parameters are + documented in the BatteryData class. + """ + super().__init__( + component_id=component_id, + timestamp=timestamp, + soc=soc, + soc_lower_bound=soc_lower_bound, + soc_upper_bound=soc_upper_bound, + capacity=capacity, + power_lower_bound=power_lower_bound, + power_upper_bound=power_upper_bound, + temperature_max=temperature_max, + _relay_state=_relay_state, + _component_state=_component_state, + _errors=_errors if _errors else [], + ) def copy_with_new_timestamp(self, new_timestamp: datetime) -> BatteryDataWrapper: """Copy the component data but insert new timestamp. @@ -68,13 +91,32 @@ def copy_with_new_timestamp(self, new_timestamp: datetime) -> BatteryDataWrapper class InverterDataWrapper(InverterData): """Wrapper for the InverterData with default arguments.""" - active_power: float = math.nan - active_power_lower_bound: float = math.nan - active_power_upper_bound: float = math.nan - _component_state: inverter_pb.ComponentState.ValueType = ( - inverter_pb.ComponentState.COMPONENT_STATE_UNSPECIFIED - ) - _errors: list[inverter_pb.Error] = field(default_factory=list) + def __init__( # pylint: disable=too-many-arguments + self, + component_id: int, + timestamp: datetime, + active_power: float = math.nan, + active_power_lower_bound: float = math.nan, + active_power_upper_bound: float = math.nan, + _component_state: inverter_pb.ComponentState.ValueType = ( + inverter_pb.ComponentState.COMPONENT_STATE_UNSPECIFIED + ), + _errors: list[inverter_pb.Error] | None = None, + ) -> None: + """Initialize the InverterDataWrapper. + + This is a wrapper for the InverterData with default arguments. The parameters + are documented in the InverterData class. + """ + super().__init__( + component_id=component_id, + timestamp=timestamp, + active_power=active_power, + active_power_lower_bound=active_power_lower_bound, + active_power_upper_bound=active_power_upper_bound, + _component_state=_component_state, + _errors=_errors if _errors else [], + ) def copy_with_new_timestamp(self, new_timestamp: datetime) -> InverterDataWrapper: """Copy the component data but insert new timestamp. @@ -95,15 +137,38 @@ def copy_with_new_timestamp(self, new_timestamp: datetime) -> InverterDataWrappe class EvChargerDataWrapper(EVChargerData): """Wrapper for the EvChargerData with default arguments.""" - active_power: float = math.nan - current_per_phase: Tuple[float, float, float] = field( - default_factory=lambda: (math.nan, math.nan, math.nan) - ) - voltage_per_phase: Tuple[float, float, float] = field( - default_factory=lambda: (math.nan, math.nan, math.nan) - ) - cable_state: EVChargerCableState = EVChargerCableState.UNSPECIFIED - component_state: EVChargerComponentState = EVChargerComponentState.UNSPECIFIED + def __init__( # pylint: disable=too-many-arguments + self, + component_id: int, + timestamp: datetime, + active_power: float = math.nan, + current_per_phase: Tuple[float, float, float] | None = None, + voltage_per_phase: Tuple[float, float, float] | None = None, + cable_state: EVChargerCableState = EVChargerCableState.UNSPECIFIED, + component_state: EVChargerComponentState = EVChargerComponentState.UNSPECIFIED, + ) -> None: + """Initialize the EvChargerDataWrapper. + + This is a wrapper for the EvChargerData with default arguments. The parameters + are documented in the EvChargerData class. + """ + super().__init__( + component_id=component_id, + timestamp=timestamp, + active_power=active_power, + current_per_phase=( + current_per_phase + if current_per_phase + else (math.nan, math.nan, math.nan) + ), + voltage_per_phase=( + voltage_per_phase + if voltage_per_phase + else (math.nan, math.nan, math.nan) + ), + cable_state=cable_state, + component_state=component_state, + ) def copy_with_new_timestamp(self, new_timestamp: datetime) -> EvChargerDataWrapper: """Copy the component data but insert new timestamp. @@ -124,14 +189,36 @@ def copy_with_new_timestamp(self, new_timestamp: datetime) -> EvChargerDataWrapp class MeterDataWrapper(MeterData): """Wrapper for the MeterData with default arguments.""" - active_power: float = math.nan - current_per_phase: Tuple[float, float, float] = field( - default_factory=lambda: (math.nan, math.nan, math.nan) - ) - voltage_per_phase: Tuple[float, float, float] = field( - default_factory=lambda: (math.nan, math.nan, math.nan) - ) - frequency: float = math.nan + def __init__( # pylint: disable=too-many-arguments + self, + component_id: int, + timestamp: datetime, + active_power: float = math.nan, + current_per_phase: Tuple[float, float, float] | None = None, + voltage_per_phase: Tuple[float, float, float] | None = None, + frequency: float = math.nan, + ) -> None: + """Initialize the MeterDataWrapper. + + This is a wrapper for the MeterData with default arguments. The parameters are + documented in the MeterData class. + """ + super().__init__( + component_id=component_id, + timestamp=timestamp, + active_power=active_power, + current_per_phase=( + current_per_phase + if current_per_phase + else (math.nan, math.nan, math.nan) + ), + voltage_per_phase=( + voltage_per_phase + if voltage_per_phase + else (math.nan, math.nan, math.nan) + ), + frequency=frequency, + ) def copy_with_new_timestamp(self, new_timestamp: datetime) -> MeterDataWrapper: """Copy the component data but insert new timestamp. From 0efb86aed6ed16564fd2e2bd8bbec163e09f015a Mon Sep 17 00:00:00 2001 From: Sahas Subramanian Date: Wed, 5 Jul 2023 15:31:58 +0200 Subject: [PATCH 22/46] Allow MockMicrogrid battery chains without meters In most component graph configs, batteries would be attached to inverters and the inverters would be meters. But there might be some cases where there are no meters, and we'll have to read data from the inverters. This commit enables such test configs, that can be used in tests that use the MockMicrogrid. Signed-off-by: Sahas Subramanian --- tests/timeseries/mock_microgrid.py | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/tests/timeseries/mock_microgrid.py b/tests/timeseries/mock_microgrid.py index 0377a64a0..b09bf2145 100644 --- a/tests/timeseries/mock_microgrid.py +++ b/tests/timeseries/mock_microgrid.py @@ -251,11 +251,12 @@ def add_chps(self, count: int) -> None: self._connections.add(Connection(self._connect_to, meter_id)) self._connections.add(Connection(meter_id, chp_id)) - def add_batteries(self, count: int) -> None: + def add_batteries(self, count: int, no_meter: bool = False) -> None: """Add batteries with connected inverters and meters to the microgrid. Args: count: number of battery sets to add. + no_meter: if True, do not add a meter for each battery set. """ for _ in range(count): meter_id = self._id_increment * 10 + self.meter_id_suffix @@ -263,17 +264,10 @@ def add_batteries(self, count: int) -> None: bat_id = self._id_increment * 10 + self.battery_id_suffix self._id_increment += 1 - self.meter_ids.append(meter_id) self.battery_inverter_ids.append(inv_id) self.battery_ids.append(bat_id) self.bat_inv_map[bat_id] = inv_id - self._components.add( - Component( - meter_id, - ComponentCategory.METER, - ) - ) self._components.add( Component(inv_id, ComponentCategory.INVERTER, InverterType.BATTERY) ) @@ -285,9 +279,20 @@ def add_batteries(self, count: int) -> None: ) self._start_battery_streaming(bat_id) self._start_inverter_streaming(inv_id) - self._start_meter_streaming(meter_id) - self._connections.add(Connection(self._connect_to, meter_id)) - self._connections.add(Connection(meter_id, inv_id)) + + if no_meter: + self._connections.add(Connection(self._connect_to, inv_id)) + else: + self.meter_ids.append(meter_id) + self._components.add( + Component( + meter_id, + ComponentCategory.METER, + ) + ) + self._start_meter_streaming(meter_id) + self._connections.add(Connection(self._connect_to, meter_id)) + self._connections.add(Connection(meter_id, inv_id)) self._connections.add(Connection(inv_id, bat_id)) def add_solar_inverters(self, count: int) -> None: From 4803deb18ed1ecacf30689cb05a2c66edd8b392b Mon Sep 17 00:00:00 2001 From: Sahas Subramanian Date: Wed, 5 Jul 2023 15:43:28 +0200 Subject: [PATCH 23/46] Update logical meter grid power test to use a battery without a meter This is the grid power test for component configs without a grid side meter, so it will have to read values from the inverter, to calculate the grid power. Signed-off-by: Sahas Subramanian --- tests/timeseries/test_logical_meter.py | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/tests/timeseries/test_logical_meter.py b/tests/timeseries/test_logical_meter.py index f39c89add..97f654c5f 100644 --- a/tests/timeseries/test_logical_meter.py +++ b/tests/timeseries/test_logical_meter.py @@ -63,31 +63,35 @@ async def test_grid_power_2( ) -> None: """Test the grid power formula without a grid side meter.""" mockgrid = MockMicrogrid(grid_side_meter=False) - mockgrid.add_batteries(2) + mockgrid.add_batteries(1, no_meter=False) + mockgrid.add_batteries(1, no_meter=True) mockgrid.add_solar_inverters(1) await mockgrid.start(mocker) logical_meter = microgrid.logical_meter() grid_power_recv = logical_meter.grid_power.new_receiver() - meter_receivers = [ + component_receivers = [ get_resampled_stream( logical_meter._namespace, # pylint: disable=protected-access - meter_id, + component_id, ComponentMetricId.ACTIVE_POWER, Power.from_watts, ) - for meter_id in mockgrid.meter_ids + for component_id in [ + *mockgrid.meter_ids, + # The last battery has no meter, so we get the power from the inverter + mockgrid.battery_inverter_ids[-1], + ] ] results: list[Quantity] = [] meter_sums: list[Quantity] = [] for count in range(10): - await mockgrid.mock_resampler.send_meter_power( - [20.0 + count, 12.0, -13.0, -5.0] - ) + await mockgrid.mock_resampler.send_meter_power([20.0 + count, 12.0, -13.0]) + await mockgrid.mock_resampler.send_bat_inverter_power([0.0, -5.0]) meter_sum = 0.0 - for recv in meter_receivers: + for recv in component_receivers: val = await recv.receive() assert ( val is not None From 799056744ff724f9741110c9cab72a635b2e21fa Mon Sep 17 00:00:00 2001 From: Sahas Subramanian Date: Wed, 5 Jul 2023 16:07:23 +0200 Subject: [PATCH 24/46] Update PowerDistributingActor constructor test to use MockMicrogrid Signed-off-by: Sahas Subramanian --- tests/actor/test_power_distributing.py | 37 ++++++++++++++++++-------- 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/tests/actor/test_power_distributing.py b/tests/actor/test_power_distributing.py index 6f652f2ed..15c5d67ab 100644 --- a/tests/actor/test_power_distributing.py +++ b/tests/actor/test_power_distributing.py @@ -30,6 +30,7 @@ ) from frequenz.sdk.microgrid.client import Connection from frequenz.sdk.microgrid.component import Component, ComponentCategory +from tests.timeseries.mock_microgrid import MockMicrogrid from ..conftest import SAFETY_TIMEOUT from ..power.test_distribution_algorithm import Bound, Metric, battery_msg, inverter_msg @@ -81,15 +82,10 @@ def component_graph(self) -> tuple[set[Component], set[Connection]]: async def test_constructor(self, mocker: MockerFixture) -> None: """Test if gets all necessary data.""" - components, connections = self.component_graph() - mock_microgrid = MockMicrogridClient(components, connections) - mock_microgrid.initialize(mocker) - - attrs = {"get_working_batteries.return_value": {306}} - mocker.patch( - "frequenz.sdk.actor.power_distributing.power_distributing.BatteryPoolStatus", - return_value=MagicMock(spec=BatteryPoolStatus, **attrs), - ) + mockgrid = MockMicrogrid(grid_side_meter=True) + mockgrid.add_batteries(2) + mockgrid.add_batteries(1, no_meter=True) + await mockgrid.start(mocker) channel = Broadcast[Request]("power_distributor") channel_registry = ChannelRegistry(name="power_distributor") @@ -100,9 +96,28 @@ async def test_constructor(self, mocker: MockerFixture) -> None: battery_status_sender=battery_status_channel.new_sender(), ) - assert distributor._bat_inv_map == {106: 105, 206: 205, 306: 305} - assert distributor._inv_bat_map == {105: 106, 205: 206, 305: 306} + assert distributor._bat_inv_map == {9: 8, 19: 18, 29: 28} + assert distributor._inv_bat_map == {8: 9, 18: 19, 28: 29} + + await distributor._stop_actor() + await mockgrid.cleanup() + + # Test if it works without grid side meter + mockgrid = MockMicrogrid(grid_side_meter=False) + mockgrid.add_batteries(1) + mockgrid.add_batteries(2, no_meter=True) + await mockgrid.start(mocker) + distributor = PowerDistributingActor( + requests_receiver=channel.new_receiver(), + channel_registry=channel_registry, + battery_status_sender=battery_status_channel.new_sender(), + ) + + assert distributor._bat_inv_map == {9: 8, 19: 18, 29: 28} + assert distributor._inv_bat_map == {8: 9, 18: 19, 28: 29} + await distributor._stop_actor() + await mockgrid.cleanup() async def init_mock_microgrid(self, mocker: MockerFixture) -> MockMicrogridClient: """Create mock microgrid and send initial data from the components. From ccb9e29f63eee9f70661c41cd3a4c316da723621 Mon Sep 17 00:00:00 2001 From: Sahas Subramanian Date: Wed, 5 Jul 2023 17:52:07 +0200 Subject: [PATCH 25/46] Expose internal mock_client to users of MockMicrogrid Signed-off-by: Sahas Subramanian --- tests/timeseries/mock_microgrid.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/tests/timeseries/mock_microgrid.py b/tests/timeseries/mock_microgrid.py index b09bf2145..29ea93b9e 100644 --- a/tests/timeseries/mock_microgrid.py +++ b/tests/timeseries/mock_microgrid.py @@ -48,7 +48,7 @@ class MockMicrogrid: # pylint: disable=too-many-instance-attributes inverter_id_suffix = 8 battery_id_suffix = 9 - _microgrid: MockMicrogridClient + mock_client: MockMicrogridClient mock_resampler: MockResampler def __init__( # pylint: disable=too-many-arguments @@ -123,8 +123,8 @@ def init_mock_client( self, initialize_cb: Callable[[MockMicrogridClient], None] ) -> None: """Set up the mock client. Does not start the streaming tasks.""" - self._microgrid = MockMicrogridClient(self._components, self._connections) - initialize_cb(self._microgrid) + self.mock_client = MockMicrogridClient(self._components, self._connections) + initialize_cb(self.mock_client) def start_mock_client( self, initialize_cb: Callable[[MockMicrogridClient], None] @@ -146,7 +146,7 @@ def start_mock_client( self._streaming_tasks = [ asyncio.create_task(coro) for coro in self._streaming_coros ] - return self._microgrid + return self.mock_client async def _comp_data_send_task( self, comp_id: int, make_comp_data: Callable[[int, datetime], ComponentData] @@ -157,12 +157,12 @@ async def _comp_data_send_task( # for inverters with component_id > 100, send only half the messages. if comp_id % 10 == self.inverter_id_suffix: if comp_id < 100 or value <= 5: - await self._microgrid.send(make_comp_data(val_to_send, timestamp)) + await self.mock_client.send(make_comp_data(val_to_send, timestamp)) else: - await self._microgrid.send(make_comp_data(val_to_send, timestamp)) + await self.mock_client.send(make_comp_data(val_to_send, timestamp)) await asyncio.sleep(self._sample_rate_s) - await self._microgrid.close_channel(comp_id) + await self.mock_client.close_channel(comp_id) def _start_meter_streaming(self, meter_id: int) -> None: if not self._api_client_streaming: @@ -359,7 +359,7 @@ async def send_meter_data(self, values: list[float]) -> None: assert len(values) == len(self.meter_ids) timestamp = datetime.now(tz=timezone.utc) for comp_id, value in zip(self.meter_ids, values): - await self._microgrid.send( + await self.mock_client.send( MeterDataWrapper( component_id=comp_id, timestamp=timestamp, @@ -381,7 +381,7 @@ async def send_battery_data(self, socs: list[float]) -> None: assert len(socs) == len(self.battery_ids) timestamp = datetime.now(tz=timezone.utc) for comp_id, value in zip(self.battery_ids, socs): - await self._microgrid.send( + await self.mock_client.send( BatteryDataWrapper(component_id=comp_id, timestamp=timestamp, soc=value) ) @@ -394,7 +394,7 @@ async def send_battery_inverter_data(self, values: list[float]) -> None: assert len(values) == len(self.battery_inverter_ids) timestamp = datetime.now(tz=timezone.utc) for comp_id, value in zip(self.battery_inverter_ids, values): - await self._microgrid.send( + await self.mock_client.send( InverterDataWrapper( component_id=comp_id, timestamp=timestamp, active_power=value ) @@ -409,7 +409,7 @@ async def send_pv_inverter_data(self, values: list[float]) -> None: assert len(values) == len(self.pv_inverter_ids) timestamp = datetime.now(tz=timezone.utc) for comp_id, value in zip(self.pv_inverter_ids, values): - await self._microgrid.send( + await self.mock_client.send( InverterDataWrapper( component_id=comp_id, timestamp=timestamp, active_power=value ) @@ -424,7 +424,7 @@ async def send_ev_charger_data(self, values: list[float]) -> None: assert len(values) == len(self.evc_ids) timestamp = datetime.now(tz=timezone.utc) for comp_id, value in zip(self.evc_ids, values): - await self._microgrid.send( + await self.mock_client.send( EvChargerDataWrapper( component_id=comp_id, timestamp=timestamp, From 691d6a5d2b9b07220355e1460f94845a6e4d9707 Mon Sep 17 00:00:00 2001 From: Sahas Subramanian Date: Mon, 10 Jul 2023 16:40:35 +0200 Subject: [PATCH 26/46] Use constant BATTERY_ID instead of hardcoded number Signed-off-by: Sahas Subramanian --- tests/actor/test_battery_status.py | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/tests/actor/test_battery_status.py b/tests/actor/test_battery_status.py index b0e7c46da..ade998e0d 100644 --- a/tests/actor/test_battery_status.py +++ b/tests/actor/test_battery_status.py @@ -378,7 +378,7 @@ async def test_sync_blocking_feature( # message is not correct, component should not block. tracker._handle_status_set_power_result( - SetPowerResult(succeed={1}, failed={106}) + SetPowerResult(succeed={1}, failed={BATTERY_ID}) ) assert tracker._get_new_status_if_changed() is None @@ -392,7 +392,7 @@ async def test_sync_blocking_feature( for timeout in expected_blocking_timeout: # message is not correct, component should not block. tracker._handle_status_set_power_result( - SetPowerResult(succeed={1}, failed={106}) + SetPowerResult(succeed={1}, failed={BATTERY_ID}) ) assert tracker._get_new_status_if_changed() is Status.UNCERTAIN @@ -400,7 +400,7 @@ async def test_sync_blocking_feature( # Battery should be still blocked, nothing should happen time.shift(timeout - 1) tracker._handle_status_set_power_result( - SetPowerResult(succeed={1}, failed={106}) + SetPowerResult(succeed={1}, failed={BATTERY_ID}) ) assert tracker._get_new_status_if_changed() is None @@ -416,7 +416,7 @@ async def test_sync_blocking_feature( # should block for 30 sec tracker._handle_status_set_power_result( - SetPowerResult(succeed={1}, failed={106}) + SetPowerResult(succeed={1}, failed={BATTERY_ID}) ) assert tracker._get_new_status_if_changed() is Status.UNCERTAIN @@ -437,14 +437,14 @@ async def test_sync_blocking_feature( # should block for 30 sec tracker._handle_status_set_power_result( - SetPowerResult(succeed={1}, failed={106}) + SetPowerResult(succeed={1}, failed={BATTERY_ID}) ) assert tracker._get_new_status_if_changed() is Status.UNCERTAIN time.shift(28) # If battery succeed, then it should unblock. tracker._handle_status_set_power_result( - SetPowerResult(succeed={106}, failed={206}) + SetPowerResult(succeed={BATTERY_ID}, failed={206}) ) assert tracker._get_new_status_if_changed() is Status.WORKING @@ -481,7 +481,7 @@ async def test_sync_blocking_interrupted_with_with_max_data( assert tracker._get_new_status_if_changed() is Status.WORKING tracker._handle_status_set_power_result( - SetPowerResult(succeed={1}, failed={106}) + SetPowerResult(succeed={1}, failed={BATTERY_ID}) ) assert tracker._get_new_status_if_changed() is Status.UNCERTAIN @@ -489,7 +489,7 @@ async def test_sync_blocking_interrupted_with_with_max_data( for timeout in expected_blocking_timeout: # message is not correct, component should not block. tracker._handle_status_set_power_result( - SetPowerResult(succeed={1}, failed={106}) + SetPowerResult(succeed={1}, failed={BATTERY_ID}) ) assert tracker._get_new_status_if_changed() is None time.shift(timeout) @@ -527,7 +527,7 @@ async def test_sync_blocking_interrupted_with_invalid_message( assert tracker._get_new_status_if_changed() is Status.WORKING tracker._handle_status_set_power_result( - SetPowerResult(succeed={1}, failed={106}) + SetPowerResult(succeed={1}, failed={BATTERY_ID}) ) assert tracker._get_new_status_if_changed() is Status.UNCERTAIN @@ -540,12 +540,12 @@ async def test_sync_blocking_interrupted_with_invalid_message( assert tracker._get_new_status_if_changed() is Status.NOT_WORKING tracker._handle_status_set_power_result( - SetPowerResult(succeed={1}, failed={106}) + SetPowerResult(succeed={1}, failed={BATTERY_ID}) ) assert tracker._get_new_status_if_changed() is None tracker._handle_status_set_power_result( - SetPowerResult(succeed={106}, failed={}) + SetPowerResult(succeed={BATTERY_ID}, failed={}) ) assert tracker._get_new_status_if_changed() is None From 1c8f58b5fe60ee9ba66a5af2efc8c5eac4b05ff7 Mon Sep 17 00:00:00 2001 From: Sahas Subramanian Date: Thu, 6 Jul 2023 13:37:00 +0200 Subject: [PATCH 27/46] Update PowerDistributingActor tests to use MockMicrogrid MockMicrogrid makes it easy to dynamically create component graph configurations to test with, rather than having to use manually written component graphs with the lower level MockMicrogridClient. Signed-off-by: Sahas Subramanian --- tests/actor/test_battery_pool_status.py | 41 ++++-- tests/actor/test_battery_status.py | 63 ++++++--- tests/actor/test_power_distributing.py | 177 +++++++++++++----------- 3 files changed, 171 insertions(+), 110 deletions(-) diff --git a/tests/actor/test_battery_pool_status.py b/tests/actor/test_battery_pool_status.py index f92b47f15..8bca3fb25 100644 --- a/tests/actor/test_battery_pool_status.py +++ b/tests/actor/test_battery_pool_status.py @@ -14,6 +14,7 @@ BatteryStatus, ) from frequenz.sdk.microgrid.component import ComponentCategory +from tests.timeseries.mock_microgrid import MockMicrogrid from ..utils.mock_microgrid_client import MockMicrogridClient from .test_battery_status import battery_data, component_graph, inverter_data @@ -38,7 +39,7 @@ async def mock_microgrid(self, mocker: MockerFixture) -> MockMicrogridClient: microgrid.initialize(mocker) return microgrid - async def test_batteries_status(self, mock_microgrid: MockMicrogridClient) -> None: + async def test_batteries_status(self, mocker: MockerFixture) -> None: """Basic tests for BatteryPoolStatus. BatteryStatusTracker is more tested in its own unit tests. @@ -46,9 +47,13 @@ async def test_batteries_status(self, mock_microgrid: MockMicrogridClient) -> No Args: mock_microgrid: mock microgrid client """ + mock_microgrid = MockMicrogrid(grid_side_meter=True) + mock_microgrid.add_batteries(3) + await mock_microgrid.start(mocker) + batteries = { battery.component_id - for battery in mock_microgrid.component_graph.components( + for battery in mock_microgrid.mock_client.component_graph.components( component_category={ComponentCategory.BATTERY} ) } @@ -67,22 +72,34 @@ async def test_batteries_status(self, mock_microgrid: MockMicrogridClient) -> No batteries_list = list(batteries) - await mock_microgrid.send(battery_data(component_id=batteries_list[0])) + await mock_microgrid.mock_client.send( + battery_data(component_id=batteries_list[0]) + ) await asyncio.sleep(0.1) assert batteries_status.get_working_batteries(batteries) == expected_working expected_working.add(batteries_list[0]) - await mock_microgrid.send(inverter_data(component_id=batteries_list[0] - 1)) + await mock_microgrid.mock_client.send( + inverter_data(component_id=batteries_list[0] - 1) + ) await asyncio.sleep(0.1) assert batteries_status.get_working_batteries(batteries) == expected_working msg = await asyncio.wait_for(battery_status_recv.receive(), timeout=0.2) assert msg == batteries_status._current_status - await mock_microgrid.send(inverter_data(component_id=batteries_list[1] - 1)) - await mock_microgrid.send(battery_data(component_id=batteries_list[1])) + await mock_microgrid.mock_client.send( + inverter_data(component_id=batteries_list[1] - 1) + ) + await mock_microgrid.mock_client.send( + battery_data(component_id=batteries_list[1]) + ) - await mock_microgrid.send(inverter_data(component_id=batteries_list[2] - 1)) - await mock_microgrid.send(battery_data(component_id=batteries_list[2])) + await mock_microgrid.mock_client.send( + inverter_data(component_id=batteries_list[2] - 1) + ) + await mock_microgrid.mock_client.send( + battery_data(component_id=batteries_list[2]) + ) expected_working = set(batteries_list) await asyncio.sleep(0.1) @@ -91,15 +108,15 @@ async def test_batteries_status(self, mock_microgrid: MockMicrogridClient) -> No assert msg == batteries_status._current_status await batteries_status.update_status( - succeed_batteries={106}, failed_batteries={206, 306} + succeed_batteries={9}, failed_batteries={19, 29} ) await asyncio.sleep(0.1) - assert batteries_status.get_working_batteries(batteries) == {106} + assert batteries_status.get_working_batteries(batteries) == {9} await batteries_status.update_status( - succeed_batteries={106, 206}, failed_batteries=set() + succeed_batteries={9, 19}, failed_batteries=set() ) await asyncio.sleep(0.1) - assert batteries_status.get_working_batteries(batteries) == {106, 206} + assert batteries_status.get_working_batteries(batteries) == {9, 19} await batteries_status.stop() diff --git a/tests/actor/test_battery_status.py b/tests/actor/test_battery_status.py index ade998e0d..6a146b3d4 100644 --- a/tests/actor/test_battery_status.py +++ b/tests/actor/test_battery_status.py @@ -33,6 +33,7 @@ ComponentCategory, InverterData, ) +from tests.timeseries.mock_microgrid import MockMicrogrid from ..utils.component_data_wrapper import BatteryDataWrapper, InverterDataWrapper from ..utils.mock_microgrid_client import MockMicrogridClient @@ -154,8 +155,8 @@ class Message(Generic[T]): inner: T -BATTERY_ID = 106 -INVERTER_ID = 105 +BATTERY_ID = 9 +INVERTER_ID = 8 # pylint: disable=protected-access, unused-argument @@ -179,7 +180,7 @@ async def mock_microgrid(self, mocker: MockerFixture) -> MockMicrogridClient: @time_machine.travel("2022-01-01 00:00 UTC", tick=False) async def test_sync_update_status_with_messages( - self, mock_microgrid: MockMicrogridClient + self, mocker: MockerFixture ) -> None: """Test if messages changes battery status/ @@ -189,6 +190,10 @@ async def test_sync_update_status_with_messages( Args: mock_microgrid: mock_microgrid fixture """ + mock_microgrid = MockMicrogrid(grid_side_meter=True) + mock_microgrid.add_batteries(3) + await mock_microgrid.start(mocker) + status_channel = Broadcast[Status]("battery_status") set_power_result_channel = Broadcast[SetPowerResult]("set_power_result") @@ -336,10 +341,9 @@ async def test_sync_update_status_with_messages( assert tracker._get_new_status_if_changed() is Status.NOT_WORKING await tracker.stop() + await mock_microgrid.cleanup() - async def test_sync_blocking_feature( - self, mock_microgrid: MockMicrogridClient - ) -> None: + async def test_sync_blocking_feature(self, mocker: MockerFixture) -> None: """Test if status changes when SetPowerResult message is received. Tests uses FakeSelect to test status in sync way. @@ -348,6 +352,9 @@ async def test_sync_blocking_feature( Args: mock_microgrid: mock_microgrid fixture """ + mock_microgrid = MockMicrogrid(grid_side_meter=True) + mock_microgrid.add_batteries(3) + await mock_microgrid.start(mocker) status_channel = Broadcast[Status]("battery_status") set_power_result_channel = Broadcast[SetPowerResult]("set_power_result") @@ -444,14 +451,15 @@ async def test_sync_blocking_feature( # If battery succeed, then it should unblock. tracker._handle_status_set_power_result( - SetPowerResult(succeed={BATTERY_ID}, failed={206}) + SetPowerResult(succeed={BATTERY_ID}, failed={19}) ) assert tracker._get_new_status_if_changed() is Status.WORKING await tracker.stop() + await mock_microgrid.cleanup() async def test_sync_blocking_interrupted_with_with_max_data( - self, mock_microgrid: MockMicrogridClient + self, mocker: MockerFixture ) -> None: """Test if status changes when SetPowerResult message is received. @@ -461,6 +469,9 @@ async def test_sync_blocking_interrupted_with_with_max_data( Args: mock_microgrid: mock_microgrid fixture """ + mock_microgrid = MockMicrogrid(grid_side_meter=True) + mock_microgrid.add_batteries(3) + await mock_microgrid.start(mocker) status_channel = Broadcast[Status]("battery_status") set_power_result_channel = Broadcast[SetPowerResult]("set_power_result") @@ -498,7 +509,7 @@ async def test_sync_blocking_interrupted_with_with_max_data( @time_machine.travel("2022-01-01 00:00 UTC", tick=False) async def test_sync_blocking_interrupted_with_invalid_message( - self, mock_microgrid: MockMicrogridClient + self, mocker: MockerFixture ) -> None: """Test if status changes when SetPowerResult message is received. @@ -508,6 +519,9 @@ async def test_sync_blocking_interrupted_with_invalid_message( Args: mock_microgrid: mock_microgrid fixture """ + mock_microgrid = MockMicrogrid(grid_side_meter=True) + mock_microgrid.add_batteries(3) + await mock_microgrid.start(mocker) status_channel = Broadcast[Status]("battery_status") set_power_result_channel = Broadcast[SetPowerResult]("set_power_result") @@ -555,9 +569,7 @@ async def test_sync_blocking_interrupted_with_invalid_message( await tracker.stop() @time_machine.travel("2022-01-01 00:00 UTC", tick=False) - async def test_timers( - self, mock_microgrid: MockMicrogridClient, mocker: MockerFixture - ) -> None: + async def test_timers(self, mocker: MockerFixture) -> None: """Test if messages changes battery status/ Tests uses FakeSelect to test status in sync way. @@ -567,6 +579,10 @@ async def test_timers( mock_microgrid: mock_microgrid fixture mocker: pytest mocker instance """ + mock_microgrid = MockMicrogrid(grid_side_meter=True) + mock_microgrid.add_batteries(3) + await mock_microgrid.start(mocker) + status_channel = Broadcast[Status]("battery_status") set_power_result_channel = Broadcast[SetPowerResult]("set_power_result") @@ -616,16 +632,18 @@ async def test_timers( assert inverter_timer_spy.call_count == 2 await tracker.stop() + await mock_microgrid.cleanup() @time_machine.travel("2022-01-01 00:00 UTC", tick=False) - async def test_async_battery_status( - self, mock_microgrid: MockMicrogridClient - ) -> None: + async def test_async_battery_status(self, mocker: MockerFixture) -> None: """Test if status changes. Args: mock_microgrid: mock_microgrid fixture """ + mock_microgrid = MockMicrogrid(grid_side_meter=True) + mock_microgrid.add_batteries(3) + await mock_microgrid.start(mocker) status_channel = Broadcast[Status]("battery_status") set_power_result_channel = Broadcast[SetPowerResult]("set_power_result") @@ -643,8 +661,10 @@ async def test_async_battery_status( await asyncio.sleep(0.01) with time_machine.travel("2022-01-01 00:00 UTC", tick=False) as time: - await mock_microgrid.send(inverter_data(component_id=INVERTER_ID)) - await mock_microgrid.send(battery_data(component_id=BATTERY_ID)) + await mock_microgrid.mock_client.send( + inverter_data(component_id=INVERTER_ID) + ) + await mock_microgrid.mock_client.send(battery_data(component_id=BATTERY_ID)) status = await asyncio.wait_for(status_receiver.receive(), timeout=0.1) assert status is Status.WORKING @@ -656,11 +676,11 @@ async def test_async_battery_status( time.shift(2) - await mock_microgrid.send(battery_data(component_id=BATTERY_ID)) + await mock_microgrid.mock_client.send(battery_data(component_id=BATTERY_ID)) status = await asyncio.wait_for(status_receiver.receive(), timeout=0.1) assert status is Status.WORKING - await mock_microgrid.send( + await mock_microgrid.mock_client.send( inverter_data( component_id=INVERTER_ID, timestamp=datetime.now(tz=timezone.utc) - timedelta(seconds=7), @@ -675,8 +695,11 @@ async def test_async_battery_status( await asyncio.sleep(0.3) assert len(status_receiver) == 0 - await mock_microgrid.send(inverter_data(component_id=INVERTER_ID)) + await mock_microgrid.mock_client.send( + inverter_data(component_id=INVERTER_ID) + ) status = await asyncio.wait_for(status_receiver.receive(), timeout=0.1) assert status is Status.WORKING await tracker.stop() + await mock_microgrid.cleanup() diff --git a/tests/actor/test_power_distributing.py b/tests/actor/test_power_distributing.py index 15c5d67ab..89dfeb130 100644 --- a/tests/actor/test_power_distributing.py +++ b/tests/actor/test_power_distributing.py @@ -15,6 +15,7 @@ from pytest import approx from pytest_mock import MockerFixture +from frequenz.sdk import microgrid from frequenz.sdk.actor import ChannelRegistry from frequenz.sdk.actor.power_distributing import ( BatteryStatus, @@ -34,7 +35,6 @@ from ..conftest import SAFETY_TIMEOUT from ..power.test_distribution_algorithm import Bound, Metric, battery_msg, inverter_msg -from ..utils.mock_microgrid_client import MockMicrogridClient T = TypeVar("T") # Declare type variable @@ -119,19 +119,11 @@ async def test_constructor(self, mocker: MockerFixture) -> None: await distributor._stop_actor() await mockgrid.cleanup() - async def init_mock_microgrid(self, mocker: MockerFixture) -> MockMicrogridClient: - """Create mock microgrid and send initial data from the components. - - Returns: - Mock microgrid instance. - """ - components, connections = self.component_graph() - microgrid = MockMicrogridClient(components, connections) - microgrid.initialize(mocker) - - graph = microgrid.component_graph + async def init_component_data(self, mockgrid: MockMicrogrid) -> None: + """Send initial component data, for power distributor to start.""" + graph = microgrid.connection_manager.get().component_graph for battery in graph.components(component_category={ComponentCategory.BATTERY}): - await microgrid.send( + await mockgrid.mock_client.send( battery_msg( battery.component_id, capacity=Metric(98000), @@ -142,19 +134,19 @@ async def init_mock_microgrid(self, mocker: MockerFixture) -> MockMicrogridClien inverters = graph.components(component_category={ComponentCategory.INVERTER}) for inverter in inverters: - await microgrid.send( + await mockgrid.mock_client.send( inverter_msg( inverter.component_id, power=Bound(-500, 500), ) ) - return microgrid - async def test_power_distributor_one_user(self, mocker: MockerFixture) -> None: - # pylint: disable=too-many-locals """Test if power distribution works with single user works.""" - await self.init_mock_microgrid(mocker) + mockgrid = MockMicrogrid(grid_side_meter=False) + mockgrid.add_batteries(3) + await mockgrid.start(mocker) + await self.init_component_data(mockgrid) channel = Broadcast[Request]("power_distributor") channel_registry = ChannelRegistry(name="power_distributor") @@ -162,7 +154,7 @@ async def test_power_distributor_one_user(self, mocker: MockerFixture) -> None: request = Request( namespace=self._namespace, power=1200.0, - batteries={106, 206}, + batteries={9, 19}, request_timeout_sec=SAFETY_TIMEOUT, ) @@ -199,13 +191,15 @@ async def test_power_distributor_one_user(self, mocker: MockerFixture) -> None: assert result.request == request async def test_battery_soc_nan(self, mocker: MockerFixture) -> None: - # pylint: disable=too-many-locals """Test if battery with SoC==NaN is not used.""" - mock_microgrid = await self.init_mock_microgrid(mocker) + mockgrid = MockMicrogrid(grid_side_meter=False) + mockgrid.add_batteries(3) + await mockgrid.start(mocker) + await self.init_component_data(mockgrid) - await mock_microgrid.send( + await mockgrid.mock_client.send( battery_msg( - 106, + 9, soc=Metric(math.nan, Bound(20, 80)), capacity=Metric(98000), power=Bound(-1000, 1000), @@ -218,7 +212,7 @@ async def test_battery_soc_nan(self, mocker: MockerFixture) -> None: request = Request( namespace=self._namespace, power=1200.0, - batteries={106, 206}, + batteries={9, 19}, request_timeout_sec=SAFETY_TIMEOUT, ) @@ -256,19 +250,21 @@ async def test_battery_soc_nan(self, mocker: MockerFixture) -> None: result: Result = done.pop().result() assert isinstance(result, Success) - assert result.succeeded_batteries == {206} + assert result.succeeded_batteries == {19} assert result.succeeded_power == approx(500.0) assert result.excess_power == approx(700.0) assert result.request == request async def test_battery_capacity_nan(self, mocker: MockerFixture) -> None: - # pylint: disable=too-many-locals """Test battery with capacity set to NaN is not used.""" - mock_microgrid = await self.init_mock_microgrid(mocker) + mockgrid = MockMicrogrid(grid_side_meter=False) + mockgrid.add_batteries(3) + await mockgrid.start(mocker) + await self.init_component_data(mockgrid) - await mock_microgrid.send( + await mockgrid.mock_client.send( battery_msg( - 106, + 9, soc=Metric(40, Bound(20, 80)), capacity=Metric(math.nan), power=Bound(-1000, 1000), @@ -281,7 +277,7 @@ async def test_battery_capacity_nan(self, mocker: MockerFixture) -> None: request = Request( namespace=self._namespace, power=1200.0, - batteries={106, 206}, + batteries={9, 19}, request_timeout_sec=SAFETY_TIMEOUT, ) attrs = {"get_working_batteries.return_value": request.batteries} @@ -312,35 +308,37 @@ async def test_battery_capacity_nan(self, mocker: MockerFixture) -> None: result: Result = done.pop().result() assert isinstance(result, Success) - assert result.succeeded_batteries == {206} + assert result.succeeded_batteries == {19} assert result.succeeded_power == approx(500.0) assert result.excess_power == approx(700.0) assert result.request == request async def test_battery_power_bounds_nan(self, mocker: MockerFixture) -> None: - # pylint: disable=too-many-locals """Test battery with power bounds set to NaN is not used.""" - mock_microgrid = await self.init_mock_microgrid(mocker) + mockgrid = MockMicrogrid(grid_side_meter=False) + mockgrid.add_batteries(3) + await mockgrid.start(mocker) + await self.init_component_data(mockgrid) - # Battery 206 should work even if his inverter sends NaN - await mock_microgrid.send( + # Battery 19 should work even if his inverter sends NaN + await mockgrid.mock_client.send( inverter_msg( - 205, + 18, power=Bound(math.nan, math.nan), ) ) # Battery 106 should not work because both battery and inverter sends NaN - await mock_microgrid.send( + await mockgrid.mock_client.send( inverter_msg( - 105, + 8, power=Bound(-1000, math.nan), ) ) - await mock_microgrid.send( + await mockgrid.mock_client.send( battery_msg( - 106, + 9, soc=Metric(40, Bound(20, 80)), capacity=Metric(float(98000)), power=Bound(math.nan, math.nan), @@ -353,7 +351,7 @@ async def test_battery_power_bounds_nan(self, mocker: MockerFixture) -> None: request = Request( namespace=self._namespace, power=1200.0, - batteries={106, 206}, + batteries={9, 19}, request_timeout_sec=SAFETY_TIMEOUT, ) attrs = {"get_working_batteries.return_value": request.batteries} @@ -384,7 +382,7 @@ async def test_battery_power_bounds_nan(self, mocker: MockerFixture) -> None: result: Result = done.pop().result() assert isinstance(result, Success) - assert result.succeeded_batteries == {206} + assert result.succeeded_batteries == {19} assert result.succeeded_power == approx(1000.0) assert result.excess_power == approx(200.0) assert result.request == request @@ -392,16 +390,18 @@ async def test_battery_power_bounds_nan(self, mocker: MockerFixture) -> None: async def test_power_distributor_invalid_battery_id( self, mocker: MockerFixture ) -> None: - # pylint: disable=too-many-locals """Test if power distribution raises error if any battery id is invalid.""" - await self.init_mock_microgrid(mocker) + mockgrid = MockMicrogrid(grid_side_meter=False) + mockgrid.add_batteries(3) + await mockgrid.start(mocker) + await self.init_component_data(mockgrid) channel = Broadcast[Request]("power_distributor") channel_registry = ChannelRegistry(name="power_distributor") request = Request( namespace=self._namespace, power=1200.0, - batteries={106, 208}, + batteries={9, 100}, request_timeout_sec=SAFETY_TIMEOUT, ) @@ -432,15 +432,17 @@ async def test_power_distributor_invalid_battery_id( result: Result = done.pop().result() assert isinstance(result, Error) assert result.request == request - err_msg = re.search(r"^No battery 208, available batteries:", result.msg) + err_msg = re.search(r"^No battery 100, available batteries:", result.msg) assert err_msg is not None async def test_power_distributor_one_user_adjust_power_consume( self, mocker: MockerFixture ) -> None: - # pylint: disable=too-many-locals """Test if power distribution works with single user works.""" - await self.init_mock_microgrid(mocker) + mockgrid = MockMicrogrid(grid_side_meter=False) + mockgrid.add_batteries(3) + await mockgrid.start(mocker) + await self.init_component_data(mockgrid) channel = Broadcast[Request]("power_distributor") channel_registry = ChannelRegistry(name="power_distributor") @@ -448,7 +450,7 @@ async def test_power_distributor_one_user_adjust_power_consume( request = Request( namespace=self._namespace, power=1200.0, - batteries={106, 206}, + batteries={9, 19}, request_timeout_sec=SAFETY_TIMEOUT, adjust_power=False, ) @@ -489,9 +491,11 @@ async def test_power_distributor_one_user_adjust_power_consume( async def test_power_distributor_one_user_adjust_power_supply( self, mocker: MockerFixture ) -> None: - # pylint: disable=too-many-locals """Test if power distribution works with single user works.""" - await self.init_mock_microgrid(mocker) + mockgrid = MockMicrogrid(grid_side_meter=False) + mockgrid.add_batteries(3) + await mockgrid.start(mocker) + await self.init_component_data(mockgrid) channel = Broadcast[Request]("power_distributor") channel_registry = ChannelRegistry(name="power_distributor") @@ -499,7 +503,7 @@ async def test_power_distributor_one_user_adjust_power_supply( request = Request( namespace=self._namespace, power=-1200.0, - batteries={106, 206}, + batteries={9, 19}, request_timeout_sec=SAFETY_TIMEOUT, adjust_power=False, ) @@ -540,9 +544,11 @@ async def test_power_distributor_one_user_adjust_power_supply( async def test_power_distributor_one_user_adjust_power_success( self, mocker: MockerFixture ) -> None: - # pylint: disable=too-many-locals """Test if power distribution works with single user works.""" - await self.init_mock_microgrid(mocker) + mockgrid = MockMicrogrid(grid_side_meter=False) + mockgrid.add_batteries(3) + await mockgrid.start(mocker) + await self.init_component_data(mockgrid) channel = Broadcast[Request]("power_distributor") channel_registry = ChannelRegistry(name="power_distributor") @@ -550,7 +556,7 @@ async def test_power_distributor_one_user_adjust_power_success( request = Request( namespace=self._namespace, power=1000.0, - batteries={106, 206}, + batteries={9, 19}, request_timeout_sec=SAFETY_TIMEOUT, adjust_power=False, ) @@ -590,13 +596,16 @@ async def test_power_distributor_one_user_adjust_power_success( async def test_not_all_batteries_are_working(self, mocker: MockerFixture) -> None: """Test if power distribution works if not all batteries are working.""" - await self.init_mock_microgrid(mocker) + mockgrid = MockMicrogrid(grid_side_meter=False) + mockgrid.add_batteries(3) + await mockgrid.start(mocker) + await self.init_component_data(mockgrid) mocker.patch("asyncio.sleep", new_callable=AsyncMock) - batteries = {106, 206} + batteries = {9, 19} - attrs = {"get_working_batteries.return_value": batteries - {106}} + attrs = {"get_working_batteries.return_value": batteries - {9}} mocker.patch( "frequenz.sdk.actor.power_distributing.power_distributing.BatteryPoolStatus", return_value=MagicMock(spec=BatteryPoolStatus, **attrs), @@ -630,7 +639,7 @@ async def test_not_all_batteries_are_working(self, mocker: MockerFixture) -> Non assert len(done) == 1 result = done.pop().result() assert isinstance(result, Success) - assert result.succeeded_batteries == {206} + assert result.succeeded_batteries == {19} assert result.excess_power == approx(700.0) assert result.succeeded_power == approx(500.0) assert result.request == request @@ -641,7 +650,10 @@ async def test_use_all_batteries_none_is_working( self, mocker: MockerFixture ) -> None: """Test all batteries are used if none of them works.""" - await self.init_mock_microgrid(mocker) + mockgrid = MockMicrogrid(grid_side_meter=False) + mockgrid.add_batteries(3) + await mockgrid.start(mocker) + await self.init_component_data(mockgrid) mocker.patch("asyncio.sleep", new_callable=AsyncMock) @@ -663,7 +675,7 @@ async def test_use_all_batteries_none_is_working( request = Request( namespace=self._namespace, power=1200.0, - batteries={106, 206}, + batteries={9, 19}, request_timeout_sec=SAFETY_TIMEOUT, include_broken_batteries=True, ) @@ -680,7 +692,7 @@ async def test_use_all_batteries_none_is_working( assert len(done) == 1 result = done.pop().result() assert isinstance(result, Success) - assert result.succeeded_batteries == {106, 206} + assert result.succeeded_batteries == {9, 19} assert result.excess_power == approx(200.0) assert result.succeeded_power == approx(1000.0) assert result.request == request @@ -691,13 +703,16 @@ async def test_force_request_a_battery_is_not_working( self, mocker: MockerFixture ) -> None: """Test force request when a battery is not working.""" - await self.init_mock_microgrid(mocker) + mockgrid = MockMicrogrid(grid_side_meter=False) + mockgrid.add_batteries(3) + await mockgrid.start(mocker) + await self.init_component_data(mockgrid) mocker.patch("asyncio.sleep", new_callable=AsyncMock) - batteries = {106, 206} + batteries = {9, 19} - attrs = {"get_working_batteries.return_value": batteries - {106}} + attrs = {"get_working_batteries.return_value": batteries - {9}} mocker.patch( "frequenz.sdk.actor.power_distributing.power_distributing.BatteryPoolStatus", return_value=MagicMock(spec=BatteryPoolStatus, **attrs), @@ -732,23 +747,26 @@ async def test_force_request_a_battery_is_not_working( assert len(done) == 1 result = done.pop().result() assert isinstance(result, Success) - assert result.succeeded_batteries == {106, 206} + assert result.succeeded_batteries == {9, 19} assert result.excess_power == approx(200.0) assert result.succeeded_power == approx(1000.0) assert result.request == request await distributor._stop_actor() - # pylint: disable=too-many-locals async def test_force_request_battery_nan_value_non_cached( self, mocker: MockerFixture ) -> None: """Test battery with NaN in SoC, capacity or power is used if request is forced.""" - mock_microgrid = await self.init_mock_microgrid(mocker) + # pylint: disable=too-many-locals + mockgrid = MockMicrogrid(grid_side_meter=False) + mockgrid.add_batteries(3) + await mockgrid.start(mocker) + await self.init_component_data(mockgrid) mocker.patch("asyncio.sleep", new_callable=AsyncMock) - batteries = {106, 206} + batteries = {9, 19} attrs = {"get_working_batteries.return_value": batteries} mocker.patch( @@ -775,13 +793,13 @@ async def test_force_request_battery_nan_value_non_cached( batteries_data = ( battery_msg( - 106, + 9, soc=Metric(math.nan, Bound(20, 80)), capacity=Metric(math.nan), power=Bound(-1000, 1000), ), battery_msg( - 206, + 19, soc=Metric(40, Bound(20, 80)), capacity=Metric(math.nan), power=Bound(-1000, 1000), @@ -789,7 +807,7 @@ async def test_force_request_battery_nan_value_non_cached( ) for battery in batteries_data: - await mock_microgrid.send(battery) + await mockgrid.mock_client.send(battery) await channel.new_sender().send(request) result_rx = channel_registry.new_receiver(self._namespace) @@ -814,11 +832,14 @@ async def test_force_request_batteries_nan_values_cached( self, mocker: MockerFixture ) -> None: """Test battery with NaN in SoC, capacity or power is used if request is forced.""" - mock_microgrid = await self.init_mock_microgrid(mocker) + mockgrid = MockMicrogrid(grid_side_meter=False) + mockgrid.add_batteries(3) + await mockgrid.start(mocker) + await self.init_component_data(mockgrid) mocker.patch("asyncio.sleep", new_callable=AsyncMock) - batteries = {106, 206, 306} + batteries = {9, 19, 29} attrs = {"get_working_batteries.return_value": batteries} mocker.patch( @@ -861,19 +882,19 @@ async def test_result() -> None: batteries_data = ( battery_msg( - 106, + 9, soc=Metric(math.nan, Bound(20, 80)), capacity=Metric(98000), power=Bound(-1000, 1000), ), battery_msg( - 206, + 19, soc=Metric(40, Bound(20, 80)), capacity=Metric(math.nan), power=Bound(-1000, 1000), ), battery_msg( - 306, + 29, soc=Metric(40, Bound(20, 80)), capacity=Metric(float(98000)), power=Bound(math.nan, math.nan), @@ -887,7 +908,7 @@ async def test_result() -> None: await test_result() for battery in batteries_data: - await mock_microgrid.send(battery) + await mockgrid.mock_client.send(battery) await channel.new_sender().send(request) await test_result() From 018e1760fc9c94b1dcba17970d6ebad577833400 Mon Sep 17 00:00:00 2001 From: Sahas Subramanian Date: Mon, 10 Jul 2023 17:34:02 +0200 Subject: [PATCH 28/46] Remove hardcoded component graph and fixtures These are now unused because tests have migrated to use dynamic component graphs provided by MockMicrogrid. Signed-off-by: Sahas Subramanian --- tests/actor/test_battery_pool_status.py | 19 +------- tests/actor/test_battery_status.py | 63 +------------------------ tests/actor/test_power_distributing.py | 38 +-------------- 3 files changed, 4 insertions(+), 116 deletions(-) diff --git a/tests/actor/test_battery_pool_status.py b/tests/actor/test_battery_pool_status.py index 8bca3fb25..b17879a40 100644 --- a/tests/actor/test_battery_pool_status.py +++ b/tests/actor/test_battery_pool_status.py @@ -5,7 +5,6 @@ import asyncio from typing import Set -import pytest from frequenz.channels import Broadcast from pytest_mock import MockerFixture @@ -16,29 +15,13 @@ from frequenz.sdk.microgrid.component import ComponentCategory from tests.timeseries.mock_microgrid import MockMicrogrid -from ..utils.mock_microgrid_client import MockMicrogridClient -from .test_battery_status import battery_data, component_graph, inverter_data +from .test_battery_status import battery_data, inverter_data # pylint: disable=protected-access class TestBatteryPoolStatus: """Tests for BatteryPoolStatus""" - @pytest.fixture - async def mock_microgrid(self, mocker: MockerFixture) -> MockMicrogridClient: - """Create and initialize mock microgrid - - Args: - mocker: pytest mocker - - Returns: - MockMicrogridClient - """ - components, connections = component_graph() - microgrid = MockMicrogridClient(components, connections) - microgrid.initialize(mocker) - return microgrid - async def test_batteries_status(self, mocker: MockerFixture) -> None: """Basic tests for BatteryPoolStatus. diff --git a/tests/actor/test_battery_status.py b/tests/actor/test_battery_status.py index 6a146b3d4..c3a6243e0 100644 --- a/tests/actor/test_battery_status.py +++ b/tests/actor/test_battery_status.py @@ -6,9 +6,8 @@ import math from dataclasses import dataclass from datetime import datetime, timedelta, timezone -from typing import Generic, Iterable, List, Optional, Set, Tuple, TypeVar +from typing import Generic, Iterable, List, Optional, TypeVar -import pytest import time_machine from frequenz.api.microgrid.battery_pb2 import ComponentState as BatteryState from frequenz.api.microgrid.battery_pb2 import Error as BatteryError @@ -26,17 +25,10 @@ SetPowerResult, Status, ) -from frequenz.sdk.microgrid.client import Connection -from frequenz.sdk.microgrid.component import ( - BatteryData, - Component, - ComponentCategory, - InverterData, -) +from frequenz.sdk.microgrid.component import BatteryData, InverterData from tests.timeseries.mock_microgrid import MockMicrogrid from ..utils.component_data_wrapper import BatteryDataWrapper, InverterDataWrapper -from ..utils.mock_microgrid_client import MockMicrogridClient def battery_data( # pylint: disable=too-many-arguments @@ -109,42 +101,6 @@ def inverter_data( ) -def component_graph() -> Tuple[Set[Component], Set[Connection]]: - """Creates components and connections for the microgrid component graph. - - Returns: - Tuple with set of components and set of connections. - """ - components = { - Component(1, ComponentCategory.GRID), - Component(2, ComponentCategory.METER), - Component(104, ComponentCategory.METER), - Component(105, ComponentCategory.INVERTER), - Component(106, ComponentCategory.BATTERY), - Component(204, ComponentCategory.METER), - Component(205, ComponentCategory.INVERTER), - Component(206, ComponentCategory.BATTERY), - Component(304, ComponentCategory.METER), - Component(305, ComponentCategory.INVERTER), - Component(306, ComponentCategory.BATTERY), - } - - connections = { - Connection(1, 2), - Connection(2, 104), - Connection(104, 105), - Connection(105, 106), - Connection(2, 204), - Connection(204, 205), - Connection(205, 206), - Connection(2, 304), - Connection(304, 305), - Connection(305, 306), - } - - return components, connections - - T = TypeVar("T") @@ -163,21 +119,6 @@ class Message(Generic[T]): class TestBatteryStatus: """Tests BatteryStatusTracker.""" - @pytest.fixture - async def mock_microgrid(self, mocker: MockerFixture) -> MockMicrogridClient: - """Create and initialize mock microgrid - - Args: - mocker: pytest mocker - - Returns: - MockMicrogridClient - """ - components, connections = component_graph() - microgrid = MockMicrogridClient(components, connections) - microgrid.initialize(mocker) - return microgrid - @time_machine.travel("2022-01-01 00:00 UTC", tick=False) async def test_sync_update_status_with_messages( self, mocker: MockerFixture diff --git a/tests/actor/test_power_distributing.py b/tests/actor/test_power_distributing.py index 89dfeb130..5b9b01be2 100644 --- a/tests/actor/test_power_distributing.py +++ b/tests/actor/test_power_distributing.py @@ -29,8 +29,7 @@ Result, Success, ) -from frequenz.sdk.microgrid.client import Connection -from frequenz.sdk.microgrid.component import Component, ComponentCategory +from frequenz.sdk.microgrid.component import ComponentCategory from tests.timeseries.mock_microgrid import MockMicrogrid from ..conftest import SAFETY_TIMEOUT @@ -45,41 +44,6 @@ class TestPowerDistributingActor: _namespace = "power_distributor" - def component_graph(self) -> tuple[set[Component], set[Connection]]: - """Create graph components - - Returns: - Tuple where first element is set of components and second element is - set of connections. - """ - components = { - Component(1, ComponentCategory.GRID), - Component(2, ComponentCategory.METER), - Component(104, ComponentCategory.METER), - Component(105, ComponentCategory.INVERTER), - Component(106, ComponentCategory.BATTERY), - Component(204, ComponentCategory.METER), - Component(205, ComponentCategory.INVERTER), - Component(206, ComponentCategory.BATTERY), - Component(304, ComponentCategory.METER), - Component(305, ComponentCategory.INVERTER), - Component(306, ComponentCategory.BATTERY), - } - - connections = { - Connection(1, 2), - Connection(2, 104), - Connection(104, 105), - Connection(105, 106), - Connection(2, 204), - Connection(204, 205), - Connection(205, 206), - Connection(2, 304), - Connection(304, 305), - Connection(305, 306), - } - return components, connections - async def test_constructor(self, mocker: MockerFixture) -> None: """Test if gets all necessary data.""" mockgrid = MockMicrogrid(grid_side_meter=True) From 82d5834f9fa86f1e6ac6339d30a1d685938cd712 Mon Sep 17 00:00:00 2001 From: Sahas Subramanian Date: Tue, 11 Jul 2023 15:52:13 +0200 Subject: [PATCH 29/46] Check that minimum blocking duration is not greater than the maximum Signed-off-by: Sahas Subramanian --- src/frequenz/sdk/actor/power_distributing/_battery_status.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/frequenz/sdk/actor/power_distributing/_battery_status.py b/src/frequenz/sdk/actor/power_distributing/_battery_status.py index c18b7cb1e..22c631a11 100644 --- a/src/frequenz/sdk/actor/power_distributing/_battery_status.py +++ b/src/frequenz/sdk/actor/power_distributing/_battery_status.py @@ -81,6 +81,10 @@ class _BlockingStatus: max_duration_sec: float def __post_init__(self) -> None: + assert self.min_duration_sec <= self.max_duration_sec, ( + f"Minimum blocking duration ({self.min_duration_sec}) cannot be greater " + f"than maximum blocking duration ({self.max_duration_sec})" + ) self.last_blocking_duration_sec: float = self.min_duration_sec self.blocked_until: Optional[datetime] = None From 478b82be66903a2085168f6924d6b9519ecab885 Mon Sep 17 00:00:00 2001 From: Sahas Subramanian Date: Tue, 18 Jul 2023 15:57:09 +0200 Subject: [PATCH 30/46] Ensure component data is stale after receiving a timer event Because of the nature of async, it might happen that data is slightly delayed, so a timer event gets triggered, but component data arrives before the timer event arrives, and in such cases, we end up going into a bad state. This issue is resolved in this PR, by checking that the component data is actually late, every time a timer triggers. Signed-off-by: Sahas Subramanian --- .../actor/power_distributing/_battery_status.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/frequenz/sdk/actor/power_distributing/_battery_status.py b/src/frequenz/sdk/actor/power_distributing/_battery_status.py index 22c631a11..514177992 100644 --- a/src/frequenz/sdk/actor/power_distributing/_battery_status.py +++ b/src/frequenz/sdk/actor/power_distributing/_battery_status.py @@ -339,9 +339,25 @@ async def _run( self._handle_status_set_power_result(selected.value) elif selected_from(selected, battery_timer): + if ( + datetime.now(tz=timezone.utc) + - self._battery.last_msg_timestamp + ) < timedelta(seconds=self._max_data_age): + # This means that we have received data from the battery + # since the timer triggered, but the timer event arrived + # late, so we can ignore it. + continue self._handle_status_battery_timer() elif selected_from(selected, inverter_timer): + if ( + datetime.now(tz=timezone.utc) + - self._inverter.last_msg_timestamp + ) < timedelta(seconds=self._max_data_age): + # This means that we have received data from the inverter + # since the timer triggered, but the timer event arrived + # late, so we can ignore it. + continue self._handle_status_inverter_timer() else: From d38764e3125d475433270f5b828975cd32a814fc Mon Sep 17 00:00:00 2001 From: Sahas Subramanian Date: Tue, 18 Jul 2023 18:56:49 +0200 Subject: [PATCH 31/46] Add recovery tests for BatteryStatusTracker Signed-off-by: Sahas Subramanian --- tests/actor/test_battery_status.py | 343 ++++++++++++++++++++++++++++- 1 file changed, 340 insertions(+), 3 deletions(-) diff --git a/tests/actor/test_battery_status.py b/tests/actor/test_battery_status.py index c3a6243e0..f8624d05e 100644 --- a/tests/actor/test_battery_status.py +++ b/tests/actor/test_battery_status.py @@ -6,8 +6,9 @@ import math from dataclasses import dataclass from datetime import datetime, timedelta, timezone -from typing import Generic, Iterable, List, Optional, TypeVar +from typing import AsyncIterator, Generic, Iterable, List, Optional, TypeVar +import pytest import time_machine from frequenz.api.microgrid.battery_pb2 import ComponentState as BatteryState from frequenz.api.microgrid.battery_pb2 import Error as BatteryError @@ -17,7 +18,7 @@ from frequenz.api.microgrid.inverter_pb2 import ComponentState as InverterState from frequenz.api.microgrid.inverter_pb2 import Error as InverterError from frequenz.api.microgrid.inverter_pb2 import ErrorCode as InverterErrorCode -from frequenz.channels import Broadcast +from frequenz.channels import Broadcast, Receiver from pytest_mock import MockerFixture from frequenz.sdk.actor.power_distributing._battery_status import ( @@ -115,6 +116,26 @@ class Message(Generic[T]): INVERTER_ID = 8 +class _Timeout: + """Sentinel for timeout.""" + + +async def recv_timeout(recv: Receiver[T], timeout: float = 0.1) -> T | type[_Timeout]: + """Receive message from receiver with timeout. + + Args: + recv: Receiver to receive message from. + timeout: Timeout in seconds. + + Returns: + Received message or _Timeout if timeout is reached. + """ + try: + return await asyncio.wait_for(recv.receive(), timeout=timeout) + except asyncio.TimeoutError: + return _Timeout + + # pylint: disable=protected-access, unused-argument class TestBatteryStatus: """Tests BatteryStatusTracker.""" @@ -446,7 +467,8 @@ async def test_sync_blocking_interrupted_with_with_max_data( assert tracker._get_new_status_if_changed() is None time.shift(timeout) - await tracker.stop() + await tracker.stop() + await mock_microgrid.cleanup() @time_machine.travel("2022-01-01 00:00 UTC", tick=False) async def test_sync_blocking_interrupted_with_invalid_message( @@ -508,6 +530,7 @@ async def test_sync_blocking_interrupted_with_invalid_message( assert tracker._get_new_status_if_changed() is Status.WORKING await tracker.stop() + await mock_microgrid.cleanup() @time_machine.travel("2022-01-01 00:00 UTC", tick=False) async def test_timers(self, mocker: MockerFixture) -> None: @@ -644,3 +667,317 @@ async def test_async_battery_status(self, mocker: MockerFixture) -> None: await tracker.stop() await mock_microgrid.cleanup() + + +class TestBatteryStatusRecovery: + """Test battery status recovery. + + The following cases are tested: + + - battery/inverter data missing + - battery/inverter bad state + - battery/inverter warning/critical error + - battery capacity missing + - received stale battery/inverter data + """ + + @pytest.fixture + async def setup_tracker( + self, mocker: MockerFixture + ) -> AsyncIterator[tuple[MockMicrogrid, Receiver[Status]]]: + """Setup a BatteryStatusTracker instance to run tests with.""" + mock_microgrid = MockMicrogrid(grid_side_meter=True) + mock_microgrid.add_batteries(1) + await mock_microgrid.start(mocker) + + status_channel = Broadcast[Status]("battery_status") + set_power_result_channel = Broadcast[SetPowerResult]("set_power_result") + + status_receiver = status_channel.new_receiver() + + _tracker = BatteryStatusTracker( + BATTERY_ID, + max_data_age_sec=0.1, + max_blocking_duration_sec=1, + status_sender=status_channel.new_sender(), + set_power_result_receiver=set_power_result_channel.new_receiver(), + ) + + await asyncio.sleep(0.05) + + yield (mock_microgrid, status_receiver) + + await _tracker.stop() + await mock_microgrid.cleanup() + + async def _send_healthy_battery( + self, mock_microgrid: MockMicrogrid, timestamp: datetime | None = None + ) -> None: + await mock_microgrid.mock_client.send( + battery_data( + timestamp=timestamp, + component_id=BATTERY_ID, + component_state=BatteryState.COMPONENT_STATE_IDLE, + relay_state=BatteryRelayState.RELAY_STATE_CLOSED, + ) + ) + + async def _send_battery_missing_capacity( + self, mock_microgrid: MockMicrogrid + ) -> None: + await mock_microgrid.mock_client.send( + battery_data( + component_id=BATTERY_ID, + component_state=BatteryState.COMPONENT_STATE_IDLE, + relay_state=BatteryRelayState.RELAY_STATE_CLOSED, + capacity=math.nan, + ) + ) + + async def _send_healthy_inverter( + self, mock_microgrid: MockMicrogrid, timestamp: datetime | None = None + ) -> None: + await mock_microgrid.mock_client.send( + inverter_data( + timestamp=timestamp, + component_id=INVERTER_ID, + component_state=InverterState.COMPONENT_STATE_IDLE, + ) + ) + + async def _send_bad_state_battery(self, mock_microgrid: MockMicrogrid) -> None: + await mock_microgrid.mock_client.send( + battery_data( + component_id=BATTERY_ID, + component_state=BatteryState.COMPONENT_STATE_ERROR, + relay_state=BatteryRelayState.RELAY_STATE_CLOSED, + ) + ) + + async def _send_bad_state_inverter(self, mock_microgrid: MockMicrogrid) -> None: + await mock_microgrid.mock_client.send( + inverter_data( + component_id=INVERTER_ID, + component_state=InverterState.COMPONENT_STATE_ERROR, + ) + ) + + async def _send_critical_error_battery(self, mock_microgrid: MockMicrogrid) -> None: + battery_critical_error = BatteryError( + code=BatteryErrorCode.ERROR_CODE_BLOCK_ERROR, + level=ErrorLevel.ERROR_LEVEL_CRITICAL, + msg="", + ) + await mock_microgrid.mock_client.send( + battery_data( + component_id=BATTERY_ID, + component_state=BatteryState.COMPONENT_STATE_IDLE, + relay_state=BatteryRelayState.RELAY_STATE_CLOSED, + errors=[battery_critical_error], + ) + ) + + async def _send_warning_error_battery(self, mock_microgrid: MockMicrogrid) -> None: + battery_warning_error = BatteryError( + code=BatteryErrorCode.ERROR_CODE_HIGH_HUMIDITY, + level=ErrorLevel.ERROR_LEVEL_WARN, + msg="", + ) + await mock_microgrid.mock_client.send( + battery_data( + component_id=BATTERY_ID, + component_state=BatteryState.COMPONENT_STATE_IDLE, + relay_state=BatteryRelayState.RELAY_STATE_CLOSED, + errors=[battery_warning_error], + ) + ) + + async def _send_critical_error_inverter( + self, mock_microgrid: MockMicrogrid + ) -> None: + inverter_critical_error = InverterError( + code=InverterErrorCode.ERROR_CODE_UNSPECIFIED, + level=ErrorLevel.ERROR_LEVEL_CRITICAL, + msg="", + ) + await mock_microgrid.mock_client.send( + inverter_data( + component_id=INVERTER_ID, + component_state=InverterState.COMPONENT_STATE_IDLE, + errors=[inverter_critical_error], + ) + ) + + async def _send_warning_error_inverter(self, mock_microgrid: MockMicrogrid) -> None: + inverter_warning_error = InverterError( + code=InverterErrorCode.ERROR_CODE_UNSPECIFIED, + level=ErrorLevel.ERROR_LEVEL_WARN, + msg="", + ) + await mock_microgrid.mock_client.send( + inverter_data( + component_id=INVERTER_ID, + component_state=InverterState.COMPONENT_STATE_IDLE, + errors=[inverter_warning_error], + ) + ) + + async def test_missing_data( + self, + setup_tracker: tuple[MockMicrogrid, Receiver[Status]], + ) -> None: + """Test recovery after missing data.""" + mock_microgrid, status_receiver = setup_tracker + + await self._send_healthy_battery(mock_microgrid) + await self._send_healthy_inverter(mock_microgrid) + assert await status_receiver.receive() is Status.WORKING + + # --- missing battery data --- + await self._send_healthy_inverter(mock_microgrid) + assert await status_receiver.receive() is Status.NOT_WORKING + + await self._send_healthy_battery(mock_microgrid) + await self._send_healthy_inverter(mock_microgrid) + assert await status_receiver.receive() is Status.WORKING + + # --- missing inverter data --- + await self._send_healthy_battery(mock_microgrid) + assert await status_receiver.receive() is Status.NOT_WORKING + + await self._send_healthy_battery(mock_microgrid) + await self._send_healthy_inverter(mock_microgrid) + assert await status_receiver.receive() is Status.WORKING + + async def test_bad_state( + self, + setup_tracker: tuple[MockMicrogrid, Receiver[Status]], + ) -> None: + """Test recovery after bad component state.""" + mock_microgrid, status_receiver = setup_tracker + + await self._send_healthy_battery(mock_microgrid) + await self._send_healthy_inverter(mock_microgrid) + assert await status_receiver.receive() is Status.WORKING + + # --- bad battery state --- + await self._send_healthy_inverter(mock_microgrid) + await self._send_bad_state_battery(mock_microgrid) + assert await status_receiver.receive() is Status.NOT_WORKING + + await self._send_healthy_battery(mock_microgrid) + await self._send_healthy_inverter(mock_microgrid) + assert await status_receiver.receive() is Status.WORKING + + # --- bad inverter state --- + await self._send_bad_state_inverter(mock_microgrid) + await self._send_healthy_battery(mock_microgrid) + assert await status_receiver.receive() is Status.NOT_WORKING + + await self._send_healthy_battery(mock_microgrid) + await self._send_healthy_inverter(mock_microgrid) + assert await status_receiver.receive() is Status.WORKING + + async def test_critical_error( + self, + setup_tracker: tuple[MockMicrogrid, Receiver[Status]], + ) -> None: + """Test recovery after critical error.""" + + mock_microgrid, status_receiver = setup_tracker + + await self._send_healthy_inverter(mock_microgrid) + await self._send_healthy_battery(mock_microgrid) + assert await status_receiver.receive() is Status.WORKING + + # --- battery warning error (keeps working) --- + await self._send_healthy_inverter(mock_microgrid) + await self._send_warning_error_battery(mock_microgrid) + assert await recv_timeout(status_receiver, timeout=0.1) is _Timeout + + await self._send_healthy_battery(mock_microgrid) + await self._send_healthy_inverter(mock_microgrid) + + # --- battery critical error --- + await self._send_healthy_inverter(mock_microgrid) + await self._send_critical_error_battery(mock_microgrid) + assert await status_receiver.receive() is Status.NOT_WORKING + + await self._send_healthy_battery(mock_microgrid) + await self._send_healthy_inverter(mock_microgrid) + assert await status_receiver.receive() is Status.WORKING + + # --- inverter warning error (keeps working) --- + await self._send_healthy_battery(mock_microgrid) + await self._send_warning_error_inverter(mock_microgrid) + assert await recv_timeout(status_receiver, timeout=0.1) is _Timeout + + await self._send_healthy_battery(mock_microgrid) + await self._send_healthy_inverter(mock_microgrid) + + # --- inverter critical error --- + await self._send_healthy_battery(mock_microgrid) + await self._send_critical_error_inverter(mock_microgrid) + assert await status_receiver.receive() is Status.NOT_WORKING + + await self._send_healthy_battery(mock_microgrid) + await self._send_healthy_inverter(mock_microgrid) + assert await status_receiver.receive() is Status.WORKING + + async def test_missing_capacity( + self, + setup_tracker: tuple[MockMicrogrid, Receiver[Status]], + ) -> None: + """Test recovery after missing capacity.""" + mock_microgrid, status_receiver = setup_tracker + + await self._send_healthy_battery(mock_microgrid) + await self._send_healthy_inverter(mock_microgrid) + assert await status_receiver.receive() is Status.WORKING + + await self._send_healthy_inverter(mock_microgrid) + await self._send_battery_missing_capacity(mock_microgrid) + assert await status_receiver.receive() is Status.NOT_WORKING + + await self._send_healthy_battery(mock_microgrid) + await self._send_healthy_inverter(mock_microgrid) + assert await status_receiver.receive() is Status.WORKING + + async def test_stale_data( + self, + setup_tracker: tuple[MockMicrogrid, Receiver[Status]], + ) -> None: + """Test recovery after stale data.""" + mock_microgrid, status_receiver = setup_tracker + + timestamp = datetime.now(timezone.utc) + await self._send_healthy_battery(mock_microgrid, timestamp) + await self._send_healthy_inverter(mock_microgrid) + assert await status_receiver.receive() is Status.WORKING + + # --- stale battery data --- + await self._send_healthy_inverter(mock_microgrid) + await self._send_healthy_battery(mock_microgrid, timestamp) + assert await recv_timeout(status_receiver) is _Timeout + + await self._send_healthy_inverter(mock_microgrid) + await self._send_healthy_battery(mock_microgrid, timestamp) + assert await recv_timeout(status_receiver) is Status.NOT_WORKING + + timestamp = datetime.now(timezone.utc) + await self._send_healthy_battery(mock_microgrid, timestamp) + await self._send_healthy_inverter(mock_microgrid, timestamp) + assert await status_receiver.receive() is Status.WORKING + + # --- stale inverter data --- + await self._send_healthy_battery(mock_microgrid) + await self._send_healthy_inverter(mock_microgrid, timestamp) + assert await recv_timeout(status_receiver) is _Timeout + + await self._send_healthy_battery(mock_microgrid) + await self._send_healthy_inverter(mock_microgrid, timestamp) + assert await recv_timeout(status_receiver) is Status.NOT_WORKING + + await self._send_healthy_battery(mock_microgrid) + await self._send_healthy_inverter(mock_microgrid) + assert await status_receiver.receive() is Status.WORKING From 6885e32fee84aeaa74d454d234833bdda7518cbc Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 27 Jul 2023 07:37:09 +0000 Subject: [PATCH 32/46] Bump pylint from 2.17.4 to 2.17.5 Bumps [pylint](https://github.com/pylint-dev/pylint) from 2.17.4 to 2.17.5. - [Release notes](https://github.com/pylint-dev/pylint/releases) - [Commits](https://github.com/pylint-dev/pylint/compare/v2.17.4...v2.17.5) --- updated-dependencies: - dependency-name: pylint dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] --- pyproject.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 4da74a47a..94b1d6aac 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -74,7 +74,7 @@ dev-mypy = [ ] dev-noxfile = ["nox == 2023.4.22", "frequenz-repo-config[lib] == 0.3.0"] dev-pylint = [ - "pylint == 2.17.4", + "pylint == 2.17.5", # For checking the noxfile, docs/ script, and tests "frequenz-sdk[dev-mkdocs,dev-noxfile,dev-pytest]", ] @@ -86,7 +86,7 @@ dev-pytest = [ "async-solipsism == 0.5", # For checking docstring code examples "sybil == 5.0.3", - "pylint == 2.17.4", + "pylint == 2.17.5", "frequenz-sdk[dev-examples]", ] dev = [ From 3f2b5eefe7f4fa88f5cf9b16975befc42569b873 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Thu, 27 Jul 2023 11:35:45 +0200 Subject: [PATCH 33/46] Update to repo-config v0.4.0 Signed-off-by: Leandro Lucarella --- .cookiecutter-replay.json | 32 ++++++++++++++++++++++++++++++-- .github/workflows/ci.yaml | 2 +- mkdocs.yml | 13 ++++++++++--- pyproject.toml | 10 +++++----- 4 files changed, 46 insertions(+), 11 deletions(-) diff --git a/.cookiecutter-replay.json b/.cookiecutter-replay.json index fae25f975..f7085ec67 100644 --- a/.cookiecutter-replay.json +++ b/.cookiecutter-replay.json @@ -1,5 +1,6 @@ { "cookiecutter": { + "Introduction": "]\n\nWelcome to repo-config Cookiecutter template!\n\nThis template will help you to create a new repository for your project. You will be asked to provide some information about your project.\n\nHere is an explanation of what each variable is for and will be used for:\n\n* `type`: The type of repository. It must be chosen from the list.\n\n* `name`: The name of the project. This will be used to build defaults for\n other inputs, such as `title`, `python_package`, etc. It should be one word,\n using only alphanumeric characters (and starting with a letter). It can\n include also `_` and `-` which will be handled differently when building\n other variables from it (replaced by spaces in titles for example).\n\n* `description`: A short description of the project. It will be used as the\n description in the `README.md`, `pyproject.toml`, `mkdocs.yml`, etc.\n\n* `title`: A human-readable name or title for the project. It will be used in\n the `README.md`, `CONTRIBUTING.md`, and other files to refer to the project,\n as well as the site title in `mkdocs.yml`.\n\n* `keywords`: A comma-separated list of keywords that will be used in the\n `pyproject.toml` file. If left untouched, it will use only some predefined\n keywords. If anything else is entered, it will be **added** to the default\n keywords.\n\n* `github_org`: The GitHub handle of the organization where the project will\n reside. This will be used to generate links to the project on GitHub.\n\n* `license`: Currently, only two options are provided: `MIT`, which should be\n used for open-source projects, and `Proprietary`, which should be used for\n closed-source projects. This will be added to file headers and used as the\n license in `pyproject.toml`.\n\n* `author_name`, `author_email`: The name and email address of the author of\n the project. They will be used in the copyright notice in file headers and\n as the author in `pyproject.toml`.\n\n* `python_package`: The Python package in which this project will reside. All\n files provided by this project should be located in this package. This needs\n to be a list of valid Python identifiers separated by dots. The source file\n structure will be derived from this. For example, `frequenz.actor.example`\n will generate files in `src/frequenz/actor/example`.\n\n* `pypi_package_name`: The name of the PyPI/wheel/distribution package. This\n should be consistent with the `python_package`, usually replacing `.` with\n `-`. For example, `frequenz-actor-example`.\n\n* `github_repo_name`: The handle of the GitHub repository where the project\n will reside. This will be used to generate links to the project on GitHub and\n as the top-level directory name.\n\n* `default_codeowners`: A space-separated list of GitHub teams (`@org/team`) or\n users (`@user`) that will be the default code owners for this project. This\n will be used to build the `CODEOWNERS` file. Please refer to the [code owners\n documentation] for more details on the valid syntax.\n\n[code owners documentation]: https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/customizing-your-repository/about-code-owners\n\n\n[Please press any key to continue", "type": "lib", "name": "sdk", "description": "A development kit to interact with the Frequenz development platform", @@ -15,14 +16,41 @@ "default_codeowners": "@frequenz-floss/python-sdk-team", "_extensions": [ "jinja2_time.TimeExtension", + "local_extensions.as_identifier", "local_extensions.default_codeowners", "local_extensions.github_repo_name", + "local_extensions.introduction", "local_extensions.keywords", "local_extensions.pypi_package_name", "local_extensions.python_package", "local_extensions.src_path", "local_extensions.title" ], - "_template": "gh:frequenz-floss/frequenz-repo-config-python" + "_template": "gh:frequenz-floss/frequenz-repo-config-python", + }, + "_cookiecutter": { + "Introduction": "{{cookiecutter | introduction}}", + "type": [ + "actor", + "api", + "app", + "lib", + "model" + ], + "name": null, + "description": null, + "title": "{{cookiecutter | title}}", + "keywords": "(comma separated: 'frequenz', and are included automatically)", + "github_org": "frequenz-floss", + "license": [ + "MIT", + "Proprietary" + ], + "author_name": "Frequenz Energy-as-a-Service GmbH", + "author_email": "floss@frequenz.com", + "python_package": "{{cookiecutter | python_package}}", + "pypi_package_name": "{{cookiecutter | pypi_package_name}}", + "github_repo_name": "{{cookiecutter | github_repo_name}}", + "default_codeowners": "(like @some-org/some-team; defaults to a team based on the repo type)" } -} +} \ No newline at end of file diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index ba8647c43..52b1f616d 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -50,7 +50,7 @@ jobs: python -m pip install -e .[dev-noxfile] - name: Run nox - # To speed things up a bit we use the speciall ci_checks_max session + # To speed things up a bit we use the special ci_checks_max session # that uses the same venv to run multiple linting sessions run: nox -e ci_checks_max pytest_min timeout-minutes: 10 diff --git a/mkdocs.yml b/mkdocs.yml index 86701d07d..e8ddb4623 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -22,6 +22,8 @@ theme: repo: fontawesome/brands/github custom_dir: docs/overrides features: + - content.code.annotate + - content.code.copy - navigation.instant - navigation.tabs - navigation.top @@ -62,9 +64,11 @@ markdown_extensions: - admonition - attr_list - pymdownx.details - - pymdownx.superfences - - pymdownx.tasklist - - pymdownx.tabbed + - pymdownx.highlight: + anchor_linenums: true + line_spans: __span + pygments_lang_class: true + - pymdownx.keys - pymdownx.snippets: check_paths: true - pymdownx.superfences: @@ -72,6 +76,8 @@ markdown_extensions: - name: mermaid class: mermaid format: "!!python/name:pymdownx.superfences.fence_code_format" + - pymdownx.tabbed + - pymdownx.tasklist - toc: permalink: "¤" @@ -97,6 +103,7 @@ plugins: show_root_members_full_path: true show_source: true import: + # See https://mkdocstrings.github.io/python/usage/#import for details - https://docs.python.org/3/objects.inv - https://frequenz-floss.github.io/frequenz-channels-python/v0.14/objects.inv - https://grpc.github.io/grpc/python/objects.inv diff --git a/pyproject.toml b/pyproject.toml index 94b1d6aac..0491b717c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -5,7 +5,7 @@ requires = [ "setuptools == 67.7.2", "setuptools_scm[toml] == 7.1.0", - "frequenz-repo-config[lib] == 0.3.0", + "frequenz-repo-config[lib] == 0.4.0", ] build-backend = "setuptools.build_meta" @@ -14,7 +14,7 @@ name = "frequenz-sdk" description = "A development kit to interact with the Frequenz development platform" readme = "README.md" license = { text = "MIT" } -keywords = ["frequenz", "sdk", "microgrid", "actor"] +keywords = ["frequenz", "python", "lib", "library", "sdk", "microgrid"] classifiers = [ "Development Status :: 3 - Alpha", "Intended Audience :: Developers", @@ -63,7 +63,7 @@ dev-mkdocs = [ "mkdocs-material == 9.1.19", "mkdocs-section-index == 0.3.5", "mkdocstrings[python] == 0.22.0", - "frequenz-repo-config[lib] == 0.3.0", + "frequenz-repo-config[lib] == 0.4.0", ] dev-mypy = [ "mypy == 1.4.1", @@ -72,7 +72,7 @@ dev-mypy = [ # For checking the noxfile, docs/ script, and tests "frequenz-sdk[dev-mkdocs,dev-noxfile,dev-pytest]", ] -dev-noxfile = ["nox == 2023.4.22", "frequenz-repo-config[lib] == 0.3.0"] +dev-noxfile = ["nox == 2023.4.22", "frequenz-repo-config[lib] == 0.4.0"] dev-pylint = [ "pylint == 2.17.5", # For checking the noxfile, docs/ script, and tests @@ -107,7 +107,7 @@ include = '\.pyi?$' [tool.isort] profile = "black" line_length = 88 -src_paths = ["src", "examples", "tests"] +src_paths = ["benchmarks", "examples", "src", "tests"] [tool.pylint.similarities] ignore-comments = ['yes'] From 5d32be8d670b0671132ad89fa3fbbcdbbabc9f8d Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 28 Jul 2023 08:04:40 +0000 Subject: [PATCH 34/46] Bump mkdocs-material from 9.1.19 to 9.1.21 Bumps [mkdocs-material](https://github.com/squidfunk/mkdocs-material) from 9.1.19 to 9.1.21. - [Release notes](https://github.com/squidfunk/mkdocs-material/releases) - [Changelog](https://github.com/squidfunk/mkdocs-material/blob/master/CHANGELOG) - [Commits](https://github.com/squidfunk/mkdocs-material/compare/9.1.19...9.1.21) --- updated-dependencies: - dependency-name: mkdocs-material dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 0491b717c..697bfd280 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -60,7 +60,7 @@ dev-mkdocs = [ "mike == 1.1.2", "mkdocs-gen-files == 0.5.0", "mkdocs-literate-nav == 0.6.0", - "mkdocs-material == 9.1.19", + "mkdocs-material == 9.1.21", "mkdocs-section-index == 0.3.5", "mkdocstrings[python] == 0.22.0", "frequenz-repo-config[lib] == 0.4.0", From ada7a61fc2a7db2978d32103e7ebe9dd9a24b8d2 Mon Sep 17 00:00:00 2001 From: Sahas Subramanian Date: Fri, 28 Jul 2023 17:25:54 +0200 Subject: [PATCH 35/46] Add tests for the `Temperature` quantity Signed-off-by: Sahas Subramanian --- tests/timeseries/test_quantities.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/timeseries/test_quantities.py b/tests/timeseries/test_quantities.py index a7b3d02bf..678c049b8 100644 --- a/tests/timeseries/test_quantities.py +++ b/tests/timeseries/test_quantities.py @@ -14,6 +14,7 @@ Percentage, Power, Quantity, + Temperature, Voltage, ) @@ -264,6 +265,19 @@ def test_energy() -> None: Energy(1.0, exponent=0) +def test_temperature() -> None: + """Test the temperature class.""" + temp = Temperature.from_celsius(30.4) + assert f"{temp}" == "30.4 °C" + + assert temp.as_celsius() == 30.4 + assert temp != Temperature.from_celsius(5.0) + + with pytest.raises(TypeError): + # using the default constructor should raise. + Temperature(1.0, exponent=0) + + def test_quantity_compositions() -> None: """Test the composition of quantities.""" power = Power.from_watts(1000.0) From 25cd07419a71ce3d764050c9aa9ff37bd9259ec4 Mon Sep 17 00:00:00 2001 From: Sahas Subramanian Date: Wed, 26 Jul 2023 16:56:31 +0200 Subject: [PATCH 36/46] Rename `temperature_max` fields to `temperature` The name `temperature_max` of a single battery indicates that a single battery can have multiple temperatures. This is not the case for all batteries, and hence can't be part of the high level API. In the high level APIs, there should be just one temperature value per battery. Signed-off-by: Sahas Subramanian --- .../sdk/actor/_data_sourcing/microgrid_api_source.py | 2 +- src/frequenz/sdk/microgrid/component/_component.py | 2 +- src/frequenz/sdk/microgrid/component/_component_data.py | 6 +++--- .../sdk/timeseries/battery_pool/_metric_calculator.py | 4 ++-- tests/utils/component_data_wrapper.py | 4 ++-- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/frequenz/sdk/actor/_data_sourcing/microgrid_api_source.py b/src/frequenz/sdk/actor/_data_sourcing/microgrid_api_source.py index 5e131a623..cc69d3bef 100644 --- a/src/frequenz/sdk/actor/_data_sourcing/microgrid_api_source.py +++ b/src/frequenz/sdk/actor/_data_sourcing/microgrid_api_source.py @@ -84,7 +84,7 @@ def get_channel_name(self) -> str: ComponentMetricId.CAPACITY: lambda msg: msg.capacity, ComponentMetricId.POWER_LOWER_BOUND: lambda msg: msg.power_lower_bound, ComponentMetricId.POWER_UPPER_BOUND: lambda msg: msg.power_upper_bound, - ComponentMetricId.TEMPERATURE_MAX: lambda msg: msg.temperature_max, + ComponentMetricId.TEMPERATURE: lambda msg: msg.temperature, } _InverterDataMethods: Dict[ComponentMetricId, Callable[[InverterData], float]] = { diff --git a/src/frequenz/sdk/microgrid/component/_component.py b/src/frequenz/sdk/microgrid/component/_component.py index da4a3d1e8..f63d054be 100644 --- a/src/frequenz/sdk/microgrid/component/_component.py +++ b/src/frequenz/sdk/microgrid/component/_component.py @@ -140,4 +140,4 @@ class ComponentMetricId(Enum): ACTIVE_POWER_LOWER_BOUND = "active_power_lower_bound" ACTIVE_POWER_UPPER_BOUND = "active_power_upper_bound" - TEMPERATURE_MAX = "temperature_max" + TEMPERATURE = "temperature" diff --git a/src/frequenz/sdk/microgrid/component/_component_data.py b/src/frequenz/sdk/microgrid/component/_component_data.py index cbeb73ab3..81696a8f9 100644 --- a/src/frequenz/sdk/microgrid/component/_component_data.py +++ b/src/frequenz/sdk/microgrid/component/_component_data.py @@ -141,8 +141,8 @@ class BatteryData(ComponentData): This will be a positive number, or zero if no charging is possible. """ - temperature_max: float - """The maximum temperature of all the blocks in a battery, in Celcius (°C).""" + temperature: float + """The (average) temperature reported by the battery a battery, in Celcius (°C).""" _relay_state: battery_pb.RelayState.ValueType """State of the battery relay.""" @@ -172,7 +172,7 @@ def from_proto(cls, raw: microgrid_pb.ComponentData) -> BatteryData: capacity=raw.battery.properties.capacity, power_lower_bound=raw.battery.data.dc.power.system_bounds.lower, power_upper_bound=raw.battery.data.dc.power.system_bounds.upper, - temperature_max=raw.battery.data.temperature.max, + temperature=raw.battery.data.temperature.avg, _relay_state=raw.battery.state.relay_state, _component_state=raw.battery.state.component_state, _errors=list(raw.battery.errors), diff --git a/src/frequenz/sdk/timeseries/battery_pool/_metric_calculator.py b/src/frequenz/sdk/timeseries/battery_pool/_metric_calculator.py index 2e2ff5647..22876038b 100644 --- a/src/frequenz/sdk/timeseries/battery_pool/_metric_calculator.py +++ b/src/frequenz/sdk/timeseries/battery_pool/_metric_calculator.py @@ -246,7 +246,7 @@ def __init__(self, batteries: Set[int]) -> None: super().__init__(batteries) self._metrics = [ - ComponentMetricId.TEMPERATURE_MAX, + ComponentMetricId.TEMPERATURE, ] @classmethod @@ -306,7 +306,7 @@ def calculate( if battery_id not in metrics_data: continue metrics = metrics_data[battery_id] - temperature = metrics.get(ComponentMetricId.TEMPERATURE_MAX) + temperature = metrics.get(ComponentMetricId.TEMPERATURE) if temperature is None: continue timestamp = max(timestamp, metrics.timestamp) diff --git a/tests/utils/component_data_wrapper.py b/tests/utils/component_data_wrapper.py index 6339fda88..059a5c8a5 100644 --- a/tests/utils/component_data_wrapper.py +++ b/tests/utils/component_data_wrapper.py @@ -43,7 +43,7 @@ def __init__( # pylint: disable=too-many-arguments capacity: float = math.nan, power_lower_bound: float = math.nan, power_upper_bound: float = math.nan, - temperature_max: float = math.nan, + temperature: float = math.nan, _relay_state: battery_pb.RelayState.ValueType = ( battery_pb.RelayState.RELAY_STATE_UNSPECIFIED ), @@ -66,7 +66,7 @@ def __init__( # pylint: disable=too-many-arguments capacity=capacity, power_lower_bound=power_lower_bound, power_upper_bound=power_upper_bound, - temperature_max=temperature_max, + temperature=temperature, _relay_state=_relay_state, _component_state=_component_state, _errors=_errors if _errors else [], From 3ae0941ababb24b343dd867dc136127ba0623e7a Mon Sep 17 00:00:00 2001 From: Sahas Subramanian Date: Wed, 26 Jul 2023 17:11:43 +0200 Subject: [PATCH 37/46] Replace `TemperatureMetrics` with `Sample[Temperature]` This would just stream average of average temperatures of all batteries in a pool. Signed-off-by: Sahas Subramanian --- .../sdk/timeseries/battery_pool/__init__.py | 3 +-- .../battery_pool/_metric_calculator.py | 20 +++++++------------ .../timeseries/battery_pool/_result_types.py | 19 ------------------ .../timeseries/battery_pool/battery_pool.py | 10 +++++----- 4 files changed, 13 insertions(+), 39 deletions(-) diff --git a/src/frequenz/sdk/timeseries/battery_pool/__init__.py b/src/frequenz/sdk/timeseries/battery_pool/__init__.py index 1d6ce5c77..56f07eda2 100644 --- a/src/frequenz/sdk/timeseries/battery_pool/__init__.py +++ b/src/frequenz/sdk/timeseries/battery_pool/__init__.py @@ -3,12 +3,11 @@ """Manage a pool of batteries.""" -from ._result_types import Bound, PowerMetrics, TemperatureMetrics +from ._result_types import Bound, PowerMetrics from .battery_pool import BatteryPool __all__ = [ "BatteryPool", "PowerMetrics", - "TemperatureMetrics", "Bound", ] diff --git a/src/frequenz/sdk/timeseries/battery_pool/_metric_calculator.py b/src/frequenz/sdk/timeseries/battery_pool/_metric_calculator.py index 22876038b..e2cd3ce60 100644 --- a/src/frequenz/sdk/timeseries/battery_pool/_metric_calculator.py +++ b/src/frequenz/sdk/timeseries/battery_pool/_metric_calculator.py @@ -14,7 +14,7 @@ from ...microgrid.component import ComponentCategory, ComponentMetricId, InverterType from ...timeseries import Energy, Percentage, Sample, Temperature from ._component_metrics import ComponentMetricsData -from ._result_types import Bound, PowerMetrics, TemperatureMetrics +from ._result_types import Bound, PowerMetrics _logger = logging.getLogger(__name__) _MIN_TIMESTAMP = datetime.min.replace(tzinfo=timezone.utc) @@ -59,7 +59,7 @@ def battery_inverter_mapping(batteries: Iterable[int]) -> dict[int, int]: # Formula output types class have no common interface # Print all possible types here. -T = TypeVar("T", Sample[Percentage], Sample[Energy], PowerMetrics, TemperatureMetrics) +T = TypeVar("T", Sample[Percentage], Sample[Energy], PowerMetrics, Sample[Temperature]) class MetricCalculator(ABC, Generic[T]): @@ -234,7 +234,7 @@ def calculate( ) -class TemperatureCalculator(MetricCalculator[TemperatureMetrics]): +class TemperatureCalculator(MetricCalculator[Sample[Temperature]]): """Define how to calculate temperature metrics.""" def __init__(self, batteries: Set[int]) -> None: @@ -280,7 +280,7 @@ def calculate( self, metrics_data: dict[int, ComponentMetricsData], working_batteries: set[int], - ) -> TemperatureMetrics | None: + ) -> Sample[Temperature] | None: """Aggregate the metrics_data and calculate high level metric for temperature. Missing components will be ignored. Formula will be calculated for all @@ -298,9 +298,7 @@ def calculate( Return None if there are no component metrics. """ timestamp = _MIN_TIMESTAMP - temperature_sum: float = 0 - temperature_min: float = float("inf") - temperature_max: float = float("-inf") + temperature_sum: float = 0.0 temperature_count: int = 0 for battery_id in working_batteries: if battery_id not in metrics_data: @@ -311,19 +309,15 @@ def calculate( continue timestamp = max(timestamp, metrics.timestamp) temperature_sum += temperature - temperature_min = min(temperature_min, temperature) - temperature_max = max(temperature_max, temperature) temperature_count += 1 if timestamp == _MIN_TIMESTAMP: return None temperature_avg = temperature_sum / temperature_count - return TemperatureMetrics( + return Sample[Temperature]( timestamp=timestamp, - min=Temperature.from_celsius(value=temperature_min), - avg=Temperature.from_celsius(value=temperature_avg), - max=Temperature.from_celsius(value=temperature_max), + value=Temperature.from_celsius(value=temperature_avg), ) diff --git a/src/frequenz/sdk/timeseries/battery_pool/_result_types.py b/src/frequenz/sdk/timeseries/battery_pool/_result_types.py index 89031f071..728d68a5e 100644 --- a/src/frequenz/sdk/timeseries/battery_pool/_result_types.py +++ b/src/frequenz/sdk/timeseries/battery_pool/_result_types.py @@ -6,8 +6,6 @@ from dataclasses import dataclass, field from datetime import datetime -from ...timeseries import Temperature - @dataclass class Bound: @@ -63,20 +61,3 @@ class PowerMetrics: ) ``` """ - - -@dataclass -class TemperatureMetrics: - """Container for temperature metrics.""" - - timestamp: datetime - """Timestamp of the metrics.""" - - max: Temperature - """Maximum temperature of the collection of temperatures.""" - - min: Temperature - """Minimum temperature of the collection of temperatures.""" - - avg: Temperature - """Average temperature of the collection of temperatures.""" diff --git a/src/frequenz/sdk/timeseries/battery_pool/battery_pool.py b/src/frequenz/sdk/timeseries/battery_pool/battery_pool.py index 2d7a32f65..1f606043a 100644 --- a/src/frequenz/sdk/timeseries/battery_pool/battery_pool.py +++ b/src/frequenz/sdk/timeseries/battery_pool/battery_pool.py @@ -27,7 +27,7 @@ FormulaGeneratorConfig, FormulaType, ) -from .._quantities import Energy, Percentage, Power +from .._quantities import Energy, Percentage, Power, Temperature from ._methods import MetricAggregator, SendOnUpdate from ._metric_calculator import ( CapacityCalculator, @@ -35,7 +35,7 @@ SoCCalculator, TemperatureCalculator, ) -from ._result_types import PowerMetrics, TemperatureMetrics +from ._result_types import PowerMetrics class BatteryPool: @@ -388,11 +388,11 @@ def soc(self) -> MetricAggregator[Sample[Percentage]]: return self._active_methods[method_name] @property - def temperature(self) -> MetricAggregator[TemperatureMetrics]: - """Fetch the maximum temperature of the batteries in the pool. + def temperature(self) -> MetricAggregator[Sample[Temperature]]: + """Fetch the average temperature of the batteries in the pool. Returns: - A MetricAggregator that will calculate and stream the maximum temperature + A MetricAggregator that will calculate and stream the average temperature of all batteries in the pool. """ method_name = SendOnUpdate.name() + "_" + TemperatureCalculator.name() From f10dfc66b214b6ebd3aa050a766dcf300e8da2c3 Mon Sep 17 00:00:00 2001 From: Sahas Subramanian Date: Fri, 28 Jul 2023 17:07:19 +0200 Subject: [PATCH 38/46] Add tests for `battery_pool.temperature` Signed-off-by: Sahas Subramanian --- .../_battery_pool/test_battery_pool.py | 132 +++++++++++++++++- 1 file changed, 131 insertions(+), 1 deletion(-) diff --git a/tests/timeseries/_battery_pool/test_battery_pool.py b/tests/timeseries/_battery_pool/test_battery_pool.py index 81368e83b..225523458 100644 --- a/tests/timeseries/_battery_pool/test_battery_pool.py +++ b/tests/timeseries/_battery_pool/test_battery_pool.py @@ -2,6 +2,7 @@ # Copyright © 2023 Frequenz Energy-as-a-Service GmbH """Tests for battery pool.""" + from __future__ import annotations import asyncio @@ -25,7 +26,7 @@ from frequenz.sdk.actor import ResamplerConfig from frequenz.sdk.actor.power_distributing import BatteryStatus from frequenz.sdk.microgrid.component import ComponentCategory -from frequenz.sdk.timeseries import Energy, Percentage, Power, Sample +from frequenz.sdk.timeseries import Energy, Percentage, Power, Sample, Temperature from frequenz.sdk.timeseries.battery_pool import BatteryPool, Bound, PowerMetrics from frequenz.sdk.timeseries.battery_pool._metric_calculator import ( battery_inverter_mapping, @@ -45,6 +46,8 @@ # pylint doesn't understand fixtures. It thinks it is redefined name. # pylint: disable=redefined-outer-name +# pylint: disable=too-many-lines + @pytest.fixture() def event_loop() -> Iterator[async_solipsism.EventLoop]: @@ -337,6 +340,24 @@ async def test_battery_pool_power_bounds(setup_batteries_pool: SetupArgs) -> Non await run_power_bounds_test(setup_batteries_pool) +async def test_all_batteries_temperature(setup_all_batteries: SetupArgs) -> None: + """Test temperature for battery pool with all components in the microgrid. + + Args: + setup_all_batteries: Fixture that creates needed microgrid tools. + """ + await run_temperature_test(setup_all_batteries) + + +async def test_battery_pool_temperature(setup_batteries_pool: SetupArgs) -> None: + """Test temperature for battery pool with subset of components in the microgrid. + + Args: + setup_all_batteries: Fixture that creates needed microgrid tools. + """ + await run_temperature_test(setup_batteries_pool) + + def assert_dataclass(arg: Any) -> None: """Raise assert error if argument is not dataclass. @@ -981,3 +1002,112 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals streamer.start_streaming(latest_data, sampling_rate=0.1) msg = await asyncio.wait_for(receiver.receive(), timeout=waiting_time_sec) compare_messages(msg, PowerMetrics(now, Bound(-100, 0), Bound(0, 100)), 0.2) + + +async def run_temperature_test( # pylint: disable=too-many-locals + setup_args: SetupArgs, +) -> None: + """Test if temperature metric is working as expected.""" + battery_pool = setup_args.battery_pool + mock_microgrid = setup_args.mock_microgrid + streamer = setup_args.streamer + battery_status_sender = setup_args.battery_status_sender + + all_batteries = get_components(mock_microgrid, ComponentCategory.BATTERY) + await battery_status_sender.send( + BatteryStatus(working=all_batteries, uncertain=set()) + ) + bat_inv_map = battery_inverter_mapping(all_batteries) + + for battery_id, inverter_id in bat_inv_map.items(): + # Sampling rate choose to reflect real application. + streamer.start_streaming( + BatteryDataWrapper( + component_id=battery_id, + timestamp=datetime.now(tz=timezone.utc), + temperature=25.0, + ), + sampling_rate=0.05, + ) + streamer.start_streaming( + InverterDataWrapper( + component_id=inverter_id, + timestamp=datetime.now(tz=timezone.utc), + ), + sampling_rate=0.1, + ) + + receiver = battery_pool.temperature.new_receiver() + + msg = await asyncio.wait_for( + receiver.receive(), timeout=WAIT_FOR_COMPONENT_DATA_SEC + 0.2 + ) + now = datetime.now(tz=timezone.utc) + expected = Sample(now, value=Temperature.from_celsius(25.0)) + compare_messages(msg, expected, WAIT_FOR_COMPONENT_DATA_SEC + 0.2) + + batteries_in_pool = list(battery_pool.battery_ids) + bat_0, bat_1 = batteries_in_pool + scenarios: list[Scenario[Sample[Temperature]]] = [ + Scenario( + bat_0, + {"temperature": 30.0}, + Sample(now, value=Temperature.from_celsius(27.5)), + ), + Scenario( + bat_1, + {"temperature": 20.0}, + Sample(now, value=Temperature.from_celsius(25.0)), + ), + Scenario( + bat_0, + {"temperature": math.nan}, + Sample(now, value=Temperature.from_celsius(20.0)), + ), + Scenario( + bat_1, + {"temperature": math.nan}, + None, + ), + Scenario( + bat_0, + {"temperature": 30.0}, + Sample(now, value=Temperature.from_celsius(30.0)), + ), + Scenario( + bat_1, + {"temperature": 15.0}, + Sample(now, value=Temperature.from_celsius(22.5)), + ), + ] + + waiting_time_sec = setup_args.min_update_interval + 0.02 + await run_scenarios(scenarios, streamer, receiver, waiting_time_sec) + + await run_test_battery_status_channel( + battery_status_sender=battery_status_sender, + battery_pool_metric_receiver=receiver, + all_batteries=all_batteries, + batteries_in_pool=batteries_in_pool, + waiting_time_sec=waiting_time_sec, + all_pool_result=Sample(now, Temperature.from_celsius(22.5)), + only_first_battery_result=Sample(now, Temperature.from_celsius(30.0)), + ) + + # one battery stops sending data. + await streamer.stop_streaming(bat_1) + await asyncio.sleep(MAX_BATTERY_DATA_AGE_SEC + 0.2) + msg = await asyncio.wait_for(receiver.receive(), timeout=waiting_time_sec) + compare_messages(msg, Sample(now, Temperature.from_celsius(30.0)), 0.2) + + # All batteries stopped sending data. + await streamer.stop_streaming(bat_0) + await asyncio.sleep(MAX_BATTERY_DATA_AGE_SEC + 0.2) + msg = await asyncio.wait_for(receiver.receive(), timeout=waiting_time_sec) + assert msg is None + + # one battery started sending data. + latest_data = streamer.get_current_component_data(bat_1) + streamer.start_streaming(latest_data, sampling_rate=0.1) + msg = await asyncio.wait_for(receiver.receive(), timeout=waiting_time_sec) + compare_messages(msg, Sample(now, Temperature.from_celsius(15.0)), 0.2) From f9cc2ab63d5d6424a597c6db0b6c62ed30bad701 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 31 Jul 2023 07:59:45 +0000 Subject: [PATCH 39/46] Bump polars from 0.18.8 to 0.18.9 Bumps [polars](https://github.com/pola-rs/polars) from 0.18.8 to 0.18.9. - [Release notes](https://github.com/pola-rs/polars/releases) - [Commits](https://github.com/pola-rs/polars/compare/py-0.18.8...py-0.18.9) --- updated-dependencies: - dependency-name: polars dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 697bfd280..ab2d9328c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -54,7 +54,7 @@ dev-docstrings = [ "darglint == 1.8.1", "tomli == 2.0.1", # Needed by pydocstyle to read pyproject.toml ] -dev-examples = ["polars == 0.18.8"] +dev-examples = ["polars == 0.18.9"] dev-formatting = ["black == 23.7.0", "isort == 5.12.0"] dev-mkdocs = [ "mike == 1.1.2", From 80ac74ec556f9322e69518918bb3846e5f473d60 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 1 Aug 2023 07:10:42 +0000 Subject: [PATCH 40/46] Bump polars from 0.18.9 to 0.18.11 Bumps [polars](https://github.com/pola-rs/polars) from 0.18.9 to 0.18.11. - [Release notes](https://github.com/pola-rs/polars/releases) - [Commits](https://github.com/pola-rs/polars/compare/py-0.18.9...py-0.18.11) --- updated-dependencies: - dependency-name: polars dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index ab2d9328c..22196fbc8 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -54,7 +54,7 @@ dev-docstrings = [ "darglint == 1.8.1", "tomli == 2.0.1", # Needed by pydocstyle to read pyproject.toml ] -dev-examples = ["polars == 0.18.9"] +dev-examples = ["polars == 0.18.11"] dev-formatting = ["black == 23.7.0", "isort == 5.12.0"] dev-mkdocs = [ "mike == 1.1.2", From 1cd226bf05e73aeceafb34cf1946d603bab00f42 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Tue, 1 Aug 2023 09:56:10 +0200 Subject: [PATCH 41/46] Remove dunder iadd, isub and imul methods for Quantity Having them mutating the objects in-place is not the best option as number are assumed to be immutable in Python. If we make them mutable, then bad things will happen if they are used as default arguments in functions, or if we want to provide a zero singleton for example. We also mention explicitly now that Quantities are immutable. This partially reverts 1043360af7b040674de553b7d663fa02404e5c19 (the tests are kept). Signed-off-by: Leandro Lucarella --- src/frequenz/sdk/timeseries/_quantities.py | 59 ++++------------------ 1 file changed, 10 insertions(+), 49 deletions(-) diff --git a/src/frequenz/sdk/timeseries/_quantities.py b/src/frequenz/sdk/timeseries/_quantities.py index c7fff9b1f..4a79e1116 100644 --- a/src/frequenz/sdk/timeseries/_quantities.py +++ b/src/frequenz/sdk/timeseries/_quantities.py @@ -24,7 +24,10 @@ class Quantity: - """A quantity with a unit.""" + """A quantity with a unit. + + Quantities try to behave like float and are also immutable. + """ _base_value: float """The value of this quantity in the base unit.""" @@ -249,48 +252,6 @@ def __mul__(self, percent: Percentage) -> Self: product._base_value = self._base_value * percent.as_fraction() return product - def __iadd__(self, other: Self) -> Self: - """Add another quantity to this one. - - Args: - other: The other quantity. - - Returns: - This quantity. - """ - if not type(other) is type(self): - return NotImplemented - self._base_value += other._base_value - return self - - def __isub__(self, other: Self) -> Self: - """Subtract another quantity from this one. - - Args: - other: The other quantity. - - Returns: - This quantity. - """ - if not type(other) is type(self): - return NotImplemented - self._base_value -= other._base_value - return self - - def __imul__(self, percent: Percentage) -> Self: - """Multiply this quantity by a percentage. - - Args: - percent: The percentage. - - Returns: - This quantity. - """ - if not isinstance(percent, Percentage): - return NotImplemented - self._base_value *= percent.as_fraction() - return self - def __gt__(self, other: Self) -> bool: """Return whether this quantity is greater than another. @@ -413,7 +374,7 @@ class Power( ): """A power quantity. - Objects of this type are wrappers around `float` values. + Objects of this type are wrappers around `float` values and are immutable. The constructors accept a single `float` value, the `as_*()` methods return a `float` value, and each of the arithmetic operators supported by this type are @@ -586,7 +547,7 @@ class Current( ): """A current quantity. - Objects of this type are wrappers around `float` values. + Objects of this type are wrappers around `float` values and are immutable. The constructors accept a single `float` value, the `as_*()` methods return a `float` value, and each of the arithmetic operators supported by this type are @@ -682,7 +643,7 @@ class Voltage( ): """A voltage quantity. - Objects of this type are wrappers around `float` values. + Objects of this type are wrappers around `float` values and are immutable. The constructors accept a single `float` value, the `as_*()` methods return a `float` value, and each of the arithmetic operators supported by this type are @@ -804,7 +765,7 @@ class Energy( ): """An energy quantity. - Objects of this type are wrappers around `float` values. + Objects of this type are wrappers around `float` values and are immutable. The constructors accept a single `float` value, the `as_*()` methods return a `float` value, and each of the arithmetic operators supported by this type are @@ -923,7 +884,7 @@ class Frequency( ): """A frequency quantity. - Objects of this type are wrappers around `float` values. + Objects of this type are wrappers around `float` values and are immutable. The constructors accept a single `float` value, the `as_*()` methods return a `float` value, and each of the arithmetic operators supported by this type are @@ -1036,7 +997,7 @@ class Percentage( ): """A percentage quantity. - Objects of this type are wrappers around `float` values. + Objects of this type are wrappers around `float` values and are immutable. The constructors accept a single `float` value, the `as_*()` methods return a `float` value, and each of the arithmetic operators supported by this type are From 361bcb0ebee9fab041baea27aa51a738208d5991 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Wed, 26 Jul 2023 12:31:29 +0200 Subject: [PATCH 42/46] Add zero() constructor for Quantities It is a very common case which becomes very verbose, and it includes unit information that is completely irrelevant (zero is zero in any unit). Signed-off-by: Leandro Lucarella --- RELEASE_NOTES.md | 14 ++++-- src/frequenz/sdk/timeseries/_quantities.py | 24 ++++++++++ tests/timeseries/test_quantities.py | 56 ++++++++++++++++++++++ 3 files changed, 89 insertions(+), 5 deletions(-) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index a6595d094..9f5230ec4 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -13,11 +13,15 @@ ## New Features -- Add `abs()` support for quantities. -- Add quantity class `Frequency` for frequency values. -- Quantities can now be multiplied with `Percentage` types. -- `FormulaEngine` arithmetics now supports scalar multiplication with floats and addition with Quantities. -- Add a `isclose()` method on quantities to compare them to other values of the same type. Because `Quantity` types are just wrappers around `float`s, direct comparison might not always be desirable. +- Quantities + + * Add `abs()`. + * Add a `isclose()` method on quantities to compare them to other values of the same type. Because `Quantity` types are just wrappers around `float`s, direct comparison might not always be desirable. + * Add `zero()` constructor (which returns a singleton) to easily get a zero value. + * Add multiplication by `Percentage` types. + * Add a new quantity class `Frequency` for frequency values. + +- `FormulaEngine` arithmetics now supports scalar multiplication with floats and addition with Quantities ## Bug Fixes diff --git a/src/frequenz/sdk/timeseries/_quantities.py b/src/frequenz/sdk/timeseries/_quantities.py index 4a79e1116..9cdf124e0 100644 --- a/src/frequenz/sdk/timeseries/_quantities.py +++ b/src/frequenz/sdk/timeseries/_quantities.py @@ -65,6 +65,30 @@ def __init_subclass__(cls, exponent_unit_map: dict[int, str]) -> None: cls._exponent_unit_map = exponent_unit_map super().__init_subclass__() + _zero_cache: dict[type, Quantity] = {} + """Cache for zero singletons. + + This is a workaround for mypy getting confused when using @functools.cache and + @classmethod combined with returning Self. It believes the resulting type of this + method is Self and complains that members of the actual class don't exist in Self, + so we need to implement the cache ourselves. + """ + + @classmethod + def zero(cls) -> Self: + """Return a quantity with value 0. + + Returns: + A quantity with value 0. + """ + _zero = cls._zero_cache.get(cls, None) + if _zero is None: + _zero = cls.__new__(cls) + _zero._base_value = 0 + cls._zero_cache[cls] = _zero + assert isinstance(_zero, cls) + return _zero + @property def base_value(self) -> float: """Return the value of this quantity in the base unit. diff --git a/tests/timeseries/test_quantities.py b/tests/timeseries/test_quantities.py index a7b3d02bf..e7588d1f7 100644 --- a/tests/timeseries/test_quantities.py +++ b/tests/timeseries/test_quantities.py @@ -42,6 +42,62 @@ class Fz2( """Frequency quantity with broad exponent unit map.""" +def test_zero() -> None: + """Test the zero value for quantity.""" + assert Quantity(0.0) == Quantity.zero() + assert Quantity(0.0, exponent=100) == Quantity.zero() + assert Quantity.zero() is Quantity.zero() # It is a "singleton" + assert Quantity.zero().base_value == 0.0 + + # Test the singleton is immutable + one = Quantity.zero() + one += Quantity(1.0) + assert one != Quantity.zero() + assert Quantity.zero() == Quantity(0.0) + + assert Power.from_watts(0.0) == Power.zero() + assert Power.from_kilowatts(0.0) == Power.zero() + assert isinstance(Power.zero(), Power) + assert Power.zero().as_watts() == 0.0 + assert Power.zero().as_kilowatts() == 0.0 + assert Power.zero() is Power.zero() # It is a "singleton" + + assert Current.from_amperes(0.0) == Current.zero() + assert Current.from_milliamperes(0.0) == Current.zero() + assert isinstance(Current.zero(), Current) + assert Current.zero().as_amperes() == 0.0 + assert Current.zero().as_milliamperes() == 0.0 + assert Current.zero() is Current.zero() # It is a "singleton" + + assert Voltage.from_volts(0.0) == Voltage.zero() + assert Voltage.from_kilovolts(0.0) == Voltage.zero() + assert isinstance(Voltage.zero(), Voltage) + assert Voltage.zero().as_volts() == 0.0 + assert Voltage.zero().as_kilovolts() == 0.0 + assert Voltage.zero() is Voltage.zero() # It is a "singleton" + + assert Energy.from_kilowatt_hours(0.0) == Energy.zero() + assert Energy.from_megawatt_hours(0.0) == Energy.zero() + assert isinstance(Energy.zero(), Energy) + assert Energy.zero().as_kilowatt_hours() == 0.0 + assert Energy.zero().as_megawatt_hours() == 0.0 + assert Energy.zero() is Energy.zero() # It is a "singleton" + + assert Frequency.from_hertz(0.0) == Frequency.zero() + assert Frequency.from_megahertz(0.0) == Frequency.zero() + assert isinstance(Frequency.zero(), Frequency) + assert Frequency.zero().as_hertz() == 0.0 + assert Frequency.zero().as_megahertz() == 0.0 + assert Frequency.zero() is Frequency.zero() # It is a "singleton" + + assert Percentage.from_percent(0.0) == Percentage.zero() + assert Percentage.from_fraction(0.0) == Percentage.zero() + assert isinstance(Percentage.zero(), Percentage) + assert Percentage.zero().as_percent() == 0.0 + assert Percentage.zero().as_fraction() == 0.0 + assert Percentage.zero() is Percentage.zero() # It is a "singleton" + + def test_string_representation() -> None: """Test the string representation of the quantities.""" assert str(Quantity(1.024445, exponent=0)) == "1.024" From 228700a3ff618ced70eccfae64b7c21f7a1f4e80 Mon Sep 17 00:00:00 2001 From: Sahas Subramanian Date: Tue, 1 Aug 2023 11:39:54 +0200 Subject: [PATCH 43/46] Update RELEASE_NOTES.md Signed-off-by: Sahas Subramanian --- RELEASE_NOTES.md | 1 + 1 file changed, 1 insertion(+) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index a6595d094..d8248b7f4 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -18,6 +18,7 @@ - Quantities can now be multiplied with `Percentage` types. - `FormulaEngine` arithmetics now supports scalar multiplication with floats and addition with Quantities. - Add a `isclose()` method on quantities to compare them to other values of the same type. Because `Quantity` types are just wrappers around `float`s, direct comparison might not always be desirable. +- Add a new method for streaming average temperature values for the battery pool. ## Bug Fixes From 18036e40a2431513b3c1ea4091b96759e180ed46 Mon Sep 17 00:00:00 2001 From: Sahas Subramanian Date: Tue, 1 Aug 2023 11:49:42 +0200 Subject: [PATCH 44/46] Update src/frequenz/sdk/microgrid/component/_component_data.py Co-authored-by: Leandro Lucarella Signed-off-by: Sahas Subramanian --- src/frequenz/sdk/microgrid/component/_component_data.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/frequenz/sdk/microgrid/component/_component_data.py b/src/frequenz/sdk/microgrid/component/_component_data.py index 81696a8f9..c7b5689d5 100644 --- a/src/frequenz/sdk/microgrid/component/_component_data.py +++ b/src/frequenz/sdk/microgrid/component/_component_data.py @@ -142,7 +142,7 @@ class BatteryData(ComponentData): """ temperature: float - """The (average) temperature reported by the battery a battery, in Celcius (°C).""" + """The (average) temperature reported by the battery, in Celcius (°C).""" _relay_state: battery_pb.RelayState.ValueType """State of the battery relay.""" From 2a240fe13c4ef954c663e19bd1b3958943300b04 Mon Sep 17 00:00:00 2001 From: Matthias Wende Date: Tue, 1 Aug 2023 18:03:37 +0200 Subject: [PATCH 45/46] Update battery pool SoC calculation Fix the SoC calculation such that it never exceeds 100%. Signed-off-by: Matthias Wende --- .../sdk/timeseries/battery_pool/_metric_calculator.py | 3 ++- src/frequenz/sdk/timeseries/battery_pool/battery_pool.py | 7 ++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/frequenz/sdk/timeseries/battery_pool/_metric_calculator.py b/src/frequenz/sdk/timeseries/battery_pool/_metric_calculator.py index e2cd3ce60..9165f4498 100644 --- a/src/frequenz/sdk/timeseries/battery_pool/_metric_calculator.py +++ b/src/frequenz/sdk/timeseries/battery_pool/_metric_calculator.py @@ -424,7 +424,8 @@ def calculate( soc_scaled = ( (soc - soc_lower_bound) / (soc_upper_bound - soc_lower_bound) * 100 ) - soc_scaled = max(soc_scaled, 0) + # we are clamping here because the SoC might be out of bounds + soc_scaled = min(max(soc_scaled, 0), 100) timestamp = max(timestamp, metrics.timestamp) used_capacity_x100 += usable_capacity_x100 * soc_scaled total_capacity_x100 += usable_capacity_x100 diff --git a/src/frequenz/sdk/timeseries/battery_pool/battery_pool.py b/src/frequenz/sdk/timeseries/battery_pool/battery_pool.py index 1f606043a..6fc3ae54e 100644 --- a/src/frequenz/sdk/timeseries/battery_pool/battery_pool.py +++ b/src/frequenz/sdk/timeseries/battery_pool/battery_pool.py @@ -347,16 +347,17 @@ def consumption_power(self) -> FormulaEngine[Power]: def soc(self) -> MetricAggregator[Sample[Percentage]]: """Fetch the normalized average weighted-by-capacity SoC values for the pool. - The values are normalized to the 0-100% range. + The values are normalized to the 0-100% range and clamped if the SoC is out of + bounds. Average soc is calculated with the formula: ``` working_batteries: Set[BatteryData] # working batteries from the battery pool - soc_scaled = max( + soc_scaled = min(max( 0, (soc - soc_lower_bound) / (soc_upper_bound - soc_lower_bound) * 100, - ) + ), 100) used_capacity = sum( battery.usable_capacity * battery.soc_scaled for battery in working_batteries From 83888c595d4b6cdb5fd42c37d99c27e53109dd53 Mon Sep 17 00:00:00 2001 From: Matthias Wende Date: Wed, 2 Aug 2023 09:58:46 +0200 Subject: [PATCH 46/46] Update release notes Signed-off-by: Matthias Wende --- RELEASE_NOTES.md | 1 + 1 file changed, 1 insertion(+) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index a88247588..f8328fd6c 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -28,5 +28,6 @@ - Fix formatting issue for `Quantity` objects with zero values. - Fix formatting isuse for `Quantity` when the base value is float.inf or float.nan. +- Fix clamping to 100% for the battery pool SoC scaling calculation.