From 5ef4d130c225a8bfb8ab0feb3046011c331d3fae Mon Sep 17 00:00:00 2001 From: Sahas Subramanian Date: Thu, 20 Jul 2023 11:55:44 +0200 Subject: [PATCH 1/9] Rename internal inclusion-bound variables to latest naming scheme Signed-off-by: Sahas Subramanian --- .../battery_pool/_metric_calculator.py | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/frequenz/sdk/timeseries/battery_pool/_metric_calculator.py b/src/frequenz/sdk/timeseries/battery_pool/_metric_calculator.py index 4908b6d70..19e087eae 100644 --- a/src/frequenz/sdk/timeseries/battery_pool/_metric_calculator.py +++ b/src/frequenz/sdk/timeseries/battery_pool/_metric_calculator.py @@ -542,8 +542,8 @@ def calculate( ) for battery_id in working_batteries: - supply_upper_bounds: list[float] = [] - consume_upper_bounds: list[float] = [] + inclusion_lower_bounds: list[float] = [] + inclusion_upper_bounds: list[float] = [] if battery_id in metrics_data: data = metrics_data[battery_id] @@ -553,12 +553,12 @@ def calculate( value = data.get(ComponentMetricId.POWER_INCLUSION_UPPER_BOUND) if value is not None: result.timestamp = max(result.timestamp, data.timestamp) - consume_upper_bounds.append(value) + inclusion_upper_bounds.append(value) value = data.get(ComponentMetricId.POWER_INCLUSION_LOWER_BOUND) if value is not None: result.timestamp = max(result.timestamp, data.timestamp) - supply_upper_bounds.append(value) + inclusion_lower_bounds.append(value) inverter_id = self._bat_inv_map[battery_id] if inverter_id in metrics_data: @@ -567,17 +567,17 @@ def calculate( value = data.get(ComponentMetricId.ACTIVE_POWER_INCLUSION_UPPER_BOUND) if value is not None: result.timestamp = max(data.timestamp, result.timestamp) - consume_upper_bounds.append(value) + inclusion_upper_bounds.append(value) value = data.get(ComponentMetricId.ACTIVE_POWER_INCLUSION_LOWER_BOUND) if value is not None: result.timestamp = max(data.timestamp, result.timestamp) - supply_upper_bounds.append(value) + inclusion_lower_bounds.append(value) - if len(consume_upper_bounds) > 0: - result.consume_bound.upper += min(consume_upper_bounds) - if len(supply_upper_bounds) > 0: - result.supply_bound.lower += max(supply_upper_bounds) + if len(inclusion_upper_bounds) > 0: + result.consume_bound.upper += min(inclusion_upper_bounds) + if len(inclusion_lower_bounds) > 0: + result.supply_bound.lower += max(inclusion_lower_bounds) if result.timestamp == _MIN_TIMESTAMP: return None From f4e51f2c1d7eb5c2fad46dd4f609561f920f4c16 Mon Sep 17 00:00:00 2001 From: Sahas Subramanian Date: Thu, 20 Jul 2023 13:20:58 +0200 Subject: [PATCH 2/9] Support fetching of exclusion bounds from the data sourcing actor Signed-off-by: Sahas Subramanian --- .../sdk/actor/_data_sourcing/microgrid_api_source.py | 12 ++++++++++++ src/frequenz/sdk/microgrid/component/_component.py | 4 ++++ 2 files changed, 16 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 f110aabd8..196c56a30 100644 --- a/src/frequenz/sdk/actor/_data_sourcing/microgrid_api_source.py +++ b/src/frequenz/sdk/actor/_data_sourcing/microgrid_api_source.py @@ -85,6 +85,12 @@ def get_channel_name(self) -> str: ComponentMetricId.POWER_INCLUSION_LOWER_BOUND: lambda msg: ( msg.power_inclusion_lower_bound ), + ComponentMetricId.POWER_EXCLUSION_LOWER_BOUND: lambda msg: ( + msg.power_exclusion_lower_bound + ), + ComponentMetricId.POWER_EXCLUSION_UPPER_BOUND: lambda msg: ( + msg.power_exclusion_upper_bound + ), ComponentMetricId.POWER_INCLUSION_UPPER_BOUND: lambda msg: ( msg.power_inclusion_upper_bound ), @@ -96,6 +102,12 @@ def get_channel_name(self) -> str: ComponentMetricId.ACTIVE_POWER_INCLUSION_LOWER_BOUND: lambda msg: ( msg.active_power_inclusion_lower_bound ), + ComponentMetricId.ACTIVE_POWER_EXCLUSION_LOWER_BOUND: lambda msg: ( + msg.active_power_exclusion_lower_bound + ), + ComponentMetricId.ACTIVE_POWER_EXCLUSION_UPPER_BOUND: lambda msg: ( + msg.active_power_exclusion_upper_bound + ), ComponentMetricId.ACTIVE_POWER_INCLUSION_UPPER_BOUND: lambda msg: ( msg.active_power_inclusion_upper_bound ), diff --git a/src/frequenz/sdk/microgrid/component/_component.py b/src/frequenz/sdk/microgrid/component/_component.py index 5dc903154..3a3c24673 100644 --- a/src/frequenz/sdk/microgrid/component/_component.py +++ b/src/frequenz/sdk/microgrid/component/_component.py @@ -145,9 +145,13 @@ class ComponentMetricId(Enum): CAPACITY = "capacity" POWER_INCLUSION_LOWER_BOUND = "power_inclusion_lower_bound" + POWER_EXCLUSION_LOWER_BOUND = "power_exclusion_lower_bound" + POWER_EXCLUSION_UPPER_BOUND = "power_exclusion_upper_bound" POWER_INCLUSION_UPPER_BOUND = "power_inclusion_upper_bound" ACTIVE_POWER_INCLUSION_LOWER_BOUND = "active_power_inclusion_lower_bound" + ACTIVE_POWER_EXCLUSION_LOWER_BOUND = "active_power_exclusion_lower_bound" + ACTIVE_POWER_EXCLUSION_UPPER_BOUND = "active_power_exclusion_upper_bound" ACTIVE_POWER_INCLUSION_UPPER_BOUND = "active_power_inclusion_upper_bound" TEMPERATURE = "temperature" From 6bf48057fc6ff2a20e0bde74523bb55b882b9811 Mon Sep 17 00:00:00 2001 From: Sahas Subramanian Date: Thu, 20 Jul 2023 13:45:02 +0200 Subject: [PATCH 3/9] Rename BatteryPool.Bound to `Bounds` It is a class that contains lower and upper bounds, so plural is better. There are other breaking changes related to inclusion and exclusion bounds, thar are coming up in subsequent commits, so now is a nice opportunity to make this change. Signed-off-by: Sahas Subramanian --- .../sdk/timeseries/battery_pool/__init__.py | 4 +- .../battery_pool/_metric_calculator.py | 6 +-- .../timeseries/battery_pool/_result_types.py | 6 +-- .../_battery_pool/test_battery_pool.py | 42 +++++++++---------- 4 files changed, 29 insertions(+), 29 deletions(-) diff --git a/src/frequenz/sdk/timeseries/battery_pool/__init__.py b/src/frequenz/sdk/timeseries/battery_pool/__init__.py index 56f07eda2..1550595b4 100644 --- a/src/frequenz/sdk/timeseries/battery_pool/__init__.py +++ b/src/frequenz/sdk/timeseries/battery_pool/__init__.py @@ -3,11 +3,11 @@ """Manage a pool of batteries.""" -from ._result_types import Bound, PowerMetrics +from ._result_types import Bounds, PowerMetrics from .battery_pool import BatteryPool __all__ = [ "BatteryPool", "PowerMetrics", - "Bound", + "Bounds", ] diff --git a/src/frequenz/sdk/timeseries/battery_pool/_metric_calculator.py b/src/frequenz/sdk/timeseries/battery_pool/_metric_calculator.py index 19e087eae..d07dbde44 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 Bounds, PowerMetrics _logger = logging.getLogger(__name__) _MIN_TIMESTAMP = datetime.min.replace(tzinfo=timezone.utc) @@ -537,8 +537,8 @@ def calculate( result = PowerMetrics( timestamp=_MIN_TIMESTAMP, - supply_bound=Bound(0, 0), - consume_bound=Bound(0, 0), + supply_bound=Bounds(0, 0), + consume_bound=Bounds(0, 0), ) for battery_id in working_batteries: diff --git a/src/frequenz/sdk/timeseries/battery_pool/_result_types.py b/src/frequenz/sdk/timeseries/battery_pool/_result_types.py index 7c969c179..3241c9ecc 100644 --- a/src/frequenz/sdk/timeseries/battery_pool/_result_types.py +++ b/src/frequenz/sdk/timeseries/battery_pool/_result_types.py @@ -8,7 +8,7 @@ @dataclass -class Bound: +class Bounds: """Lower and upper bound values.""" lower: float @@ -26,7 +26,7 @@ class PowerMetrics: timestamp: datetime = field(compare=False) """Timestamp of the metrics.""" - supply_bound: Bound + supply_bound: Bounds """Supply power bounds. Upper bound is always 0 and will be supported later. @@ -44,7 +44,7 @@ class PowerMetrics: ``` """ - consume_bound: Bound + consume_bound: Bounds """Consume power bounds. Lower bound is always 0 and will be supported later. diff --git a/tests/timeseries/_battery_pool/test_battery_pool.py b/tests/timeseries/_battery_pool/test_battery_pool.py index f1aa70598..4fcfed9ad 100644 --- a/tests/timeseries/_battery_pool/test_battery_pool.py +++ b/tests/timeseries/_battery_pool/test_battery_pool.py @@ -27,7 +27,7 @@ from frequenz.sdk.actor.power_distributing import BatteryStatus from frequenz.sdk.microgrid.component import ComponentCategory 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 import BatteryPool, Bounds, PowerMetrics from frequenz.sdk.timeseries.battery_pool._metric_calculator import ( battery_inverter_mapping, ) @@ -853,8 +853,8 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals now = datetime.now(tz=timezone.utc) expected = PowerMetrics( timestamp=now, - supply_bound=Bound(-1800, 0), - consume_bound=Bound(0, 10000), + supply_bound=Bounds(-1800, 0), + consume_bound=Bounds(0, 10000), ) compare_messages(msg, expected, WAIT_FOR_COMPONENT_DATA_SEC + 0.2) @@ -863,7 +863,7 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals Scenario( bat_inv_map[batteries_in_pool[0]], {"active_power_inclusion_lower_bound": -100}, - PowerMetrics(now, Bound(-1000, 0), Bound(0, 10000)), + PowerMetrics(now, Bounds(-1000, 0), Bounds(0, 10000)), ), # Inverter bound changed, but metric result should not change. Scenario( @@ -875,12 +875,12 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals Scenario( batteries_in_pool[0], {"power_inclusion_lower_bound": 0, "power_inclusion_upper_bound": 4000}, - PowerMetrics(now, Bound(-900, 0), Bound(0, 9000)), + PowerMetrics(now, Bounds(-900, 0), Bounds(0, 9000)), ), Scenario( batteries_in_pool[1], {"power_inclusion_lower_bound": -10, "power_inclusion_upper_bound": 200}, - PowerMetrics(now, Bound(-10, 0), Bound(0, 4200)), + PowerMetrics(now, Bounds(-10, 0), Bounds(0, 4200)), ), # Test 2 things: # 1. Battery is sending upper bounds=NaN, use only inverter upper bounds @@ -892,7 +892,7 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals "power_inclusion_lower_bound": -50, "power_inclusion_upper_bound": math.nan, }, - PowerMetrics(now, Bound(-60, 0), Bound(0, 9200)), + PowerMetrics(now, Bounds(-60, 0), Bounds(0, 9200)), ), Scenario( bat_inv_map[batteries_in_pool[0]], @@ -900,12 +900,12 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals "active_power_inclusion_lower_bound": math.nan, "active_power_inclusion_upper_bound": math.nan, }, - PowerMetrics(now, Bound(-60, 0), Bound(0, 200)), + PowerMetrics(now, Bounds(-60, 0), Bounds(0, 200)), ), Scenario( batteries_in_pool[0], {"power_inclusion_lower_bound": math.nan}, - PowerMetrics(now, Bound(-10, 0), Bound(0, 200)), + PowerMetrics(now, Bounds(-10, 0), Bounds(0, 200)), ), Scenario( batteries_in_pool[1], @@ -913,7 +913,7 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals "power_inclusion_lower_bound": -100, "power_inclusion_upper_bound": math.nan, }, - PowerMetrics(now, Bound(-100, 0), Bound(0, 6000)), + PowerMetrics(now, Bounds(-100, 0), Bounds(0, 6000)), ), Scenario( bat_inv_map[batteries_in_pool[1]], @@ -921,7 +921,7 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals "active_power_inclusion_lower_bound": math.nan, "active_power_inclusion_upper_bound": math.nan, }, - PowerMetrics(now, Bound(-100, 0), Bound(0, 0)), + PowerMetrics(now, Bounds(-100, 0), Bounds(0, 0)), ), # All components are sending NaN, can't calculate bounds Scenario( @@ -935,7 +935,7 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals Scenario( batteries_in_pool[0], {"power_inclusion_lower_bound": -100, "power_inclusion_upper_bound": 100}, - PowerMetrics(now, Bound(-100, 0), Bound(0, 100)), + PowerMetrics(now, Bounds(-100, 0), Bounds(0, 100)), ), Scenario( bat_inv_map[batteries_in_pool[1]], @@ -943,7 +943,7 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals "active_power_inclusion_lower_bound": -400, "active_power_inclusion_upper_bound": 400, }, - PowerMetrics(now, Bound(-500, 0), Bound(0, 500)), + PowerMetrics(now, Bounds(-500, 0), Bounds(0, 500)), ), Scenario( batteries_in_pool[1], @@ -951,7 +951,7 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals "power_inclusion_lower_bound": -300, "power_inclusion_upper_bound": 700, }, - PowerMetrics(now, Bound(-400, 0), Bound(0, 500)), + PowerMetrics(now, Bounds(-400, 0), Bounds(0, 500)), ), Scenario( bat_inv_map[batteries_in_pool[0]], @@ -959,7 +959,7 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals "active_power_inclusion_lower_bound": -200, "active_power_inclusion_upper_bound": 50, }, - PowerMetrics(now, Bound(-400, 0), Bound(0, 450)), + PowerMetrics(now, Bounds(-400, 0), Bounds(0, 450)), ), ] @@ -972,27 +972,27 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals all_batteries=all_batteries, batteries_in_pool=batteries_in_pool, waiting_time_sec=waiting_time_sec, - all_pool_result=PowerMetrics(now, Bound(-400, 0), Bound(0, 450)), - only_first_battery_result=PowerMetrics(now, Bound(-100, 0), Bound(0, 50)), + all_pool_result=PowerMetrics(now, Bounds(-400, 0), Bounds(0, 450)), + only_first_battery_result=PowerMetrics(now, Bounds(-100, 0), Bounds(0, 50)), ) # One battery stopped sending data, inverter data should be used. await streamer.stop_streaming(batteries_in_pool[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, PowerMetrics(now, Bound(-500, 0), Bound(0, 450)), 0.2) + compare_messages(msg, PowerMetrics(now, Bounds(-500, 0), Bounds(0, 450)), 0.2) # All batteries stopped sending data, use inverters only. await streamer.stop_streaming(batteries_in_pool[0]) await asyncio.sleep(MAX_BATTERY_DATA_AGE_SEC + 0.2) msg = await asyncio.wait_for(receiver.receive(), timeout=waiting_time_sec) - compare_messages(msg, PowerMetrics(now, Bound(-600, 0), Bound(0, 450)), 0.2) + compare_messages(msg, PowerMetrics(now, Bounds(-600, 0), Bounds(0, 450)), 0.2) # One inverter stopped sending data, use one remaining inverter await streamer.stop_streaming(bat_inv_map[batteries_in_pool[0]]) await asyncio.sleep(MAX_BATTERY_DATA_AGE_SEC + 0.2) msg = await asyncio.wait_for(receiver.receive(), timeout=waiting_time_sec) - compare_messages(msg, PowerMetrics(now, Bound(-400, 0), Bound(0, 400)), 0.2) + compare_messages(msg, PowerMetrics(now, Bounds(-400, 0), Bounds(0, 400)), 0.2) # All components stopped sending data, we can assume that power bounds are 0 await streamer.stop_streaming(bat_inv_map[batteries_in_pool[1]]) @@ -1004,7 +1004,7 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals latest_data = streamer.get_current_component_data(batteries_in_pool[0]) 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) + compare_messages(msg, PowerMetrics(now, Bounds(-100, 0), Bounds(0, 100)), 0.2) async def run_temperature_test( # pylint: disable=too-many-locals From 77a45481065f9bcf7a79a8ecdb29042705aaf409 Mon Sep 17 00:00:00 2001 From: Sahas Subramanian Date: Fri, 21 Jul 2023 15:25:35 +0200 Subject: [PATCH 4/9] Rename fields to power inclusion/exclusion bounds, in battery pool `inclusion/exclusion` bounds are a better representation of the passive sign convention, than `supply/consume` bounds. But `supply/consume` bounds are not gone for ever, they will be re-implemented as unsigned `charge/discharge` bounds in a separate PR. Signed-off-by: Sahas Subramanian --- .../battery_pool/_metric_calculator.py | 8 +-- .../timeseries/battery_pool/_result_types.py | 50 +++++++------------ .../_battery_pool/test_battery_pool.py | 38 +++++++------- 3 files changed, 41 insertions(+), 55 deletions(-) diff --git a/src/frequenz/sdk/timeseries/battery_pool/_metric_calculator.py b/src/frequenz/sdk/timeseries/battery_pool/_metric_calculator.py index d07dbde44..c0b2fed61 100644 --- a/src/frequenz/sdk/timeseries/battery_pool/_metric_calculator.py +++ b/src/frequenz/sdk/timeseries/battery_pool/_metric_calculator.py @@ -537,8 +537,8 @@ def calculate( result = PowerMetrics( timestamp=_MIN_TIMESTAMP, - supply_bound=Bounds(0, 0), - consume_bound=Bounds(0, 0), + inclusion_bounds=Bounds(0, 0), + exclusion_bounds=Bounds(0, 0), ) for battery_id in working_batteries: @@ -575,9 +575,9 @@ def calculate( inclusion_lower_bounds.append(value) if len(inclusion_upper_bounds) > 0: - result.consume_bound.upper += min(inclusion_upper_bounds) + result.inclusion_bounds.upper += min(inclusion_upper_bounds) if len(inclusion_lower_bounds) > 0: - result.supply_bound.lower += max(inclusion_lower_bounds) + result.inclusion_bounds.lower += max(inclusion_lower_bounds) if result.timestamp == _MIN_TIMESTAMP: return None diff --git a/src/frequenz/sdk/timeseries/battery_pool/_result_types.py b/src/frequenz/sdk/timeseries/battery_pool/_result_types.py index 3241c9ecc..a5d2500c4 100644 --- a/src/frequenz/sdk/timeseries/battery_pool/_result_types.py +++ b/src/frequenz/sdk/timeseries/battery_pool/_result_types.py @@ -26,38 +26,24 @@ class PowerMetrics: timestamp: datetime = field(compare=False) """Timestamp of the metrics.""" - supply_bound: Bounds - """Supply power bounds. - - Upper bound is always 0 and will be supported later. - Lower bound is negative number calculated with with the formula: - ```python - working_pairs: Set[BatteryData, InverterData] # working batteries from the battery - pool and adjacent inverters - - supply_bound.lower = sum( - max( - battery.power_inclusion_lower_bound, inverter.active_power_inclusion_lower_bound) - for each working battery in battery pool - ) - ) - ``` + # pylint: disable=line-too-long + inclusion_bounds: Bounds + """Inclusion power bounds for all batteries in the battery pool instance. + + This is the range within which power requests are allowed by the battery pool. + + When exclusion bounds are present, they will exclude a subset of the inclusion + bounds. + + More details [here](https://github.com/frequenz-floss/frequenz-api-common/blob/v0.3.0/proto/frequenz/api/common/metrics.proto#L37-L91). """ - consume_bound: Bounds - """Consume power bounds. - - Lower bound is always 0 and will be supported later. - Upper bound is positive number calculated with with the formula: - ```python - working_pairs: Set[BatteryData, InverterData] # working batteries from the battery - pool and adjacent inverters - - consume_bound.upper = sum( - min( - battery.power_inclusion_upper_bound, inverter.active_power_inclusion_upper_bound) - for each working battery in battery pool - ) - ) - ``` + exclusion_bounds: Bounds + """Exclusion power bounds for all batteries in the battery pool instance. + + This is the range within which power requests are NOT allowed by the battery pool. + If present, they will be a subset of the inclusion bounds. + + More details [here](https://github.com/frequenz-floss/frequenz-api-common/blob/v0.3.0/proto/frequenz/api/common/metrics.proto#L37-L91). """ + # pylint: enable=line-too-long diff --git a/tests/timeseries/_battery_pool/test_battery_pool.py b/tests/timeseries/_battery_pool/test_battery_pool.py index 4fcfed9ad..fdc450c4e 100644 --- a/tests/timeseries/_battery_pool/test_battery_pool.py +++ b/tests/timeseries/_battery_pool/test_battery_pool.py @@ -853,8 +853,8 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals now = datetime.now(tz=timezone.utc) expected = PowerMetrics( timestamp=now, - supply_bound=Bounds(-1800, 0), - consume_bound=Bounds(0, 10000), + inclusion_bounds=Bounds(-1800, 10000), + exclusion_bounds=Bounds(0, 0), ) compare_messages(msg, expected, WAIT_FOR_COMPONENT_DATA_SEC + 0.2) @@ -863,7 +863,7 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals Scenario( bat_inv_map[batteries_in_pool[0]], {"active_power_inclusion_lower_bound": -100}, - PowerMetrics(now, Bounds(-1000, 0), Bounds(0, 10000)), + PowerMetrics(now, Bounds(-1000, 10000), Bounds(0, 0)), ), # Inverter bound changed, but metric result should not change. Scenario( @@ -875,12 +875,12 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals Scenario( batteries_in_pool[0], {"power_inclusion_lower_bound": 0, "power_inclusion_upper_bound": 4000}, - PowerMetrics(now, Bounds(-900, 0), Bounds(0, 9000)), + PowerMetrics(now, Bounds(-900, 9000), Bounds(0, 0)), ), Scenario( batteries_in_pool[1], {"power_inclusion_lower_bound": -10, "power_inclusion_upper_bound": 200}, - PowerMetrics(now, Bounds(-10, 0), Bounds(0, 4200)), + PowerMetrics(now, Bounds(-10, 4200), Bounds(0, 0)), ), # Test 2 things: # 1. Battery is sending upper bounds=NaN, use only inverter upper bounds @@ -892,7 +892,7 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals "power_inclusion_lower_bound": -50, "power_inclusion_upper_bound": math.nan, }, - PowerMetrics(now, Bounds(-60, 0), Bounds(0, 9200)), + PowerMetrics(now, Bounds(-60, 9200), Bounds(0, 0)), ), Scenario( bat_inv_map[batteries_in_pool[0]], @@ -900,12 +900,12 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals "active_power_inclusion_lower_bound": math.nan, "active_power_inclusion_upper_bound": math.nan, }, - PowerMetrics(now, Bounds(-60, 0), Bounds(0, 200)), + PowerMetrics(now, Bounds(-60, 200), Bounds(0, 0)), ), Scenario( batteries_in_pool[0], {"power_inclusion_lower_bound": math.nan}, - PowerMetrics(now, Bounds(-10, 0), Bounds(0, 200)), + PowerMetrics(now, Bounds(-10, 200), Bounds(0, 0)), ), Scenario( batteries_in_pool[1], @@ -913,7 +913,7 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals "power_inclusion_lower_bound": -100, "power_inclusion_upper_bound": math.nan, }, - PowerMetrics(now, Bounds(-100, 0), Bounds(0, 6000)), + PowerMetrics(now, Bounds(-100, 6000), Bounds(0, 0)), ), Scenario( bat_inv_map[batteries_in_pool[1]], @@ -935,7 +935,7 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals Scenario( batteries_in_pool[0], {"power_inclusion_lower_bound": -100, "power_inclusion_upper_bound": 100}, - PowerMetrics(now, Bounds(-100, 0), Bounds(0, 100)), + PowerMetrics(now, Bounds(-100, 100), Bounds(0, 0)), ), Scenario( bat_inv_map[batteries_in_pool[1]], @@ -943,7 +943,7 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals "active_power_inclusion_lower_bound": -400, "active_power_inclusion_upper_bound": 400, }, - PowerMetrics(now, Bounds(-500, 0), Bounds(0, 500)), + PowerMetrics(now, Bounds(-500, 500), Bounds(0, 0)), ), Scenario( batteries_in_pool[1], @@ -951,7 +951,7 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals "power_inclusion_lower_bound": -300, "power_inclusion_upper_bound": 700, }, - PowerMetrics(now, Bounds(-400, 0), Bounds(0, 500)), + PowerMetrics(now, Bounds(-400, 500), Bounds(0, 0)), ), Scenario( bat_inv_map[batteries_in_pool[0]], @@ -959,7 +959,7 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals "active_power_inclusion_lower_bound": -200, "active_power_inclusion_upper_bound": 50, }, - PowerMetrics(now, Bounds(-400, 0), Bounds(0, 450)), + PowerMetrics(now, Bounds(-400, 450), Bounds(0, 0)), ), ] @@ -972,27 +972,27 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals all_batteries=all_batteries, batteries_in_pool=batteries_in_pool, waiting_time_sec=waiting_time_sec, - all_pool_result=PowerMetrics(now, Bounds(-400, 0), Bounds(0, 450)), - only_first_battery_result=PowerMetrics(now, Bounds(-100, 0), Bounds(0, 50)), + all_pool_result=PowerMetrics(now, Bounds(-400, 450), Bounds(0, 0)), + only_first_battery_result=PowerMetrics(now, Bounds(-100, 50), Bounds(0, 0)), ) # One battery stopped sending data, inverter data should be used. await streamer.stop_streaming(batteries_in_pool[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, PowerMetrics(now, Bounds(-500, 0), Bounds(0, 450)), 0.2) + compare_messages(msg, PowerMetrics(now, Bounds(-500, 450), Bounds(0, 0)), 0.2) # All batteries stopped sending data, use inverters only. await streamer.stop_streaming(batteries_in_pool[0]) await asyncio.sleep(MAX_BATTERY_DATA_AGE_SEC + 0.2) msg = await asyncio.wait_for(receiver.receive(), timeout=waiting_time_sec) - compare_messages(msg, PowerMetrics(now, Bounds(-600, 0), Bounds(0, 450)), 0.2) + compare_messages(msg, PowerMetrics(now, Bounds(-600, 450), Bounds(0, 0)), 0.2) # One inverter stopped sending data, use one remaining inverter await streamer.stop_streaming(bat_inv_map[batteries_in_pool[0]]) await asyncio.sleep(MAX_BATTERY_DATA_AGE_SEC + 0.2) msg = await asyncio.wait_for(receiver.receive(), timeout=waiting_time_sec) - compare_messages(msg, PowerMetrics(now, Bounds(-400, 0), Bounds(0, 400)), 0.2) + compare_messages(msg, PowerMetrics(now, Bounds(-400, 400), Bounds(0, 0)), 0.2) # All components stopped sending data, we can assume that power bounds are 0 await streamer.stop_streaming(bat_inv_map[batteries_in_pool[1]]) @@ -1004,7 +1004,7 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals latest_data = streamer.get_current_component_data(batteries_in_pool[0]) 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, Bounds(-100, 0), Bounds(0, 100)), 0.2) + compare_messages(msg, PowerMetrics(now, Bounds(-100, 100), Bounds(0, 0)), 0.2) async def run_temperature_test( # pylint: disable=too-many-locals From bbf2a0c097674de7cbcdcc4baac0d8ff3217e170 Mon Sep 17 00:00:00 2001 From: Sahas Subramanian Date: Fri, 21 Jul 2023 18:21:40 +0200 Subject: [PATCH 5/9] Use `Power` objects to represent power bounds in battery pool They were represented with `float`s before. Closes #524 Signed-off-by: Sahas Subramanian --- .../battery_pool/_metric_calculator.py | 34 +++-- .../timeseries/battery_pool/_result_types.py | 6 +- .../_battery_pool/test_battery_pool.py | 130 +++++++++++++++--- 3 files changed, 134 insertions(+), 36 deletions(-) diff --git a/src/frequenz/sdk/timeseries/battery_pool/_metric_calculator.py b/src/frequenz/sdk/timeseries/battery_pool/_metric_calculator.py index c0b2fed61..7711a5034 100644 --- a/src/frequenz/sdk/timeseries/battery_pool/_metric_calculator.py +++ b/src/frequenz/sdk/timeseries/battery_pool/_metric_calculator.py @@ -12,7 +12,8 @@ from ...microgrid import connection_manager from ...microgrid.component import ComponentCategory, ComponentMetricId, InverterType -from ...timeseries import Energy, Percentage, Sample, Temperature +from ...timeseries import Sample +from .._quantities import Energy, Percentage, Power, Temperature from ._component_metrics import ComponentMetricsData from ._result_types import Bounds, PowerMetrics @@ -535,11 +536,9 @@ def calculate( """ # In the future we will have lower bound, too. - result = PowerMetrics( - timestamp=_MIN_TIMESTAMP, - inclusion_bounds=Bounds(0, 0), - exclusion_bounds=Bounds(0, 0), - ) + timestamp = _MIN_TIMESTAMP + inclusion_bounds_lower = 0.0 + inclusion_bounds_upper = 0.0 for battery_id in working_batteries: inclusion_lower_bounds: list[float] = [] @@ -552,12 +551,12 @@ def calculate( # If one is missing, then we can still use the other. value = data.get(ComponentMetricId.POWER_INCLUSION_UPPER_BOUND) if value is not None: - result.timestamp = max(result.timestamp, data.timestamp) + timestamp = max(timestamp, data.timestamp) inclusion_upper_bounds.append(value) value = data.get(ComponentMetricId.POWER_INCLUSION_LOWER_BOUND) if value is not None: - result.timestamp = max(result.timestamp, data.timestamp) + timestamp = max(timestamp, data.timestamp) inclusion_lower_bounds.append(value) inverter_id = self._bat_inv_map[battery_id] @@ -566,20 +565,27 @@ def calculate( value = data.get(ComponentMetricId.ACTIVE_POWER_INCLUSION_UPPER_BOUND) if value is not None: - result.timestamp = max(data.timestamp, result.timestamp) + timestamp = max(data.timestamp, timestamp) inclusion_upper_bounds.append(value) value = data.get(ComponentMetricId.ACTIVE_POWER_INCLUSION_LOWER_BOUND) if value is not None: - result.timestamp = max(data.timestamp, result.timestamp) + timestamp = max(data.timestamp, timestamp) inclusion_lower_bounds.append(value) if len(inclusion_upper_bounds) > 0: - result.inclusion_bounds.upper += min(inclusion_upper_bounds) + inclusion_bounds_upper += min(inclusion_upper_bounds) if len(inclusion_lower_bounds) > 0: - result.inclusion_bounds.lower += max(inclusion_lower_bounds) + inclusion_bounds_lower += max(inclusion_lower_bounds) - if result.timestamp == _MIN_TIMESTAMP: + if timestamp == _MIN_TIMESTAMP: return None - return result + return PowerMetrics( + timestamp=timestamp, + inclusion_bounds=Bounds( + Power.from_watts(inclusion_bounds_lower), + Power.from_watts(inclusion_bounds_upper), + ), + exclusion_bounds=Bounds(Power.from_watts(0.0), Power.from_watts(0.0)), + ) diff --git a/src/frequenz/sdk/timeseries/battery_pool/_result_types.py b/src/frequenz/sdk/timeseries/battery_pool/_result_types.py index a5d2500c4..9215ad442 100644 --- a/src/frequenz/sdk/timeseries/battery_pool/_result_types.py +++ b/src/frequenz/sdk/timeseries/battery_pool/_result_types.py @@ -6,15 +6,17 @@ from dataclasses import dataclass, field from datetime import datetime +from .._quantities import Power + @dataclass class Bounds: """Lower and upper bound values.""" - lower: float + lower: Power """Lower bound.""" - upper: float + upper: Power """Upper bound.""" diff --git a/tests/timeseries/_battery_pool/test_battery_pool.py b/tests/timeseries/_battery_pool/test_battery_pool.py index fdc450c4e..0a1a58690 100644 --- a/tests/timeseries/_battery_pool/test_battery_pool.py +++ b/tests/timeseries/_battery_pool/test_battery_pool.py @@ -3,6 +3,8 @@ """Tests for battery pool.""" +# pylint: disable=too-many-lines + from __future__ import annotations import asyncio @@ -853,8 +855,8 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals now = datetime.now(tz=timezone.utc) expected = PowerMetrics( timestamp=now, - inclusion_bounds=Bounds(-1800, 10000), - exclusion_bounds=Bounds(0, 0), + inclusion_bounds=Bounds(Power.from_watts(-1800), Power.from_watts(10000)), + exclusion_bounds=Bounds(Power.from_watts(0), Power.from_watts(0)), ) compare_messages(msg, expected, WAIT_FOR_COMPONENT_DATA_SEC + 0.2) @@ -863,7 +865,11 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals Scenario( bat_inv_map[batteries_in_pool[0]], {"active_power_inclusion_lower_bound": -100}, - PowerMetrics(now, Bounds(-1000, 10000), Bounds(0, 0)), + PowerMetrics( + now, + Bounds(Power.from_watts(-1000), Power.from_watts(10000)), + Bounds(Power.from_watts(0), Power.from_watts(0)), + ), ), # Inverter bound changed, but metric result should not change. Scenario( @@ -875,12 +881,20 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals Scenario( batteries_in_pool[0], {"power_inclusion_lower_bound": 0, "power_inclusion_upper_bound": 4000}, - PowerMetrics(now, Bounds(-900, 9000), Bounds(0, 0)), + PowerMetrics( + now, + Bounds(Power.from_watts(-900), Power.from_watts(9000)), + Bounds(Power.from_watts(0), Power.from_watts(0)), + ), ), Scenario( batteries_in_pool[1], {"power_inclusion_lower_bound": -10, "power_inclusion_upper_bound": 200}, - PowerMetrics(now, Bounds(-10, 4200), Bounds(0, 0)), + PowerMetrics( + now, + Bounds(Power.from_watts(-10), Power.from_watts(4200)), + Bounds(Power.from_watts(0), Power.from_watts(0)), + ), ), # Test 2 things: # 1. Battery is sending upper bounds=NaN, use only inverter upper bounds @@ -892,7 +906,11 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals "power_inclusion_lower_bound": -50, "power_inclusion_upper_bound": math.nan, }, - PowerMetrics(now, Bounds(-60, 9200), Bounds(0, 0)), + PowerMetrics( + now, + Bounds(Power.from_watts(-60), Power.from_watts(9200)), + Bounds(Power.from_watts(0), Power.from_watts(0)), + ), ), Scenario( bat_inv_map[batteries_in_pool[0]], @@ -900,12 +918,20 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals "active_power_inclusion_lower_bound": math.nan, "active_power_inclusion_upper_bound": math.nan, }, - PowerMetrics(now, Bounds(-60, 200), Bounds(0, 0)), + PowerMetrics( + now, + Bounds(Power.from_watts(-60), Power.from_watts(200)), + Bounds(Power.from_watts(0), Power.from_watts(0)), + ), ), Scenario( batteries_in_pool[0], {"power_inclusion_lower_bound": math.nan}, - PowerMetrics(now, Bounds(-10, 200), Bounds(0, 0)), + PowerMetrics( + now, + Bounds(Power.from_watts(-10), Power.from_watts(200)), + Bounds(Power.from_watts(0), Power.from_watts(0)), + ), ), Scenario( batteries_in_pool[1], @@ -913,7 +939,11 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals "power_inclusion_lower_bound": -100, "power_inclusion_upper_bound": math.nan, }, - PowerMetrics(now, Bounds(-100, 6000), Bounds(0, 0)), + PowerMetrics( + now, + Bounds(Power.from_watts(-100), Power.from_watts(6000)), + Bounds(Power.from_watts(0), Power.from_watts(0)), + ), ), Scenario( bat_inv_map[batteries_in_pool[1]], @@ -921,7 +951,11 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals "active_power_inclusion_lower_bound": math.nan, "active_power_inclusion_upper_bound": math.nan, }, - PowerMetrics(now, Bounds(-100, 0), Bounds(0, 0)), + PowerMetrics( + now, + Bounds(Power.from_watts(-100), Power.from_watts(0)), + Bounds(Power.from_watts(0), Power.from_watts(0)), + ), ), # All components are sending NaN, can't calculate bounds Scenario( @@ -935,7 +969,11 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals Scenario( batteries_in_pool[0], {"power_inclusion_lower_bound": -100, "power_inclusion_upper_bound": 100}, - PowerMetrics(now, Bounds(-100, 100), Bounds(0, 0)), + PowerMetrics( + now, + Bounds(Power.from_watts(-100), Power.from_watts(100)), + Bounds(Power.from_watts(0), Power.from_watts(0)), + ), ), Scenario( bat_inv_map[batteries_in_pool[1]], @@ -943,7 +981,11 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals "active_power_inclusion_lower_bound": -400, "active_power_inclusion_upper_bound": 400, }, - PowerMetrics(now, Bounds(-500, 500), Bounds(0, 0)), + PowerMetrics( + now, + Bounds(Power.from_watts(-500), Power.from_watts(500)), + Bounds(Power.from_watts(0), Power.from_watts(0)), + ), ), Scenario( batteries_in_pool[1], @@ -951,7 +993,11 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals "power_inclusion_lower_bound": -300, "power_inclusion_upper_bound": 700, }, - PowerMetrics(now, Bounds(-400, 500), Bounds(0, 0)), + PowerMetrics( + now, + Bounds(Power.from_watts(-400), Power.from_watts(500)), + Bounds(Power.from_watts(0), Power.from_watts(0)), + ), ), Scenario( bat_inv_map[batteries_in_pool[0]], @@ -959,7 +1005,11 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals "active_power_inclusion_lower_bound": -200, "active_power_inclusion_upper_bound": 50, }, - PowerMetrics(now, Bounds(-400, 450), Bounds(0, 0)), + PowerMetrics( + now, + Bounds(Power.from_watts(-400), Power.from_watts(450)), + Bounds(Power.from_watts(0), Power.from_watts(0)), + ), ), ] @@ -972,27 +1022,59 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals all_batteries=all_batteries, batteries_in_pool=batteries_in_pool, waiting_time_sec=waiting_time_sec, - all_pool_result=PowerMetrics(now, Bounds(-400, 450), Bounds(0, 0)), - only_first_battery_result=PowerMetrics(now, Bounds(-100, 50), Bounds(0, 0)), + all_pool_result=PowerMetrics( + now, + Bounds(Power.from_watts(-400), Power.from_watts(450)), + Bounds(Power.from_watts(0), Power.from_watts(0)), + ), + only_first_battery_result=PowerMetrics( + now, + Bounds(Power.from_watts(-100), Power.from_watts(50)), + Bounds(Power.from_watts(0), Power.from_watts(0)), + ), ) # One battery stopped sending data, inverter data should be used. await streamer.stop_streaming(batteries_in_pool[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, PowerMetrics(now, Bounds(-500, 450), Bounds(0, 0)), 0.2) + compare_messages( + msg, + PowerMetrics( + now, + Bounds(Power.from_watts(-500), Power.from_watts(450)), + Bounds(Power.from_watts(0), Power.from_watts(0)), + ), + 0.2, + ) # All batteries stopped sending data, use inverters only. await streamer.stop_streaming(batteries_in_pool[0]) await asyncio.sleep(MAX_BATTERY_DATA_AGE_SEC + 0.2) msg = await asyncio.wait_for(receiver.receive(), timeout=waiting_time_sec) - compare_messages(msg, PowerMetrics(now, Bounds(-600, 450), Bounds(0, 0)), 0.2) + compare_messages( + msg, + PowerMetrics( + now, + Bounds(Power.from_watts(-600), Power.from_watts(450)), + Bounds(Power.from_watts(0), Power.from_watts(0)), + ), + 0.2, + ) # One inverter stopped sending data, use one remaining inverter await streamer.stop_streaming(bat_inv_map[batteries_in_pool[0]]) await asyncio.sleep(MAX_BATTERY_DATA_AGE_SEC + 0.2) msg = await asyncio.wait_for(receiver.receive(), timeout=waiting_time_sec) - compare_messages(msg, PowerMetrics(now, Bounds(-400, 400), Bounds(0, 0)), 0.2) + compare_messages( + msg, + PowerMetrics( + now, + Bounds(Power.from_watts(-400), Power.from_watts(400)), + Bounds(Power.from_watts(0), Power.from_watts(0)), + ), + 0.2, + ) # All components stopped sending data, we can assume that power bounds are 0 await streamer.stop_streaming(bat_inv_map[batteries_in_pool[1]]) @@ -1004,7 +1086,15 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals latest_data = streamer.get_current_component_data(batteries_in_pool[0]) 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, Bounds(-100, 100), Bounds(0, 0)), 0.2) + compare_messages( + msg, + PowerMetrics( + now, + Bounds(Power.from_watts(-100), Power.from_watts(100)), + Bounds(Power.from_watts(0), Power.from_watts(0)), + ), + 0.2, + ) async def run_temperature_test( # pylint: disable=too-many-locals From dfa6edfb7db10aa4229de8d2b4b81dbd7476a5ac Mon Sep 17 00:00:00 2001 From: Sahas Subramanian Date: Mon, 24 Jul 2023 19:10:17 +0200 Subject: [PATCH 6/9] Refactor inclusion_bounds extraction code to a separate method This is because once we add exclusion bounds, the function will become too big. And instead, we can add it as a separate method in a subsequent commit. Signed-off-by: Sahas Subramanian --- .../battery_pool/_metric_calculator.py | 76 +++++++++++-------- 1 file changed, 45 insertions(+), 31 deletions(-) diff --git a/src/frequenz/sdk/timeseries/battery_pool/_metric_calculator.py b/src/frequenz/sdk/timeseries/battery_pool/_metric_calculator.py index 7711a5034..d1f54b6fe 100644 --- a/src/frequenz/sdk/timeseries/battery_pool/_metric_calculator.py +++ b/src/frequenz/sdk/timeseries/battery_pool/_metric_calculator.py @@ -515,6 +515,45 @@ def inverter_metrics(self) -> Mapping[int, list[ComponentMetricId]]: """ return {cid: self._inverter_metrics for cid in set(self._bat_inv_map.values())} + def _fetch_inclusion_bounds( + self, + battery_id: int, + inverter_id: int, + metrics_data: dict[int, ComponentMetricsData], + ) -> tuple[datetime, list[float], list[float]]: + timestamp = _MIN_TIMESTAMP + inclusion_lower_bounds: list[float] = [] + inclusion_upper_bounds: list[float] = [] + + # Inclusion upper and lower bounds are not related. + # If one is missing, then we can still use the other. + if battery_id in metrics_data: + data = metrics_data[battery_id] + value = data.get(ComponentMetricId.POWER_INCLUSION_UPPER_BOUND) + if value is not None: + timestamp = max(timestamp, data.timestamp) + inclusion_upper_bounds.append(value) + + value = data.get(ComponentMetricId.POWER_INCLUSION_LOWER_BOUND) + if value is not None: + timestamp = max(timestamp, data.timestamp) + inclusion_lower_bounds.append(value) + + if inverter_id in metrics_data: + data = metrics_data[inverter_id] + + value = data.get(ComponentMetricId.ACTIVE_POWER_INCLUSION_UPPER_BOUND) + if value is not None: + timestamp = max(data.timestamp, timestamp) + inclusion_upper_bounds.append(value) + + value = data.get(ComponentMetricId.ACTIVE_POWER_INCLUSION_LOWER_BOUND) + if value is not None: + timestamp = max(data.timestamp, timestamp) + inclusion_lower_bounds.append(value) + + return (timestamp, inclusion_lower_bounds, inclusion_upper_bounds) + def calculate( self, metrics_data: dict[int, ComponentMetricsData], @@ -541,38 +580,13 @@ def calculate( inclusion_bounds_upper = 0.0 for battery_id in working_batteries: - inclusion_lower_bounds: list[float] = [] - inclusion_upper_bounds: list[float] = [] - - if battery_id in metrics_data: - data = metrics_data[battery_id] - - # Consume and supply bounds are not related. - # If one is missing, then we can still use the other. - value = data.get(ComponentMetricId.POWER_INCLUSION_UPPER_BOUND) - if value is not None: - timestamp = max(timestamp, data.timestamp) - inclusion_upper_bounds.append(value) - - value = data.get(ComponentMetricId.POWER_INCLUSION_LOWER_BOUND) - if value is not None: - timestamp = max(timestamp, data.timestamp) - inclusion_lower_bounds.append(value) - inverter_id = self._bat_inv_map[battery_id] - if inverter_id in metrics_data: - data = metrics_data[inverter_id] - - value = data.get(ComponentMetricId.ACTIVE_POWER_INCLUSION_UPPER_BOUND) - if value is not None: - timestamp = max(data.timestamp, timestamp) - inclusion_upper_bounds.append(value) - - value = data.get(ComponentMetricId.ACTIVE_POWER_INCLUSION_LOWER_BOUND) - if value is not None: - timestamp = max(data.timestamp, timestamp) - inclusion_lower_bounds.append(value) - + ( + _ts, + inclusion_lower_bounds, + inclusion_upper_bounds, + ) = self._fetch_inclusion_bounds(battery_id, inverter_id, metrics_data) + timestamp = max(timestamp, _ts) if len(inclusion_upper_bounds) > 0: inclusion_bounds_upper += min(inclusion_upper_bounds) if len(inclusion_lower_bounds) > 0: From a3797b640983fa4753df1c6c2ff71da13b33e43d Mon Sep 17 00:00:00 2001 From: Sahas Subramanian Date: Mon, 24 Jul 2023 19:38:33 +0200 Subject: [PATCH 7/9] Implement fetching and streaming of exclusion bounds in battery pool Signed-off-by: Sahas Subramanian --- .../battery_pool/_metric_calculator.py | 61 +++++++++++- .../_battery_pool/test_battery_pool.py | 93 +++++++++++++------ 2 files changed, 125 insertions(+), 29 deletions(-) diff --git a/src/frequenz/sdk/timeseries/battery_pool/_metric_calculator.py b/src/frequenz/sdk/timeseries/battery_pool/_metric_calculator.py index d1f54b6fe..ceb730e0d 100644 --- a/src/frequenz/sdk/timeseries/battery_pool/_metric_calculator.py +++ b/src/frequenz/sdk/timeseries/battery_pool/_metric_calculator.py @@ -480,11 +480,15 @@ def __init__( super().__init__(used_batteries) self._battery_metrics = [ ComponentMetricId.POWER_INCLUSION_LOWER_BOUND, + ComponentMetricId.POWER_EXCLUSION_LOWER_BOUND, + ComponentMetricId.POWER_EXCLUSION_UPPER_BOUND, ComponentMetricId.POWER_INCLUSION_UPPER_BOUND, ] self._inverter_metrics = [ ComponentMetricId.ACTIVE_POWER_INCLUSION_LOWER_BOUND, + ComponentMetricId.ACTIVE_POWER_EXCLUSION_LOWER_BOUND, + ComponentMetricId.ACTIVE_POWER_EXCLUSION_UPPER_BOUND, ComponentMetricId.ACTIVE_POWER_INCLUSION_UPPER_BOUND, ] @@ -554,6 +558,45 @@ def _fetch_inclusion_bounds( return (timestamp, inclusion_lower_bounds, inclusion_upper_bounds) + def _fetch_exclusion_bounds( + self, + battery_id: int, + inverter_id: int, + metrics_data: dict[int, ComponentMetricsData], + ) -> tuple[datetime, list[float], list[float]]: + timestamp = _MIN_TIMESTAMP + exclusion_lower_bounds: list[float] = [] + exclusion_upper_bounds: list[float] = [] + + # Exclusion upper and lower bounds are not related. + # If one is missing, then we can still use the other. + if battery_id in metrics_data: + data = metrics_data[battery_id] + value = data.get(ComponentMetricId.POWER_EXCLUSION_UPPER_BOUND) + if value is not None: + timestamp = max(timestamp, data.timestamp) + exclusion_upper_bounds.append(value) + + value = data.get(ComponentMetricId.POWER_EXCLUSION_LOWER_BOUND) + if value is not None: + timestamp = max(timestamp, data.timestamp) + exclusion_lower_bounds.append(value) + + if inverter_id in metrics_data: + data = metrics_data[inverter_id] + + value = data.get(ComponentMetricId.ACTIVE_POWER_EXCLUSION_UPPER_BOUND) + if value is not None: + timestamp = max(data.timestamp, timestamp) + exclusion_upper_bounds.append(value) + + value = data.get(ComponentMetricId.ACTIVE_POWER_EXCLUSION_LOWER_BOUND) + if value is not None: + timestamp = max(data.timestamp, timestamp) + exclusion_lower_bounds.append(value) + + return (timestamp, exclusion_lower_bounds, exclusion_upper_bounds) + def calculate( self, metrics_data: dict[int, ComponentMetricsData], @@ -573,11 +616,11 @@ def calculate( High level metric calculated from the given metrics. Return None if there are no component metrics. """ - # In the future we will have lower bound, too. - timestamp = _MIN_TIMESTAMP inclusion_bounds_lower = 0.0 inclusion_bounds_upper = 0.0 + exclusion_bounds_lower = 0.0 + exclusion_bounds_upper = 0.0 for battery_id in working_batteries: inverter_id = self._bat_inv_map[battery_id] @@ -587,10 +630,19 @@ def calculate( inclusion_upper_bounds, ) = self._fetch_inclusion_bounds(battery_id, inverter_id, metrics_data) timestamp = max(timestamp, _ts) + ( + _ts, + exclusion_lower_bounds, + exclusion_upper_bounds, + ) = self._fetch_exclusion_bounds(battery_id, inverter_id, metrics_data) if len(inclusion_upper_bounds) > 0: inclusion_bounds_upper += min(inclusion_upper_bounds) if len(inclusion_lower_bounds) > 0: inclusion_bounds_lower += max(inclusion_lower_bounds) + if len(exclusion_upper_bounds) > 0: + exclusion_bounds_upper += max(exclusion_upper_bounds) + if len(exclusion_lower_bounds) > 0: + exclusion_bounds_lower += min(exclusion_lower_bounds) if timestamp == _MIN_TIMESTAMP: return None @@ -601,5 +653,8 @@ def calculate( Power.from_watts(inclusion_bounds_lower), Power.from_watts(inclusion_bounds_upper), ), - exclusion_bounds=Bounds(Power.from_watts(0.0), Power.from_watts(0.0)), + exclusion_bounds=Bounds( + Power.from_watts(exclusion_bounds_lower), + Power.from_watts(exclusion_bounds_upper), + ), ) diff --git a/tests/timeseries/_battery_pool/test_battery_pool.py b/tests/timeseries/_battery_pool/test_battery_pool.py index 0a1a58690..c7a7f6527 100644 --- a/tests/timeseries/_battery_pool/test_battery_pool.py +++ b/tests/timeseries/_battery_pool/test_battery_pool.py @@ -833,6 +833,8 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals timestamp=datetime.now(tz=timezone.utc), power_inclusion_lower_bound=-1000, power_inclusion_upper_bound=5000, + power_exclusion_lower_bound=-300, + power_exclusion_upper_bound=300, ), sampling_rate=0.05, ) @@ -842,6 +844,8 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals timestamp=datetime.now(tz=timezone.utc), active_power_inclusion_lower_bound=-900, active_power_inclusion_upper_bound=6000, + active_power_exclusion_lower_bound=-200, + active_power_exclusion_upper_bound=200, ), sampling_rate=0.1, ) @@ -856,7 +860,7 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals expected = PowerMetrics( timestamp=now, inclusion_bounds=Bounds(Power.from_watts(-1800), Power.from_watts(10000)), - exclusion_bounds=Bounds(Power.from_watts(0), Power.from_watts(0)), + exclusion_bounds=Bounds(Power.from_watts(-600), Power.from_watts(600)), ) compare_messages(msg, expected, WAIT_FOR_COMPONENT_DATA_SEC + 0.2) @@ -864,36 +868,52 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals scenarios: list[Scenario[PowerMetrics]] = [ Scenario( bat_inv_map[batteries_in_pool[0]], - {"active_power_inclusion_lower_bound": -100}, + { + "active_power_inclusion_lower_bound": -100, + "active_power_exclusion_lower_bound": -400, + }, PowerMetrics( now, Bounds(Power.from_watts(-1000), Power.from_watts(10000)), - Bounds(Power.from_watts(0), Power.from_watts(0)), + Bounds(Power.from_watts(-700), Power.from_watts(600)), ), ), # Inverter bound changed, but metric result should not change. Scenario( component_id=bat_inv_map[batteries_in_pool[0]], - new_metrics={"active_power_inclusion_upper_bound": 9000}, + new_metrics={ + "active_power_inclusion_upper_bound": 9000, + "active_power_exclusion_upper_bound": 250, + }, expected_result=None, wait_for_result=False, ), Scenario( batteries_in_pool[0], - {"power_inclusion_lower_bound": 0, "power_inclusion_upper_bound": 4000}, + { + "power_inclusion_lower_bound": 0, + "power_inclusion_upper_bound": 4000, + "power_exclusion_lower_bound": 0, + "power_exclusion_upper_bound": 100, + }, PowerMetrics( now, Bounds(Power.from_watts(-900), Power.from_watts(9000)), - Bounds(Power.from_watts(0), Power.from_watts(0)), + Bounds(Power.from_watts(-700), Power.from_watts(550)), ), ), Scenario( batteries_in_pool[1], - {"power_inclusion_lower_bound": -10, "power_inclusion_upper_bound": 200}, + { + "power_inclusion_lower_bound": -10, + "power_inclusion_upper_bound": 200, + "power_exclusion_lower_bound": -5, + "power_exclusion_upper_bound": 5, + }, PowerMetrics( now, Bounds(Power.from_watts(-10), Power.from_watts(4200)), - Bounds(Power.from_watts(0), Power.from_watts(0)), + Bounds(Power.from_watts(-600), Power.from_watts(450)), ), ), # Test 2 things: @@ -905,11 +925,13 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals { "power_inclusion_lower_bound": -50, "power_inclusion_upper_bound": math.nan, + "power_exclusion_lower_bound": -30, + "power_exclusion_upper_bound": 300, }, PowerMetrics( now, Bounds(Power.from_watts(-60), Power.from_watts(9200)), - Bounds(Power.from_watts(0), Power.from_watts(0)), + Bounds(Power.from_watts(-600), Power.from_watts(500)), ), ), Scenario( @@ -917,20 +939,25 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals { "active_power_inclusion_lower_bound": math.nan, "active_power_inclusion_upper_bound": math.nan, + "active_power_exclusion_lower_bound": math.nan, + "active_power_exclusion_upper_bound": math.nan, }, PowerMetrics( now, Bounds(Power.from_watts(-60), Power.from_watts(200)), - Bounds(Power.from_watts(0), Power.from_watts(0)), + Bounds(Power.from_watts(-230), Power.from_watts(500)), ), ), Scenario( batteries_in_pool[0], - {"power_inclusion_lower_bound": math.nan}, + { + "power_inclusion_lower_bound": math.nan, + "power_exclusion_lower_bound": math.nan, + }, PowerMetrics( now, Bounds(Power.from_watts(-10), Power.from_watts(200)), - Bounds(Power.from_watts(0), Power.from_watts(0)), + Bounds(Power.from_watts(-200), Power.from_watts(500)), ), ), Scenario( @@ -938,11 +965,13 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals { "power_inclusion_lower_bound": -100, "power_inclusion_upper_bound": math.nan, + "power_exclusion_lower_bound": -50, + "power_exclusion_upper_bound": 50, }, PowerMetrics( now, Bounds(Power.from_watts(-100), Power.from_watts(6000)), - Bounds(Power.from_watts(0), Power.from_watts(0)), + Bounds(Power.from_watts(-200), Power.from_watts(500)), ), ), Scenario( @@ -950,11 +979,13 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals { "active_power_inclusion_lower_bound": math.nan, "active_power_inclusion_upper_bound": math.nan, + "active_power_exclusion_lower_bound": math.nan, + "active_power_exclusion_upper_bound": math.nan, }, PowerMetrics( now, Bounds(Power.from_watts(-100), Power.from_watts(0)), - Bounds(Power.from_watts(0), Power.from_watts(0)), + Bounds(Power.from_watts(-50), Power.from_watts(350)), ), ), # All components are sending NaN, can't calculate bounds @@ -968,11 +999,16 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals ), Scenario( batteries_in_pool[0], - {"power_inclusion_lower_bound": -100, "power_inclusion_upper_bound": 100}, + { + "power_inclusion_lower_bound": -100, + "power_inclusion_upper_bound": 100, + "power_exclusion_lower_bound": -20, + "power_exclusion_upper_bound": 20, + }, PowerMetrics( now, Bounds(Power.from_watts(-100), Power.from_watts(100)), - Bounds(Power.from_watts(0), Power.from_watts(0)), + Bounds(Power.from_watts(-70), Power.from_watts(70)), ), ), Scenario( @@ -980,11 +1016,13 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals { "active_power_inclusion_lower_bound": -400, "active_power_inclusion_upper_bound": 400, + "active_power_exclusion_lower_bound": -100, + "active_power_exclusion_upper_bound": 100, }, PowerMetrics( now, Bounds(Power.from_watts(-500), Power.from_watts(500)), - Bounds(Power.from_watts(0), Power.from_watts(0)), + Bounds(Power.from_watts(-120), Power.from_watts(120)), ), ), Scenario( @@ -992,11 +1030,13 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals { "power_inclusion_lower_bound": -300, "power_inclusion_upper_bound": 700, + "power_exclusion_lower_bound": -130, + "power_exclusion_upper_bound": 130, }, PowerMetrics( now, Bounds(Power.from_watts(-400), Power.from_watts(500)), - Bounds(Power.from_watts(0), Power.from_watts(0)), + Bounds(Power.from_watts(-150), Power.from_watts(150)), ), ), Scenario( @@ -1004,15 +1044,16 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals { "active_power_inclusion_lower_bound": -200, "active_power_inclusion_upper_bound": 50, + "active_power_exclusion_lower_bound": -80, + "active_power_exclusion_upper_bound": 80, }, PowerMetrics( now, Bounds(Power.from_watts(-400), Power.from_watts(450)), - Bounds(Power.from_watts(0), Power.from_watts(0)), + Bounds(Power.from_watts(-210), Power.from_watts(210)), ), ), ] - waiting_time_sec = setup_args.min_update_interval + 0.02 await run_scenarios(scenarios, streamer, receiver, waiting_time_sec) @@ -1025,12 +1066,12 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals all_pool_result=PowerMetrics( now, Bounds(Power.from_watts(-400), Power.from_watts(450)), - Bounds(Power.from_watts(0), Power.from_watts(0)), + Bounds(Power.from_watts(-210), Power.from_watts(210)), ), only_first_battery_result=PowerMetrics( now, Bounds(Power.from_watts(-100), Power.from_watts(50)), - Bounds(Power.from_watts(0), Power.from_watts(0)), + Bounds(Power.from_watts(-80), Power.from_watts(80)), ), ) @@ -1043,7 +1084,7 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals PowerMetrics( now, Bounds(Power.from_watts(-500), Power.from_watts(450)), - Bounds(Power.from_watts(0), Power.from_watts(0)), + Bounds(Power.from_watts(-180), Power.from_watts(180)), ), 0.2, ) @@ -1057,7 +1098,7 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals PowerMetrics( now, Bounds(Power.from_watts(-600), Power.from_watts(450)), - Bounds(Power.from_watts(0), Power.from_watts(0)), + Bounds(Power.from_watts(-180), Power.from_watts(180)), ), 0.2, ) @@ -1071,7 +1112,7 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals PowerMetrics( now, Bounds(Power.from_watts(-400), Power.from_watts(400)), - Bounds(Power.from_watts(0), Power.from_watts(0)), + Bounds(Power.from_watts(-100), Power.from_watts(100)), ), 0.2, ) @@ -1091,7 +1132,7 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals PowerMetrics( now, Bounds(Power.from_watts(-100), Power.from_watts(100)), - Bounds(Power.from_watts(0), Power.from_watts(0)), + Bounds(Power.from_watts(-20), Power.from_watts(20)), ), 0.2, ) From 5bb5bb38a29d3cac5eed21f23d0bcfa13db5b6fc Mon Sep 17 00:00:00 2001 From: Sahas Subramanian Date: Thu, 3 Aug 2023 14:11:34 +0200 Subject: [PATCH 8/9] Update RELEASE_NOTES.md Signed-off-by: Sahas Subramanian --- RELEASE_NOTES.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 4c24dc22d..85a36f477 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -8,6 +8,8 @@ - Upgrade to microgrid API v0.15.1. If you're using any of the lower level microgrid interfaces, you will need to upgrade your code. +- The `BatteryPool.power_bounds` method now streams inclusion/exclusion bounds. The bounds are now represented by `Power` objects and not `float`s. + ## New Features From 639394c8fa876a74d1e6247c6bb0efa3732d2e03 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Tue, 8 Aug 2023 11:28:07 +0200 Subject: [PATCH 9/9] Replace `Power.from_watts(0)` with `Power.zero()` Signed-off-by: Leandro Lucarella --- tests/timeseries/_battery_pool/test_battery_pool.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/timeseries/_battery_pool/test_battery_pool.py b/tests/timeseries/_battery_pool/test_battery_pool.py index c7a7f6527..72f41e3bf 100644 --- a/tests/timeseries/_battery_pool/test_battery_pool.py +++ b/tests/timeseries/_battery_pool/test_battery_pool.py @@ -984,7 +984,7 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals }, PowerMetrics( now, - Bounds(Power.from_watts(-100), Power.from_watts(0)), + Bounds(Power.from_watts(-100), Power.zero()), Bounds(Power.from_watts(-50), Power.from_watts(350)), ), ),