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/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/timeseries/_battery_pool/test_battery_pool.py b/tests/timeseries/_battery_pool/test_battery_pool.py index 0787f0659..d65a3c7ab 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 @@ -488,10 +492,110 @@ 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) + await _test_battery_pool_power(mockgrid) + + +async def test_battery_pool_power_two_inverters_per_battery( + mocker: MockerFixture, +) -> None: + """Test power method 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 2737ca846..87e1b8041 100644 --- a/tests/utils/graph_generator.py +++ b/tests/utils/graph_generator.py @@ -59,12 +59,68 @@ def new_id(self) -> dict[ComponentCategory, int]: self._id_increment += 1 return id_per_category + def battery_with_inverter(self, battery: Component, num_inverters: int) -> Any: + """Add a meter and inverters to the given battery. + + Args: + 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( + 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. + """ + 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( + 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: Component) -> Component: + 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.