More updates for n:m support - battery pool's power formula (#730)

Marenz · web-flow · commit b0cf5e2d39ef · 2024-02-22T12:59:38.000Z
This specifically focuses on the power forumla for the battery pool. * [x] test with no battery * [x] test with no inverter * [x] test with requests that don't specify all batteries behind the inverters refs #501
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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.
