Always require a priority argument in calls to microgrid.*_pool methods (#944)

This PR also makes the methods accept only keyword arguments.

Closes #916

Author: shsms

RELEASE_NOTES.md

@@ -6,7 +6,9 @@
 
 ## Upgrading
 
-<!-- Here goes notes on how to upgrade from previous versions, including deprecations and what they should be replaced with -->
+- Calls to `microgrid.*_pool` methods now always need to specified a priority value, corresponding to the requirements/priority of the actor making the call.
+
+- The `microgrid.*_pool` methods would only accept keyword arguments from now on.
 
 ## New Features
 

benchmarks/power_distribution/power_distributor.py

@@ -50,7 +50,7 @@ async def send_requests(batteries: set[int], request_num: int) -> list[Result]:
     Returns:
         List of the results from the PowerDistributingActor.
     """
-    battery_pool = microgrid.battery_pool(batteries)
+    battery_pool = microgrid.battery_pool(priority=5, component_ids=batteries)
     results_rx = battery_pool.power_status.new_receiver()
     result: list[Any] = []
     for _ in range(request_num):

examples/battery_pool.py

@@ -28,7 +28,7 @@ async def main() -> None:
         resampler_config=ResamplerConfig(resampling_period=timedelta(seconds=1.0)),
     )
 
-    battery_pool = microgrid.battery_pool()
+    battery_pool = microgrid.battery_pool(priority=5)
     receivers = [
         battery_pool.soc.new_receiver(limit=1),
         battery_pool.capacity.new_receiver(limit=1),

src/frequenz/sdk/actor/_power_managing/_power_managing_actor.py

@@ -7,6 +7,7 @@
 
 import asyncio
 import logging
+import sys
 import typing
 from datetime import datetime, timedelta, timezone
 
@@ -146,16 +147,22 @@ def _add_bounds_tracker(self, component_ids: frozenset[int]) -> None:
         bounds_receiver: Receiver[SystemBounds]
         # pylint: disable=protected-access
         if self._component_category is ComponentCategory.BATTERY:
-            battery_pool = microgrid.battery_pool(component_ids)
+            battery_pool = microgrid.battery_pool(
+                priority=-sys.maxsize - 1, component_ids=component_ids
+            )
             bounds_receiver = battery_pool._system_power_bounds.new_receiver()
         elif self._component_category is ComponentCategory.EV_CHARGER:
-            ev_charger_pool = microgrid.ev_charger_pool(component_ids)
+            ev_charger_pool = microgrid.ev_charger_pool(
+                priority=-sys.maxsize - 1, component_ids=component_ids
+            )
             bounds_receiver = ev_charger_pool._system_power_bounds.new_receiver()
         elif (
             self._component_category is ComponentCategory.INVERTER
             and self._component_type is InverterType.SOLAR
         ):
-            pv_pool = microgrid.pv_pool(component_ids)
+            pv_pool = microgrid.pv_pool(
+                priority=-sys.maxsize - 1, component_ids=component_ids
+            )
             bounds_receiver = pv_pool._system_power_bounds.new_receiver()
         # pylint: enable=protected-access
         else:

src/frequenz/sdk/microgrid/_data_pipeline.py

@@ -11,7 +11,6 @@
 from __future__ import annotations
 
 import logging
-import sys
 import typing
 from collections import abc
 from dataclasses import dataclass
@@ -185,21 +184,22 @@ def producer(self) -> Producer:
 
     def ev_charger_pool(
         self,
-        ev_charger_ids: abc.Set[int] | None = None,
+        *,
+        priority: int,
+        component_ids: abc.Set[int] | None = None,
         name: str | None = None,
-        priority: int = -sys.maxsize - 1,
     ) -> EVChargerPool:
         """Return the corresponding EVChargerPool instance for the given ids.
 
         If an EVChargerPool instance for the given ids doesn't exist, a new one is
         created and returned.
 
         Args:
-            ev_charger_ids: Optional set of IDs of EV Chargers to be managed by the
+            priority: The priority of the actor making the call.
+            component_ids: Optional set of IDs of EV Chargers to be managed by the
                 EVChargerPool.
             name: An optional name used to identify this instance of the pool or a
                 corresponding actor in the logs.
-            priority: The priority of the actor making the call.
 
         Returns:
             An EVChargerPool instance.
@@ -214,8 +214,8 @@ def ev_charger_pool(
 
         # We use frozenset to make a hashable key from the input set.
         ref_store_key: frozenset[int] = frozenset()
-        if ev_charger_ids is not None:
-            ref_store_key = frozenset(ev_charger_ids)
+        if component_ids is not None:
+            ref_store_key = frozenset(component_ids)
 
         pool_key = f"{ref_store_key}-{priority}"
         if pool_key in self._known_pool_keys:
@@ -226,7 +226,7 @@ def ev_charger_pool(
                 "consider reusing the same instance."
                 "\n  Hint: If the instances are created from different actors, "
                 "consider using different priorities to distinguish them.",
-                ev_charger_ids,
+                component_ids,
                 priority,
             )
         else:
@@ -246,7 +246,7 @@ def ev_charger_pool(
                     power_manager_bounds_subs_sender=(
                         self._ev_power_wrapper.bounds_subscription_channel.new_sender()
                     ),
-                    component_ids=ev_charger_ids,
+                    component_ids=component_ids,
                 )
             )
         return EVChargerPool(
@@ -255,21 +255,22 @@ def ev_charger_pool(
 
     def pv_pool(
         self,
-        pv_inverter_ids: abc.Set[int] | None = None,
+        *,
+        priority: int,
+        component_ids: abc.Set[int] | None = None,
         name: str | None = None,
-        priority: int = -sys.maxsize - 1,
     ) -> PVPool:
         """Return a new `PVPool` instance for the given ids.
 
         If a `PVPoolReferenceStore` instance for the given PV inverter ids doesn't
         exist, a new one is created and used for creating the `PVPool`.
 
         Args:
-            pv_inverter_ids: Optional set of IDs of PV inverters to be managed by the
+            priority: The priority of the actor making the call.
+            component_ids: Optional set of IDs of PV inverters to be managed by the
                 `PVPool`.
             name: An optional name used to identify this instance of the pool or a
                 corresponding actor in the logs.
-            priority: The priority of the actor making the call.
 
         Returns:
             A `PVPool` instance.
@@ -282,8 +283,8 @@ def pv_pool(
 
         # We use frozenset to make a hashable key from the input set.
         ref_store_key: frozenset[int] = frozenset()
-        if pv_inverter_ids is not None:
-            ref_store_key = frozenset(pv_inverter_ids)
+        if component_ids is not None:
+            ref_store_key = frozenset(component_ids)
 
         pool_key = f"{ref_store_key}-{priority}"
         if pool_key in self._known_pool_keys:
@@ -294,7 +295,7 @@ def pv_pool(
                 "consider reusing the same instance."
                 "\n  Hint: If the instances are created from different actors, "
                 "consider using different priorities to distinguish them.",
-                pv_inverter_ids,
+                component_ids,
                 priority,
             )
         else:
@@ -313,7 +314,7 @@ def pv_pool(
                 power_manager_bounds_subs_sender=(
                     self._pv_power_wrapper.bounds_subscription_channel.new_sender()
                 ),
-                component_ids=pv_inverter_ids,
+                component_ids=component_ids,
             )
 
         return PVPool(self._pv_pool_reference_stores[ref_store_key], name, priority)
@@ -331,21 +332,22 @@ def grid(self) -> Grid:
 
     def battery_pool(
         self,
-        battery_ids: abc.Set[int] | None = None,
+        *,
+        priority: int,
+        component_ids: abc.Set[int] | None = None,
         name: str | None = None,
-        priority: int = -sys.maxsize - 1,
     ) -> BatteryPool:
         """Return a new `BatteryPool` instance for the given ids.
 
-        If a `BatteryPoolReferenceStore` instance for the given battery ids doesn't exist,
-        a new one is created and used for creating the `BatteryPool`.
+        If a `BatteryPoolReferenceStore` instance for the given battery ids doesn't
+        exist, a new one is created and used for creating the `BatteryPool`.
 
         Args:
-            battery_ids: Optional set of IDs of batteries to be managed by the
+            priority: The priority of the actor making the call.
+            component_ids: Optional set of IDs of batteries to be managed by the
                 `BatteryPool`.
             name: An optional name used to identify this instance of the pool or a
                 corresponding actor in the logs.
-            priority: The priority of the actor making the call.
 
         Returns:
             A `BatteryPool` instance.
@@ -360,8 +362,8 @@ def battery_pool(
 
         # We use frozenset to make a hashable key from the input set.
         ref_store_key: frozenset[int] = frozenset()
-        if battery_ids is not None:
-            ref_store_key = frozenset(battery_ids)
+        if component_ids is not None:
+            ref_store_key = frozenset(component_ids)
 
         pool_key = f"{ref_store_key}-{priority}"
         if pool_key in self._known_pool_keys:
@@ -372,7 +374,7 @@ def battery_pool(
                 "consider reusing the same instance."
                 "\n  Hint: If the instances are created from different actors, "
                 "consider using different priorities to distinguish them.",
-                battery_ids,
+                component_ids,
                 priority,
             )
         else:
@@ -393,7 +395,7 @@ def battery_pool(
                         self._battery_power_wrapper.bounds_subscription_channel.new_sender()
                     ),
                     min_update_interval=self._resampler_config.resampling_period,
-                    batteries_id=battery_ids,
+                    batteries_id=component_ids,
                 )
             )
 
@@ -505,9 +507,10 @@ def producer() -> Producer:
 
 
 def ev_charger_pool(
-    ev_charger_ids: abc.Set[int] | None = None,
+    *,
+    priority: int,
+    component_ids: abc.Set[int] | None = None,
     name: str | None = None,
-    priority: int = -sys.maxsize - 1,
 ) -> EVChargerPool:
     """Return a new `EVChargerPool` instance for the given parameters.
 
@@ -528,23 +531,26 @@ def ev_charger_pool(
         power.
 
     Args:
-        ev_charger_ids: Optional set of IDs of EV Chargers to be managed by the
+        priority: The priority of the actor making the call.
+        component_ids: Optional set of IDs of EV Chargers to be managed by the
             EVChargerPool.  If not specified, all EV Chargers available in the
             component graph are used.
         name: An optional name used to identify this instance of the pool or a
             corresponding actor in the logs.
-        priority: The priority of the actor making the call.
 
     Returns:
         An `EVChargerPool` instance.
     """
-    return _get().ev_charger_pool(ev_charger_ids, name, priority)
+    return _get().ev_charger_pool(
+        priority=priority, component_ids=component_ids, name=name
+    )
 
 
 def battery_pool(
-    battery_ids: abc.Set[int] | None = None,
+    *,
+    priority: int,
+    component_ids: abc.Set[int] | None = None,
     name: str | None = None,
-    priority: int = -sys.maxsize - 1,
 ) -> BatteryPool:
     """Return a new `BatteryPool` instance for the given parameters.
 
@@ -565,22 +571,26 @@ def battery_pool(
         power.
 
     Args:
-        battery_ids: Optional set of IDs of batteries to be managed by the `BatteryPool`.
-            If not specified, all batteries available in the component graph are used.
+        priority: The priority of the actor making the call.
+        component_ids: Optional set of IDs of batteries to be managed by the
+            `BatteryPool`.  If not specified, all batteries available in the component
+            graph are used.
         name: An optional name used to identify this instance of the pool or a
             corresponding actor in the logs.
-        priority: The priority of the actor making the call.
 
     Returns:
         A `BatteryPool` instance.
     """
-    return _get().battery_pool(battery_ids, name, priority)
+    return _get().battery_pool(
+        priority=priority, component_ids=component_ids, name=name
+    )
 
 
 def pv_pool(
-    pv_inverter_ids: abc.Set[int] | None = None,
+    *,
+    priority: int,
+    component_ids: abc.Set[int] | None = None,
     name: str | None = None,
-    priority: int = -sys.maxsize - 1,
 ) -> PVPool:
     """Return a new `PVPool` instance for the given parameters.
 
@@ -601,17 +611,17 @@ def pv_pool(
         power.
 
     Args:
-        pv_inverter_ids: Optional set of IDs of PV inverters to be managed by the
+        priority: The priority of the actor making the call.
+        component_ids: Optional set of IDs of PV inverters to be managed by the
             `PVPool`. If not specified, all PV inverters available in the component
             graph are used.
         name: An optional name used to identify this instance of the pool or a
             corresponding actor in the logs.
-        priority: The priority of the actor making the call.
 
     Returns:
         A `PVPool` instance.
     """
-    return _get().pv_pool(pv_inverter_ids, name, priority)
+    return _get().pv_pool(priority=priority, component_ids=component_ids, name=name)
 
 
 def grid() -> Grid:

src/frequenz/sdk/timeseries/battery_pool/_result_types.py

@@ -66,7 +66,9 @@ def adjust_to_bounds(self, power: Power) -> tuple[Power | None, Power | None]:
             ```python
             from frequenz.sdk import microgrid
 
-            power_status_rx = microgrid.battery_pool().power_status.new_receiver()
+            power_status_rx = microgrid.battery_pool(
+                priority=5,
+            ).power_status.new_receiver()
             power_status = await power_status_rx.receive()
             desired_power = Power.from_watts(1000.0)
 

src/frequenz/sdk/timeseries/formula_engine/_formula_engine.py

@@ -253,7 +253,7 @@ class FormulaEngine(
     ```python
     from frequenz.sdk import microgrid
 
-    battery_pool = microgrid.battery_pool()
+    battery_pool = microgrid.battery_pool(priority=5)
 
     async for power in battery_pool.power.new_receiver():
         print(f"{power=}")
@@ -277,8 +277,8 @@ class FormulaEngine(
     from frequenz.sdk import microgrid
 
     logical_meter = microgrid.logical_meter()
-    battery_pool = microgrid.battery_pool()
-    ev_charger_pool = microgrid.ev_charger_pool()
+    battery_pool = microgrid.battery_pool(priority=5)
+    ev_charger_pool = microgrid.ev_charger_pool(priority=5)
     grid = microgrid.grid()
 
     # apply operations on formula engines to create a formula engine that would
@@ -459,7 +459,7 @@ class FormulaEngine3Phase(
     ```python
     from frequenz.sdk import microgrid
 
-    ev_charger_pool = microgrid.ev_charger_pool()
+    ev_charger_pool = microgrid.ev_charger_pool(priority=5)
 
     async for sample in ev_charger_pool.current.new_receiver():
         print(f"Current: {sample}")
@@ -474,7 +474,7 @@ class FormulaEngine3Phase(
     from frequenz.sdk import microgrid
 
     logical_meter = microgrid.logical_meter()
-    ev_charger_pool = microgrid.ev_charger_pool()
+    ev_charger_pool = microgrid.ev_charger_pool(priority=5)
     grid = microgrid.grid()
 
     # Calculate grid consumption current that's not used by the EV chargers

src/frequenz/sdk/timeseries/logical_meter/_logical_meter.py

@@ -42,7 +42,7 @@ class LogicalMeter:
         )
 
         logical_meter = microgrid.logical_meter()
-        pv_pool = microgrid.pv_pool()
+        pv_pool = microgrid.pv_pool(priority=5)
         grid = microgrid.grid()
 
         # Get a receiver for a builtin formula