From 9cdbdae1fedf81b79f03f746a61df712096d0c64 Mon Sep 17 00:00:00 2001 From: "Mathias L. Baumann" Date: Wed, 18 Oct 2023 14:51:36 +0200 Subject: [PATCH 1/3] Tests: Add convenience methods to create batteries with multiple inverters Signed-off-by: Mathias L. Baumann --- .../test_power_distributing.py | 39 +-------------- tests/utils/graph_generator.py | 49 +++++++++++++++++++ 2 files changed, 51 insertions(+), 37 deletions(-) diff --git a/tests/actor/power_distributing/test_power_distributing.py b/tests/actor/power_distributing/test_power_distributing.py index 32db583f1..56bf0636f 100644 --- a/tests/actor/power_distributing/test_power_distributing.py +++ b/tests/actor/power_distributing/test_power_distributing.py @@ -537,21 +537,7 @@ async def test_battery_two_inverters(self, mocker: MockerFixture) -> None: graph = gen.to_graph( ( ComponentCategory.METER, - [ - ( - ComponentCategory.METER, - [ - ( - ComponentCategory.INVERTER, - bat_component, - ), - ( - ComponentCategory.INVERTER, - bat_component, - ), - ], - ), - ], + [gen.battery_with_inverter(bat_component, 2)], ) ) @@ -600,28 +586,7 @@ async def test_two_batteries_three_inverters(self, mocker: MockerFixture) -> Non batteries = gen.components(*[ComponentCategory.BATTERY] * 2) graph = gen.to_graph( - ( - ComponentCategory.METER, - [ - ( - ComponentCategory.METER, - [ - ( - ComponentCategory.INVERTER, - [*batteries], - ), - ( - ComponentCategory.INVERTER, - [*batteries], - ), - ( - ComponentCategory.INVERTER, - [*batteries], - ), - ], - ), - ], - ) + (ComponentCategory.METER, gen.batteries_with_inverter(batteries, 3)) ) async with _mocks(mocker, graph=graph) as mocks: diff --git a/tests/utils/graph_generator.py b/tests/utils/graph_generator.py index 2737ca846..c6ffafe54 100644 --- a/tests/utils/graph_generator.py +++ b/tests/utils/graph_generator.py @@ -70,6 +70,55 @@ def component(self, other: Component) -> Component: the given component. """ + def battery_with_inverter(self, battery: Component, num_inverters: int) -> Any: + """Add a meter and inverters to the given battery. + + Args: + one_or_more_batteries: the battery component. + num_inverters: the number of inverters to create. + + Returns: + connected graph components for the given battery. + """ + return self._battery_with_inverter(battery, num_inverters) + + def batteries_with_inverter( + self, one_or_more_batteries: list[Component], num_inverters: int + ) -> Any: + """Add a meter and inverters to the given batteries. + + Args: + one_or_more_batteries: the battery components. + num_inverters: the number of inverters to create. + + Returns: + connected graph components for the given batteries. + """ + return self._battery_with_inverter(one_or_more_batteries, num_inverters) + + def _battery_with_inverter( + self, one_or_more_batteries: Component | list[Component], num_inverters: int + ) -> Any: + """Add a meter and inverters to the given battery or batteries. + + Args: + one_or_more_batteries: the battery component or components. + num_inverters: the number of inverters to create. + + Returns: + connected graph components for the given battery or batteries. + """ + return ( + ComponentCategory.METER, + [ + ( + self.component(ComponentCategory.INVERTER, InverterType.BATTERY), + one_or_more_batteries, + ) + for _ in range(num_inverters) + ], + ) + @overload def component( self, other: ComponentCategory, comp_type: ComponentType | None = None From ec3ce8a610e912b7ba01ac0728f72a362d0f62c7 Mon Sep 17 00:00:00 2001 From: "Mathias L. Baumann" Date: Wed, 18 Oct 2023 17:15:41 +0200 Subject: [PATCH 2/3] Update power formula to work properly with multiple inverters/batteries Signed-off-by: Mathias L. Baumann --- RELEASE_NOTES.md | 2 + .../_battery_power_formula.py | 36 +++++- .../_battery_pool/test_battery_pool.py | 104 ++++++++++++++++++ tests/utils/graph_generator.py | 31 ++++-- 4 files changed, 156 insertions(+), 17 deletions(-) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 9942fc8dd..51e48b403 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -13,6 +13,8 @@ - Allow multiplying and dividing any `Quantity` by a `float`. This just scales the `Quantity` value. - Allow dividing any `Quantity` by another quaintity of the same type. This just returns a ration between both quantities. +- The battery pool `power` method now supports scenarios where one or more inverters can have multiple batteries connected to it and one or more batteries can have multiple inverters connected to it. + ## Bug Fixes - Fix grid current formula generator to add the operator `+` to the engine only when the component category is handled. diff --git a/src/frequenz/sdk/timeseries/formula_engine/_formula_generators/_battery_power_formula.py b/src/frequenz/sdk/timeseries/formula_engine/_formula_generators/_battery_power_formula.py index 4504287ba..77d23ed01 100644 --- a/src/frequenz/sdk/timeseries/formula_engine/_formula_generators/_battery_power_formula.py +++ b/src/frequenz/sdk/timeseries/formula_engine/_formula_generators/_battery_power_formula.py @@ -12,6 +12,7 @@ from ._formula_generator import ( NON_EXISTING_COMPONENT_ID, ComponentNotFound, + FormulaGenerationError, FormulaGenerator, ) @@ -40,6 +41,8 @@ def generate( they don't have an inverter as a predecessor. FormulaGenerationError: If a battery has a non-inverter predecessor in the component graph. + FormulaGenerationError: If not all batteries behind a set of inverters + have been requested. """ builder = self._get_builder( "battery-power", ComponentMetricId.ACTIVE_POWER, Power.from_watts @@ -61,16 +64,39 @@ def generate( component_graph = connection_manager.get().component_graph - battery_inverters = list( - next(iter(component_graph.predecessors(bat_id))) for bat_id in component_ids + battery_inverters = frozenset( + frozenset( + filter( + component_graph.is_battery_inverter, + component_graph.predecessors(bat_id), + ) + ) + for bat_id in component_ids ) - if len(component_ids) != len(battery_inverters): + if not all(battery_inverters): raise ComponentNotFound( - "Can't find inverters for all batteries from the component graph." + "All batteries must have at least one inverter as a predecessor." + ) + + all_connected_batteries = set() + for inverters in battery_inverters: + for inverter in inverters: + all_connected_batteries.update( + component_graph.successors(inverter.component_id) + ) + + if len(all_connected_batteries) != len(component_ids): + raise FormulaGenerationError( + "All batteries behind a set of inverters must be requested." ) - for idx, comp in enumerate(battery_inverters): + builder.push_oper("(") + builder.push_oper("(") + # Iterate over the flattened list of inverters + for idx, comp in enumerate( + inverter for inverters in battery_inverters for inverter in inverters + ): if idx > 0: builder.push_oper("+") builder.push_component_metric(comp.component_id, nones_are_zeros=True) diff --git a/tests/timeseries/_battery_pool/test_battery_pool.py b/tests/timeseries/_battery_pool/test_battery_pool.py index 0787f0659..44745c06e 100644 --- a/tests/timeseries/_battery_pool/test_battery_pool.py +++ b/tests/timeseries/_battery_pool/test_battery_pool.py @@ -42,6 +42,10 @@ ) from frequenz.sdk.timeseries._base_types import SystemBounds from frequenz.sdk.timeseries.battery_pool import BatteryPool +from frequenz.sdk.timeseries.formula_engine._formula_generators._formula_generator import ( + FormulaGenerationError, +) +from tests.utils.graph_generator import GraphGenerator from ...timeseries.mock_microgrid import MockMicrogrid from ...utils.component_data_streamer import MockComponentDataStreamer @@ -491,7 +495,107 @@ async def test_battery_pool_power(mocker: MockerFixture) -> None: """Test `BatteryPool.{,production,consumption}_power` methods.""" mockgrid = MockMicrogrid(grid_meter=True, mocker=mocker) mockgrid.add_batteries(2) + await mockgrid.start(mocker) + await _test_battery_pool_power(mockgrid) + + +async def test_battery_pool_power_two_inverters_per_battery( + mocker: MockerFixture, +) -> None: + """Test power methods with two inverters per battery.""" + gen = GraphGenerator() + bat = gen.component(ComponentCategory.BATTERY) + mockgrid = MockMicrogrid( + graph=gen.to_graph((ComponentCategory.METER, gen.battery_with_inverter(bat, 2))) + ) + await mockgrid.start(mocker) + await _test_battery_pool_power(mockgrid) + + +async def test_batter_pool_power_two_batteries_per_inverter( + mocker: MockerFixture, +) -> None: + """Test power method with two batteries per inverter.""" + gen = GraphGenerator() + mockgrid = MockMicrogrid( + graph=gen.to_graph( + [ + ComponentCategory.METER, + ( + ComponentCategory.INVERTER, + [ComponentCategory.BATTERY, ComponentCategory.BATTERY], + ), + ComponentCategory.METER, + ( + ComponentCategory.INVERTER, + [ComponentCategory.BATTERY, ComponentCategory.BATTERY], + ), + ] + ) + ) + await mockgrid.start(mocker) + await _test_battery_pool_power(mockgrid) + + +async def test_batter_pool_power_no_batteries(mocker: MockerFixture) -> None: + """Test power method with no batteries.""" + mockgrid = MockMicrogrid( + graph=GraphGenerator().to_graph( + ( + ComponentCategory.METER, + [ComponentCategory.INVERTER, ComponentCategory.INVERTER], + ) + ) + ) + await mockgrid.start(mocker) + battery_pool = microgrid.battery_pool() + power_receiver = battery_pool.power.new_receiver() + + await mockgrid.mock_resampler.send_non_existing_component_value() + assert (await power_receiver.receive()).value == Power.from_watts(0) + + +async def test_battery_pool_power_with_no_inverters(mocker: MockerFixture) -> None: + """Test power method with no inverters.""" + mockgrid = MockMicrogrid( + graph=GraphGenerator().to_graph( + (ComponentCategory.METER, ComponentCategory.BATTERY) + ) + ) + await mockgrid.start(mocker) + + with pytest.raises(RuntimeError): + microgrid.battery_pool() + + +async def test_battery_pool_power_incomplete_bat_request(mocker: MockerFixture) -> None: + """Test power method when not all requested ids are behind the same inverter.""" + gen = GraphGenerator() + bats = gen.components( + ComponentCategory.BATTERY, ComponentCategory.BATTERY, ComponentCategory.BATTERY + ) + + mockgrid = MockMicrogrid( + graph=gen.to_graph( + ( + ComponentCategory.METER, + gen.batteries_with_inverter(bats, 2), + ) + ) + ) + await mockgrid.start(mocker) + + with pytest.raises(FormulaGenerationError): + # Request only two of the three batteries behind the inverters + battery_pool = microgrid.battery_pool( + battery_ids=set([bats[1].component_id, bats[0].component_id]) + ) + power_receiver = battery_pool.power.new_receiver() + await mockgrid.mock_resampler.send_bat_inverter_power([2.0]) + assert (await power_receiver.receive()).value == Power.from_watts(2.0) + +async def _test_battery_pool_power(mockgrid: MockMicrogrid) -> None: async with mockgrid: battery_pool = microgrid.battery_pool() power_receiver = battery_pool.power.new_receiver() diff --git a/tests/utils/graph_generator.py b/tests/utils/graph_generator.py index c6ffafe54..87e1b8041 100644 --- a/tests/utils/graph_generator.py +++ b/tests/utils/graph_generator.py @@ -59,27 +59,17 @@ def new_id(self) -> dict[ComponentCategory, int]: self._id_increment += 1 return id_per_category - @overload - def component(self, other: Component) -> Component: - """Just return the given component. - - Args: - other: the component to return. - - Returns: - the given component. - """ - def battery_with_inverter(self, battery: Component, num_inverters: int) -> Any: """Add a meter and inverters to the given battery. Args: - one_or_more_batteries: the battery component. + battery: the battery component. num_inverters: the number of inverters to create. Returns: connected graph components for the given battery. """ + assert battery.category == ComponentCategory.BATTERY return self._battery_with_inverter(battery, num_inverters) def batteries_with_inverter( @@ -94,6 +84,9 @@ def batteries_with_inverter( Returns: connected graph components for the given batteries. """ + assert all( + b.category == ComponentCategory.BATTERY for b in one_or_more_batteries + ) return self._battery_with_inverter(one_or_more_batteries, num_inverters) def _battery_with_inverter( @@ -119,6 +112,20 @@ def _battery_with_inverter( ], ) + @overload + def component( + self, other: Component, comp_type: ComponentType | None = None + ) -> Component: + """Just return the given component. + + Args: + other: the component to return. + comp_type: the component type to set, ignored + + Returns: + the given component. + """ + @overload def component( self, other: ComponentCategory, comp_type: ComponentType | None = None From c26c51834cd4c7cf8eb066d4b2045f680b77b1c5 Mon Sep 17 00:00:00 2001 From: "Mathias L. Baumann" Date: Thu, 22 Feb 2024 13:17:35 +0100 Subject: [PATCH 3/3] Update test method descriptions Signed-off-by: Mathias L. Baumann --- tests/timeseries/_battery_pool/test_battery_pool.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/timeseries/_battery_pool/test_battery_pool.py b/tests/timeseries/_battery_pool/test_battery_pool.py index 44745c06e..d65a3c7ab 100644 --- a/tests/timeseries/_battery_pool/test_battery_pool.py +++ b/tests/timeseries/_battery_pool/test_battery_pool.py @@ -492,7 +492,7 @@ async def run_test_battery_status_channel( # pylint: disable=too-many-arguments async def test_battery_pool_power(mocker: MockerFixture) -> None: - """Test `BatteryPool.{,production,consumption}_power` methods.""" + """Test `BatteryPool.power` method.""" mockgrid = MockMicrogrid(grid_meter=True, mocker=mocker) mockgrid.add_batteries(2) await mockgrid.start(mocker) @@ -502,7 +502,7 @@ async def test_battery_pool_power(mocker: MockerFixture) -> None: async def test_battery_pool_power_two_inverters_per_battery( mocker: MockerFixture, ) -> None: - """Test power methods with two inverters per battery.""" + """Test power method with two inverters per battery.""" gen = GraphGenerator() bat = gen.component(ComponentCategory.BATTERY) mockgrid = MockMicrogrid(