Apply exclusion bounds only after the target power is calculated

Earlier, the exclusion bounds were applied to every proposal, and in
some cases, that was leading to much higher target powers than what
was necessary.

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

Author: shsms

src/frequenz/sdk/microgrid/_power_managing/_shifting_matryoshka.py

@@ -80,19 +80,11 @@ def _calc_targets(  # pylint: disable=too-many-locals,too-many-branches
         if not proposals:
             return None, Bounds[Power](lower=lower_bound, upper=upper_bound)
 
-        exclusion_bounds = None
-        if system_bounds.exclusion_bounds is not None and (
-            system_bounds.exclusion_bounds.lower != Power.zero()
-            or system_bounds.exclusion_bounds.upper != Power.zero()
-        ):
-            exclusion_bounds = Bounds(
-                system_bounds.exclusion_bounds.lower,
-                system_bounds.exclusion_bounds.upper,
-            )
+        available_bounds = Bounds[Power](lower=lower_bound, upper=upper_bound)
+        top_pri_bounds: Bounds[Power] | None = None
 
         target_power = Power.zero()
         for next_proposal in sorted(proposals, reverse=True):
-            unshifted_power = Power.zero()
             if priority is not None and next_proposal.priority <= priority:
                 break
 
@@ -112,45 +104,57 @@ def _calc_targets(  # pylint: disable=too-many-locals,too-many-branches
                 continue
 
             if proposal_lower >= upper_bound:
-                if proposal_power:
-                    proposal_power = upper_bound
+                proposal_lower = upper_bound
+                proposal_upper = upper_bound
             elif proposal_upper <= lower_bound:
-                if proposal_power:
-                    proposal_power = lower_bound
-            else:
-                lower_bound = max(lower_bound, proposal_lower)
-                upper_bound = min(upper_bound, proposal_upper)
-
-            if proposal_power:
-                match _bounds.clamp_to_bounds(
+                proposal_lower = lower_bound
+                proposal_upper = lower_bound
+
+            lower_bound = max(lower_bound, proposal_lower)
+            upper_bound = min(upper_bound, proposal_upper)
+
+            if proposal_power is not None:
+                if top_pri_bounds is None and proposal_power != Power.zero():
+                    top_pri_bounds = Bounds[Power](lower=lower_bound, upper=upper_bound)
+                clamped = _bounds.clamp_to_bounds(
                     proposal_power,
                     lower_bound,
                     upper_bound,
-                    exclusion_bounds,
-                ):
+                    None,
+                )
+                match clamped:
+                    case (None, None):
+                        proposal_power = Power.zero()
                     case (None, power) | (power, None) if power:
-                        unshifted_power = power
+                        proposal_power = power
                     case (power_low, power_high) if power_low and power_high:
                         if power_high - proposal_power < proposal_power - power_low:
-                            unshifted_power = power_high
+                            proposal_power = power_high
                         else:
-                            unshifted_power = power_low
-                    case _:
-                        pass
-
-            lower_bound, upper_bound = _bounds.adjust_exclusion_bounds(
-                lower_bound, upper_bound, exclusion_bounds
-            )
-
-            lower_bound = lower_bound - unshifted_power
-            upper_bound = upper_bound - unshifted_power
-            target_power += unshifted_power
-
-            if exclusion_bounds is not None:
-                exclusion_bounds = Bounds[Power](
-                    exclusion_bounds.lower - unshifted_power,
-                    exclusion_bounds.upper - unshifted_power,
-                )
+                            proposal_power = power_low
+                lower_bound = lower_bound - proposal_power
+                upper_bound = upper_bound - proposal_power
+                target_power += proposal_power
+
+        if top_pri_bounds is not None:
+            available_bounds = top_pri_bounds
+
+        clamped = _bounds.clamp_to_bounds(
+            target_power,
+            available_bounds.lower,
+            available_bounds.upper,
+            system_bounds.exclusion_bounds,
+        )
+        match clamped:
+            case (None, None):
+                target_power = Power.zero()
+            case (None, power) | (power, None) if power:
+                target_power = power
+            case (power_low, power_high) if power_low and power_high:
+                if power_high - target_power < target_power - power_low:
+                    target_power = power_high
+                else:
+                    target_power = power_low
 
         return target_power, Bounds[Power](lower=lower_bound, upper=upper_bound)
 

tests/actor/_power_managing/test_shifting_matryoshka.py

@@ -122,15 +122,15 @@ async def test_matryoshka_no_excl() -> None:  # pylint: disable=too-many-stateme
     tester.tgt_power(priority=3, power=10.0, bounds=(10.0, 15.0), expected=15.0)
     tester.bounds(priority=3, expected_power=15.0, expected_bounds=(-200.0, 200.0))
     tester.bounds(priority=2, expected_power=15.0, expected_bounds=(0.0, 5.0))
-    tester.bounds(priority=1, expected_power=15.0, expected_bounds=(-5.0, 0.0))
+    tester.bounds(priority=1, expected_power=15.0, expected_bounds=(0.0, 0.0))
 
     tester.tgt_power(priority=3, power=10.0, bounds=(10.0, 22.0), expected=22.0)
     tester.bounds(priority=3, expected_power=22.0, expected_bounds=(-200.0, 200.0))
     tester.bounds(priority=2, expected_power=22.0, expected_bounds=(0.0, 12.0))
-    tester.bounds(priority=1, expected_power=22.0, expected_bounds=(-12.0, 0.0))
+    tester.bounds(priority=1, expected_power=22.0, expected_bounds=(0.0, 0.0))
 
     tester.tgt_power(priority=1, power=30.0, bounds=(20.0, 50.0), expected=None)
-    tester.bounds(priority=1, expected_power=22.0, expected_bounds=(-12.0, 0.0))
+    tester.bounds(priority=1, expected_power=22.0, expected_bounds=(0.0, 0.0))
 
     tester.tgt_power(priority=3, power=10.0, bounds=(10.0, 50.0), expected=50.0)
     tester.bounds(priority=3, expected_power=50.0, expected_bounds=(-200.0, 200.0))
@@ -140,7 +140,7 @@ async def test_matryoshka_no_excl() -> None:  # pylint: disable=too-many-stateme
     tester.tgt_power(priority=2, power=40.0, bounds=(40.0, None), expected=None)
     tester.bounds(priority=3, expected_power=50.0, expected_bounds=(-200.0, 200.0))
     tester.bounds(priority=2, expected_power=50.0, expected_bounds=(0.0, 40.0))
-    tester.bounds(priority=1, expected_power=50.0, expected_bounds=(-40.0, 0.0))
+    tester.bounds(priority=1, expected_power=50.0, expected_bounds=(0.0, 0.0))
 
     tester.tgt_power(priority=2, power=0.0, bounds=(-200.0, 200.0), expected=40.0)
     tester.bounds(priority=4, expected_power=40.0, expected_bounds=(-200.0, 200.0))
@@ -151,8 +151,8 @@ async def test_matryoshka_no_excl() -> None:  # pylint: disable=too-many-stateme
     tester.tgt_power(priority=4, power=-50.0, bounds=(None, -50.0), expected=-50.0)
     tester.bounds(priority=4, expected_power=-50.0, expected_bounds=(-200.0, 200.0))
     tester.bounds(priority=3, expected_power=-50.0, expected_bounds=(-150.0, 0.0))
-    tester.bounds(priority=2, expected_power=-50.0, expected_bounds=(-150.0, 0.0))
-    tester.bounds(priority=1, expected_power=-50.0, expected_bounds=(-150.0, 0.0))
+    tester.bounds(priority=2, expected_power=-50.0, expected_bounds=(0.0, 0.0))
+    tester.bounds(priority=1, expected_power=-50.0, expected_bounds=(0.0, 0.0))
 
     tester.tgt_power(priority=3, power=0.0, bounds=(-200.0, 200.0), expected=None)
     tester.bounds(priority=1, expected_power=-50.0, expected_bounds=(-150.0, 0.0))
@@ -202,6 +202,35 @@ async def test_matryoshka_no_excl() -> None:  # pylint: disable=too-many-stateme
     tester.bounds(priority=1, expected_power=50.0, expected_bounds=(-150.0, 50.0))
 
 
+async def test_matryoshka_simple() -> None:
+    """Tests for the power managing actor.
+
+    With inclusion bounds, and exclusion bounds -30.0 to 0.0.
+    """
+    batteries = frozenset({2, 5})
+
+    system_bounds = _base_types.SystemBounds(
+        timestamp=datetime.now(tz=timezone.utc),
+        inclusion_bounds=timeseries.Bounds(
+            lower=Power.from_watts(-200.0), upper=Power.from_watts(200.0)
+        ),
+        exclusion_bounds=timeseries.Bounds(
+            lower=Power.from_watts(-30.0), upper=Power.from_watts(30.0)
+        ),
+    )
+
+    tester = StatefulTester(batteries, system_bounds)
+    tester.tgt_power(priority=3, power=None, bounds=(-200.0, 200.0), expected=0.0)
+    tester.bounds(priority=2, expected_power=0.0, expected_bounds=(-200.0, 200.0))
+    tester.tgt_power(priority=1, power=None, bounds=(-200.0, 200.0), expected=None)
+    tester.bounds(priority=2, expected_power=0.0, expected_bounds=(-200.0, 200.0))
+    tester.tgt_power(priority=2, power=25.0, bounds=(25.0, 50.0), expected=30.0)
+    # tester.tgt_power(priority=2, power=-10.0, bounds=(-10.0, 50.0), expected=-10.0)
+    # tester.bounds(priority=1, expected_power=-10.0, expected_bounds=(0.0, 60.0))
+    # tester.tgt_power(priority=1, power=-10.0, bounds=(-10.0, 20.0), expected=None)
+    # tester.bounds(priority=0, expected_power=-10.0, expected_bounds=(0.0, 20.0))
+
+
 async def test_matryoshka_with_excl_1() -> None:
     """Tests for the power managing actor.
 
@@ -228,32 +257,29 @@ async def test_matryoshka_with_excl_1() -> None:
     tester.tgt_power(priority=1, power=20.0, bounds=(20.0, 50.0), expected=45.0)
     tester.bounds(priority=1, expected_power=45.0, expected_bounds=(0.0, 25.0))
 
-    tester.tgt_power(priority=2, power=-10.0, bounds=(-10.0, 50.0), expected=20.0)
-    tester.bounds(priority=1, expected_power=20.0, expected_bounds=(0.0, 50.0))
-    tester.bounds(priority=0, expected_power=20.0, expected_bounds=(0.0, 30.0))
+    tester.tgt_power(priority=2, power=-10.0, bounds=(-10.0, 50.0), expected=10.0)
+    tester.bounds(priority=1, expected_power=10.0, expected_bounds=(0.0, 60.0))
+    tester.bounds(priority=0, expected_power=10.0, expected_bounds=(0.0, 30.0))
 
     tester.tgt_power(priority=1, power=-10.0, bounds=(-10.0, 50.0), expected=0.0)
     tester.bounds(priority=0, expected_power=0.0, expected_bounds=(0.0, 50.0))
 
-    tester.tgt_power(priority=1, power=-10.0, bounds=(-10.0, 20.0), expected=None)
-    tester.bounds(priority=0, expected_power=0.0, expected_bounds=(0.0, 20.0))
-
     tester.tgt_power(priority=1, power=-10.0, bounds=(-10.0, -5.0), expected=None)
-    tester.bounds(priority=0, expected_power=0.0, expected_bounds=(0.0, 50.0))
+    tester.bounds(priority=0, expected_power=0.0, expected_bounds=(0.0, 0.0))
 
-    tester.tgt_power(priority=2, power=-10.0, bounds=(-200.0, -5.0), expected=-40.0)
-    tester.bounds(priority=1, expected_power=-40.0, expected_bounds=(-170.0, 0.0))
-    tester.bounds(priority=0, expected_power=-40.0, expected_bounds=(0.0, 5.0))
+    tester.tgt_power(priority=2, power=-10.0, bounds=(-200.0, -5.0), expected=-30.0)
+    tester.bounds(priority=1, expected_power=-30.0, expected_bounds=(-190.0, 5.0))
+    tester.bounds(priority=0, expected_power=-30.0, expected_bounds=(0.0, 5.0))
 
     tester.tgt_power(priority=2, power=None, bounds=(None, None), expected=0.0)
     tester.bounds(priority=2, expected_power=0.0, expected_bounds=(-200.0, 200.0))
     tester.bounds(priority=1, expected_power=0.0, expected_bounds=(-200.0, 200.0))
 
     tester.tgt_power(priority=1, power=-10.0, bounds=(-10.0, -5.0), expected=None)
-    tester.bounds(priority=0, expected_power=0.0, expected_bounds=(0.0, 0.0))
+    tester.bounds(priority=0, expected_power=0.0, expected_bounds=(0.0, 5.0))
 
     tester.tgt_power(priority=1, power=-10.0, bounds=(-100.0, -5.0), expected=-30.0)
-    tester.bounds(priority=0, expected_power=-30.0, expected_bounds=(-70.0, 0.0))
+    tester.bounds(priority=0, expected_power=-30.0, expected_bounds=(-90.0, 5.0))
 
     tester.tgt_power(priority=1, power=-40.0, bounds=(-100.0, -35.0), expected=-40.0)
     tester.bounds(priority=0, expected_power=-40.0, expected_bounds=(-60.0, 5.0))
@@ -280,13 +306,13 @@ async def test_matryoshka_with_excl_2() -> None:
 
     tester.tgt_power(priority=2, power=25.0, bounds=(25.0, 50.0), expected=30.0)
     tester.bounds(priority=2, expected_power=30.0, expected_bounds=(-200.0, 200.0))
-    tester.bounds(priority=1, expected_power=30.0, expected_bounds=(0.0, 20.0))
+    tester.bounds(priority=1, expected_power=30.0, expected_bounds=(0.0, 25.0))
 
-    tester.tgt_power(priority=1, power=20.0, bounds=(20.0, 50.0), expected=50.0)
-    tester.bounds(priority=1, expected_power=50.0, expected_bounds=(0.0, 20.0))
+    tester.tgt_power(priority=1, power=20.0, bounds=(20.0, 50.0), expected=45.0)
+    tester.bounds(priority=1, expected_power=45.0, expected_bounds=(0.0, 25.0))
 
-    tester.tgt_power(priority=1, power=10.0, bounds=(5.0, 10.0), expected=40.0)
-    tester.bounds(priority=0, expected_power=40.0, expected_bounds=(-5.0, 0.0))
+    tester.tgt_power(priority=1, power=10.0, bounds=(5.0, 10.0), expected=35.0)
+    tester.bounds(priority=0, expected_power=35.0, expected_bounds=(-5.0, 0.0))
 
     tester.tgt_power(priority=2, power=-10.0, bounds=(-10.0, 50.0), expected=0.0)
     tester.bounds(priority=1, expected_power=0.0, expected_bounds=(0.0, 60.0))
@@ -298,10 +324,10 @@ async def test_matryoshka_with_excl_2() -> None:
     tester.bounds(priority=0, expected_power=30.0, expected_bounds=(0.0, 50.0))
 
     tester.tgt_power(priority=1, power=-10.0, bounds=(-10.0, 20.0), expected=-10.0)
-    tester.bounds(priority=0, expected_power=-10.0, expected_bounds=(0.0, 10.0))
+    tester.bounds(priority=0, expected_power=-10.0, expected_bounds=(0.0, 20.0))
 
-    tester.tgt_power(priority=1, power=-10.0, bounds=(-10.0, -5.0), expected=30.0)
-    tester.bounds(priority=0, expected_power=30.0, expected_bounds=(0.0, 60.0))
+    tester.tgt_power(priority=1, power=-10.0, bounds=(-10.0, -5.0), expected=None)
+    tester.bounds(priority=0, expected_power=-10.0, expected_bounds=(0.0, 0.0))
 
     tester.tgt_power(priority=2, power=-10.0, bounds=(-200.0, -5.0), expected=-20.0)
     tester.bounds(priority=1, expected_power=-20.0, expected_bounds=(-190.0, 5.0))
@@ -336,39 +362,39 @@ async def test_matryoshka_with_excl_3() -> None:
     tester.tgt_power(priority=2, power=-10.0, bounds=(-200.0, 200.0), expected=-30.0)
     tester.tgt_power(priority=2, power=0.0, bounds=(-200.0, 200.0), expected=0.0)
     tester.tgt_power(priority=3, power=20.0, bounds=(-200.0, 200.0), expected=30.0)
-    tester.tgt_power(priority=1, power=-20.0, bounds=(-200.0, 200.0), expected=None)
+    tester.tgt_power(priority=1, power=-20.0, bounds=(-200.0, 200.0), expected=0.0)
     tester.tgt_power(priority=3, power=None, bounds=(-200.0, 200.0), expected=-30.0)
     tester.tgt_power(priority=1, power=None, bounds=(-200.0, 200.0), expected=0.0)
 
     tester.tgt_power(priority=2, power=25.0, bounds=(25.0, 50.0), expected=30.0)
     tester.bounds(priority=2, expected_power=30.0, expected_bounds=(-200.0, 200.0))
-    tester.bounds(priority=1, expected_power=30.0, expected_bounds=(0.0, 20.0))
+    tester.bounds(priority=1, expected_power=30.0, expected_bounds=(0.0, 25.0))
 
-    tester.tgt_power(priority=1, power=20.0, bounds=(20.0, 50.0), expected=50.0)
-    tester.bounds(priority=1, expected_power=50.0, expected_bounds=(0.0, 20.0))
+    tester.tgt_power(priority=1, power=20.0, bounds=(20.0, 50.0), expected=45.0)
+    tester.bounds(priority=1, expected_power=45.0, expected_bounds=(0.0, 25.0))
 
-    tester.tgt_power(priority=1, power=10.0, bounds=(5.0, 10.0), expected=40.0)
-    tester.bounds(priority=0, expected_power=40.0, expected_bounds=(-5.0, 0.0))
+    tester.tgt_power(priority=1, power=10.0, bounds=(5.0, 10.0), expected=35.0)
+    tester.bounds(priority=0, expected_power=35.0, expected_bounds=(-5.0, 0.0))
 
-    tester.tgt_power(priority=2, power=-10.0, bounds=(-10.0, 50.0), expected=None)
-    tester.bounds(priority=1, expected_power=40.0, expected_bounds=(0.0, 20.0))
-    tester.bounds(priority=0, expected_power=40.0, expected_bounds=(-5.0, 0.0))
+    tester.tgt_power(priority=2, power=-10.0, bounds=(-10.0, 50.0), expected=30.0)
+    tester.bounds(priority=1, expected_power=30.0, expected_bounds=(0.0, 60.0))
+    tester.bounds(priority=0, expected_power=30.0, expected_bounds=(-5.0, 0.0))
 
-    tester.tgt_power(priority=1, power=40.0, bounds=(-10.0, 50.0), expected=50.0)
-    tester.bounds(priority=0, expected_power=50.0, expected_bounds=(-20.0, 0.0))
+    tester.tgt_power(priority=1, power=40.0, bounds=(-10.0, 50.0), expected=None)
+    tester.bounds(priority=0, expected_power=30.0, expected_bounds=(-40.0, 10.0))
 
-    tester.tgt_power(priority=1, power=-10.0, bounds=(-10.0, 20.0), expected=30.0)
+    tester.tgt_power(priority=1, power=-10.0, bounds=(-10.0, 20.0), expected=None)
     tester.bounds(priority=0, expected_power=30.0, expected_bounds=(0.0, 20.0))
 
-    tester.tgt_power(priority=2, power=-10.0, bounds=(-200.0, -5.0), expected=-40.0)
-    tester.bounds(priority=1, expected_power=-40.0, expected_bounds=(-170.0, 0.0))
-    tester.bounds(priority=0, expected_power=-40.0, expected_bounds=(0.0, 10.0))
+    tester.tgt_power(priority=2, power=-10.0, bounds=(-200.0, -5.0), expected=-30.0)
+    tester.bounds(priority=1, expected_power=-30.0, expected_bounds=(-190.0, 5.0))
+    tester.bounds(priority=0, expected_power=-30.0, expected_bounds=(0.0, 15.0))
 
     tester.tgt_power(priority=1, power=-10.0, bounds=(-100.0, -5.0), expected=None)
-    tester.bounds(priority=0, expected_power=-40.0, expected_bounds=(-90.0, 5.0))
+    tester.bounds(priority=0, expected_power=-30.0, expected_bounds=(-90.0, 5.0))
 
-    tester.tgt_power(priority=1, power=-40.0, bounds=(-100.0, -35.0), expected=-70.0)
-    tester.bounds(priority=0, expected_power=-70.0, expected_bounds=(-60.0, 5.0))
+    tester.tgt_power(priority=1, power=-40.0, bounds=(-100.0, -35.0), expected=-50.0)
+    tester.bounds(priority=0, expected_power=-50.0, expected_bounds=(-60.0, 5.0))
 
 
 async def test_matryoshka_drop_old_proposals() -> None: