Improve target_power calculation and handling of overlapping bounds

When actors with different priorities have not completely-contained,
but overlapping bounds, use the intersection to find the target_power
closest to the requirements of the lowest priority actor.

This commit also adds some test cases and updates the test to run
directly on a `Matryoshka()` instance, rather than through a
`PowerManagingActor` instance.

Signed-off-by: Sahas Subramanian <[email protected]>

Author: shsms

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

@@ -99,15 +99,17 @@ def get_target_power(
 
         target_power = Power.zero()
         for next_proposal in reversed(proposals):
-            if (
-                next_proposal.preferred_power > upper_bound
-                or next_proposal.preferred_power < lower_bound
-            ):
-                continue
-            target_power = next_proposal.preferred_power
+            if upper_bound < lower_bound:
+                break
+            if next_proposal.preferred_power > upper_bound:
+                target_power = upper_bound
+            elif next_proposal.preferred_power < lower_bound:
+                target_power = lower_bound
+            else:
+                target_power = next_proposal.preferred_power
             if next_proposal.bounds:
-                lower_bound = next_proposal.bounds[0]
-                upper_bound = next_proposal.bounds[1]
+                lower_bound = max(lower_bound, next_proposal.bounds[0])
+                upper_bound = min(upper_bound, next_proposal.bounds[1])
 
         if (
             battery_ids not in self._target_power
@@ -146,8 +148,8 @@ def get_status(
             if next_proposal.priority <= priority:
                 break
             if next_proposal.bounds:
-                lower_bound = next_proposal.bounds[0]
-                upper_bound = next_proposal.bounds[1]
+                lower_bound = max(lower_bound, next_proposal.bounds[0])
+                upper_bound = min(upper_bound, next_proposal.bounds[1])
 
         return Report(
             target_power=self._target_power.get(battery_ids, Power.zero()),

tests/actor/_power_managing/test_power_managing_actor.py

@@ -1,85 +1,72 @@
 # License: MIT
 # Copyright © 2023 Frequenz Energy-as-a-Service GmbH
 
-"""Tests for the power managing actor."""
+"""Tests for the Matryoshka power manager algorithm."""
 
 from datetime import datetime, timezone
-from unittest.mock import AsyncMock
 
-from frequenz.channels import Broadcast
-from pytest_mock import MockerFixture
-
-from frequenz.sdk.actor._channel_registry import ChannelRegistry
-from frequenz.sdk.actor._power_managing import Proposal, ReportRequest
-from frequenz.sdk.actor._power_managing._power_managing_actor import PowerManagingActor
-from frequenz.sdk.actor.power_distributing.request import Request
+from frequenz.sdk.actor._power_managing import Proposal
+from frequenz.sdk.actor._power_managing._matryoshka import Matryoshka
 from frequenz.sdk.timeseries import Power, battery_pool
 
 
-async def test_power_managing_actor_matryoshka(mocker: MockerFixture) -> None:
+async def test_matryoshka_algorithm() -> None:
     """Tests for the power managing actor."""
-    input_channel = Broadcast[Proposal]("power managing proposals")
-    output_channel = Broadcast[Request]("Power managing outputs")
-    power_bounds_sub_channel = Broadcast[ReportRequest]("power bounds subscription")
-    channel_registry = ChannelRegistry(name="test_channel_registry")
-    input_tx = input_channel.new_sender()
-    output_rx = output_channel.new_receiver()
-
     batteries = frozenset({2, 5})
 
-    mocker.patch(
-        "frequenz.sdk.actor._power_managing._power_managing_actor"
-        ".PowerManagingActor._add_bounds_tracker",
-        new_callable=AsyncMock,
+    algorithm = Matryoshka()
+    system_bounds = battery_pool.PowerMetrics(
+        timestamp=datetime.now(tz=timezone.utc),
+        inclusion_bounds=battery_pool.Bounds(
+            lower=Power.from_watts(-200.0), upper=Power.from_watts(200.0)
+        ),
+        exclusion_bounds=battery_pool.Bounds(lower=Power.zero(), upper=Power.zero()),
     )
 
-    async def case(
-        *,
+    call_count = 0
+
+    def case(
         priority: int,
         power: float,
-        bounds: tuple[float, float],
+        bounds: tuple[float, float] | None,
         expected: float | None,
     ) -> None:
-        await input_tx.send(
+        nonlocal call_count
+        call_count += 1
+        tgt_power = algorithm.get_target_power(
+            batteries,
             Proposal(
                 battery_ids=batteries,
                 source_id=f"actor-{priority}",
                 preferred_power=Power.from_watts(power),
-                bounds=(Power.from_watts(bounds[0]), Power.from_watts(bounds[1])),
+                bounds=(Power.from_watts(bounds[0]), Power.from_watts(bounds[1]))
+                if bounds
+                else None,
                 priority=priority,
-            )
-        )
-        if expected is not None:
-            assert (await output_rx.receive()).power.as_watts() == expected
-
-    async with PowerManagingActor(
-        input_channel.new_receiver(),
-        power_bounds_sub_channel.new_receiver(),
-        output_channel.new_sender(),
-        channel_registry,
-    ) as powmgract:
-        powmgract._system_bounds[  # pylint: disable=protected-access
-            batteries
-        ] = battery_pool.PowerMetrics(
-            timestamp=datetime.now(tz=timezone.utc),
-            inclusion_bounds=battery_pool.Bounds(
-                lower=Power.from_watts(-200.0), upper=Power.from_watts(200.0)
-            ),
-            exclusion_bounds=battery_pool.Bounds(
-                lower=Power.zero(), upper=Power.zero()
             ),
+            system_bounds,
         )
+        assert tgt_power == (
+            Power.from_watts(expected) if expected is not None else None
+        )
+
+    case(priority=2, power=25.0, bounds=(25.0, 50.0), expected=25.0)
+    case(priority=1, power=20.0, bounds=(20.0, 50.0), expected=None)
+    case(priority=3, power=10.0, bounds=(10.0, 15.0), expected=15.0)
+    case(priority=3, power=10.0, bounds=(10.0, 22.0), expected=22.0)
+    case(priority=1, power=30.0, bounds=(20.0, 50.0), expected=None)
+    case(priority=3, power=10.0, bounds=(10.0, 50.0), expected=30.0)
+    case(priority=2, power=40.0, bounds=(40.0, 50.0), expected=40.0)
+    case(priority=2, power=0.0, bounds=(-200.0, 200.0), expected=30.0)
+    case(priority=4, power=-50.0, bounds=(-200.0, -50.0), expected=-50.0)
+    case(priority=3, power=-0.0, bounds=(-200.0, 200.0), expected=None)
+    case(priority=1, power=-150.0, bounds=(-200.0, -150.0), expected=-150.0)
+    case(priority=4, power=-180.0, bounds=(-200.0, -50.0), expected=None)
+    case(priority=4, power=50.0, bounds=(50.0, 200.0), expected=50.0)
 
-        await case(priority=2, power=25.0, bounds=(25.0, 50.0), expected=25.0)
-        await case(priority=1, power=20.0, bounds=(20.0, 50.0), expected=None)
-        await case(priority=3, power=10.0, bounds=(10.0, 15.0), expected=10.0)
-        await case(priority=3, power=10.0, bounds=(10.0, 22.0), expected=20.0)
-        await case(priority=1, power=30.0, bounds=(20.0, 50.0), expected=10.0)
-        await case(priority=3, power=10.0, bounds=(10.0, 50.0), expected=30.0)
-        await case(priority=2, power=40.0, bounds=(40.0, 50.0), expected=40.0)
-        await case(priority=2, power=0.0, bounds=(-200.0, 200.0), expected=30.0)
-        await case(priority=4, power=-50.0, bounds=(-200.0, -50.0), expected=-50.0)
-        await case(priority=3, power=-0.0, bounds=(-200.0, 200.0), expected=None)
-        await case(priority=1, power=-150.0, bounds=(-200.0, -150.0), expected=-150.0)
-        await case(priority=4, power=-180.0, bounds=(-200.0, -50.0), expected=None)
-        await case(priority=4, power=50.0, bounds=(50.0, 200.0), expected=50.0)
+    case(priority=4, power=0.0, bounds=(-200.0, 200.0), expected=-150.0)
+    case(priority=3, power=0.0, bounds=(-200.0, 200.0), expected=None)
+    case(priority=2, power=50.0, bounds=(-100, 100), expected=-100.0)
+    case(priority=1, power=100.0, bounds=(100, 200), expected=100.0)
+    case(priority=1, power=50.0, bounds=(50, 200), expected=50.0)
+    case(priority=1, power=200.0, bounds=(50, 200), expected=100.0)