Improve formula generators (#599)

We use the recently introduced DFS in the component graph to update the
pv power and the producer power formule in the logical meter.

Author: matthias-wende-frequenz

RELEASE_NOTES.md

@@ -22,3 +22,6 @@
 - Fix `consumer_power()` not working certain configurations.
   In microgrids without consumers and no main meter, the formula
   would never return any values.
+- Fix `pv_power` not working in setups with 2 grid meters by using a new
+  reliable function to search for components in the components graph
+- Fix `consumer_power` similar to `pv_power`

src/frequenz/sdk/microgrid/_graph.py

@@ -276,7 +276,7 @@ def dfs(
         """
         Search for components that fulfill the condition in the Graph.
 
-        DFS is used for searching the graph. The graph travarsal is stopped
+        DFS is used for searching the graph. The graph traversal is stopped
         once a component fulfills the condition.
 
         Args:
@@ -285,7 +285,7 @@ def dfs(
             condition: The condition function to check for.
 
         Returns:
-            A set of component ids where the coresponding components fulfill
+            A set of component ids where the corresponding components fulfill
             the condition function.
         """
 

src/frequenz/sdk/timeseries/_formula_engine/_formula_generators/_producer_power_formula.py

@@ -3,14 +3,16 @@
 
 """Formula generator from component graph for Producer Power."""
 
-from __future__ import annotations
+import logging
 
 from ....microgrid import connection_manager
 from ....microgrid.component import ComponentCategory, ComponentMetricId
 from ..._quantities import Power
 from .._formula_engine import FormulaEngine
 from ._formula_generator import NON_EXISTING_COMPONENT_ID, FormulaGenerator
 
+_logger = logging.getLogger(__name__)
+
 
 class ProducerPowerFormula(FormulaGenerator[Power]):
     """Formula generator from component graph for calculating the Producer Power.
@@ -34,36 +36,37 @@ def generate(self) -> FormulaEngine[Power]:
         builder = self._get_builder(
             "producer_power", ComponentMetricId.ACTIVE_POWER, Power.from_watts
         )
-        component_graph = connection_manager.get().component_graph
-        grid_successors = self._get_grid_component_successors()
-
-        if len(grid_successors) == 1:
-            grid_meter = next(iter(grid_successors))
-            if grid_meter.category != ComponentCategory.METER:
-                raise RuntimeError(
-                    "Only grid successor in the component graph is not a meter."
-                )
-            grid_successors = component_graph.successors(grid_meter.component_id)
 
-        first_iteration = True
-        for successor in iter(grid_successors):
-            # if in the future we support additional producers, we need to add them here
-            if component_graph.is_chp_chain(successor) or component_graph.is_pv_chain(
-                successor
-            ):
-                if not first_iteration:
-                    builder.push_oper("+")
+        component_graph = connection_manager.get().component_graph
+        # if in the future we support additional producers, we need to add them to the lambda
+        producer_components = component_graph.dfs(
+            self._get_grid_component(),
+            set(),
+            lambda component: component_graph.is_pv_chain(component)
+            or component_graph.is_chp_chain(component),
+        )
 
-                first_iteration = False
+        if not producer_components:
+            _logger.warning(
+                "Unable to find any producer components in the component graph. "
+                "Subscribing to the resampling actor with a non-existing "
+                "component id, so that `0` values are sent from the formula."
+            )
+            # If there are no producer components, we have to send 0 values at the same
+            # frequency as the other streams.  So we subscribe with a non-existing
+            # component id, just to get a `None` message at the resampling interval.
+            builder.push_component_metric(
+                NON_EXISTING_COMPONENT_ID, nones_are_zeros=True
+            )
+            return builder.build()
 
-                builder.push_component_metric(
-                    successor.component_id,
-                    nones_are_zeros=successor.category != ComponentCategory.METER,
-                )
+        for idx, component in enumerate(producer_components):
+            if idx > 0:
+                builder.push_oper("+")
 
-        if first_iteration:
             builder.push_component_metric(
-                NON_EXISTING_COMPONENT_ID, nones_are_zeros=True
+                component.component_id,
+                nones_are_zeros=component.category != ComponentCategory.METER,
             )
 
         return builder.build()

src/frequenz/sdk/timeseries/_formula_engine/_formula_generators/_pv_power_formula.py

@@ -3,10 +3,7 @@
 
 """Formula generator for PV Power, from the component graph."""
 
-from __future__ import annotations
-
 import logging
-from collections import abc
 
 from ....microgrid import connection_manager
 from ....microgrid.component import ComponentCategory, ComponentMetricId
@@ -33,14 +30,20 @@ def generate(self) -> FormulaEngine[Power]:
             "pv-power", ComponentMetricId.ACTIVE_POWER, Power.from_watts
         )
 
-        pv_components = self._get_pv_power_components()
+        component_graph = connection_manager.get().component_graph
+        pv_components = component_graph.dfs(
+            self._get_grid_component(),
+            set(),
+            component_graph.is_pv_chain,
+        )
+
         if not pv_components:
             _logger.warning(
-                "Unable to find any PV inverters in the component graph. "
+                "Unable to find any PV components in the component graph. "
                 "Subscribing to the resampling actor with a non-existing "
                 "component id, so that `0` values are sent from the formula."
             )
-            # If there are no PV inverters, we have to send 0 values at the same
+            # If there are no PV components, we have to send 0 values at the same
             # frequency as the other streams.  So we subscribe with a non-existing
             # component id, just to get a `None` message at the resampling interval.
             builder.push_component_metric(
@@ -50,11 +53,15 @@ def generate(self) -> FormulaEngine[Power]:
 
         builder.push_oper("(")
         builder.push_oper("(")
-        for idx, comp_id in enumerate(pv_components):
+        for idx, component in enumerate(pv_components):
             if idx > 0:
                 builder.push_oper("+")
 
-            builder.push_component_metric(comp_id, nones_are_zeros=True)
+            # should only be the case if the component is not a meter
+            builder.push_component_metric(
+                component.component_id,
+                nones_are_zeros=component.category != ComponentCategory.METER,
+            )
         builder.push_oper(")")
         if self._config.formula_type == FormulaType.PRODUCTION:
             builder.push_oper("*")
@@ -65,31 +72,3 @@ def generate(self) -> FormulaEngine[Power]:
             builder.push_clipper(0.0, None)
 
         return builder.build()
-
-    def _get_pv_power_components(self) -> abc.Set[int]:
-        """Get the component ids of the PV inverters or meters in the component graph.
-
-        Returns:
-            A set of component ids of the PV inverters or meters in the component graph.
-
-        Raises:
-            RuntimeError: if the grid component has no PV inverters or meters as successors.
-        """
-        component_graph = connection_manager.get().component_graph
-        grid_successors = self._get_grid_component_successors()
-
-        if len(grid_successors) == 1:
-            successor = next(iter(grid_successors))
-            if successor.category != ComponentCategory.METER:
-                raise RuntimeError(
-                    "Only grid successor in the component graph is not a meter."
-                )
-            grid_successors = component_graph.successors(successor.component_id)
-
-        pv_meters_or_inverters: set[int] = set()
-
-        for successor in grid_successors:
-            if component_graph.is_pv_chain(successor):
-                pv_meters_or_inverters.add(successor.component_id)
-
-        return pv_meters_or_inverters

tests/timeseries/mock_microgrid.py

@@ -249,7 +249,7 @@ def add_consumer_meters(self, count: int = 1) -> None:
             self._connections.add(Connection(self._connect_to, meter_id))
             self._start_meter_streaming(meter_id)
 
-    def add_chps(self, count: int) -> None:
+    def add_chps(self, count: int, no_meters: bool = False) -> None:
         """Add CHPs with connected meters to the mock microgrid.
 
         Args:
@@ -263,12 +263,13 @@ def add_chps(self, count: int) -> None:
             self.meter_ids.append(meter_id)
             self.chp_ids.append(chp_id)
 
-            self._components.add(
-                Component(
-                    meter_id,
-                    ComponentCategory.METER,
+            if not no_meters:
+                self._components.add(
+                    Component(
+                        meter_id,
+                        ComponentCategory.METER,
+                    )
                 )
-            )
             self._components.add(
                 Component(
                     chp_id,
@@ -277,8 +278,11 @@ def add_chps(self, count: int) -> None:
             )
 
             self._start_meter_streaming(meter_id)
-            self._connections.add(Connection(self._connect_to, meter_id))
-            self._connections.add(Connection(meter_id, chp_id))
+            if no_meters:
+                self._connections.add(Connection(self._connect_to, chp_id))
+            else:
+                self._connections.add(Connection(self._connect_to, meter_id))
+                self._connections.add(Connection(meter_id, chp_id))
 
     def add_batteries(self, count: int, no_meter: bool = False) -> None:
         """Add batteries with connected inverters and meters to the microgrid.

tests/timeseries/test_logical_meter.py

@@ -240,6 +240,17 @@ async def test_pv_power_no_meter(self, mocker: MockerFixture) -> None:
             await pv_consumption_power_receiver.receive()
         ).value == Power.from_watts(0.0)
 
+    async def test_pv_power_no_pv_components(self, mocker: MockerFixture) -> None:
+        """Test the pv power formula without having any pv components."""
+        mockgrid = MockMicrogrid(grid_meter=True)
+        await mockgrid.start(mocker)
+
+        logical_meter = microgrid.logical_meter()
+        pv_power_receiver = logical_meter.pv_power.new_receiver()
+
+        await mockgrid.mock_resampler.send_non_existing_component_value()
+        assert (await pv_power_receiver.receive()).value == Power.zero()
+
     async def test_consumer_power_grid_meter(self, mocker: MockerFixture) -> None:
         """Test the consumer power formula with a grid meter."""
         mockgrid = MockMicrogrid(grid_meter=True)
@@ -313,15 +324,12 @@ async def test_producer_power_no_pv_no_consumer_meter(
     ) -> None:
         """Test the producer power formula without pv and without consumer meter."""
         mockgrid = MockMicrogrid(grid_meter=False)
-        mockgrid.add_chps(1)
+        mockgrid.add_chps(1, True)
         await mockgrid.start(mocker)
 
         logical_meter = microgrid.logical_meter()
         producer_power_receiver = logical_meter.producer_power.new_receiver()
 
-        # As there is only one meter in the microgrid, the formula interprets it
-        # as main meter instead of chp meter, so it reads the power from the
-        # chp component directly.
         await mockgrid.mock_resampler.send_chp_power([2.0])
         assert (await producer_power_receiver.receive()).value == Power.from_watts(2.0)