Forward zero power requests always to the microgrid API (#591)

shsms · web-flow · commit e48110ddc3ee · 2023-08-24T13:22:46.000Z
Only non-zero requests should be rejected if they fall within the
exclusion bounds.

This PR also:
- moves distribution algorithm into the power distributing actor's dir.
- remove unused methods
- rename `OutOfBound` -&gt; `OutOfBounds`
- Improve power distribution algorithm docstrings.
diff --git a/RELEASE_NOTES.md b/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 -->
+- The `frequenz.sdk.power` package contained the power distribution algorithm, which is for internal use in the sdk, and is no longer part of the public API.
+
+- `PowerDistributingActor`'s result type `OutOfBound` has been renamed to `OutOfBounds`, and its member variable `bound` has been renamed to `bounds`.
 
 ## New Features
 
@@ -25,3 +27,4 @@
 - 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`
+- Zero value requests received by the `PowerDistributingActor` will now always be accepted, even when there are non-zero exclusion bounds.
diff --git a/benchmarks/power_distribution/power_distributor.py b/benchmarks/power_distribution/power_distributor.py
@@ -17,7 +17,7 @@
 from frequenz.sdk.actor.power_distributing import (
     BatteryStatus,
     Error,
-    OutOfBound,
+    OutOfBounds,
     PartialFailure,
     PowerDistributingActor,
     Request,
@@ -75,7 +75,7 @@ def parse_result(result: List[List[Result]]) -> Dict[str, float]:
         Error: 0,
         Success: 0,
         PartialFailure: 0,
-        OutOfBound: 0,
+        OutOfBounds: 0,
     }
 
     for result_list in result:
@@ -86,7 +86,7 @@ def parse_result(result: List[List[Result]]) -> Dict[str, float]:
         "success_num": result_counts[Success],
         "failed_num": result_counts[PartialFailure],
         "error_num": result_counts[Error],
-        "out_of_bound": result_counts[OutOfBound],
+        "out_of_bounds": result_counts[OutOfBounds],
     }
 
 
diff --git a/src/frequenz/sdk/actor/power_distributing/__init__.py b/src/frequenz/sdk/actor/power_distributing/__init__.py
@@ -13,15 +13,15 @@
 from ._battery_pool_status import BatteryStatus
 from .power_distributing import PowerDistributingActor
 from .request import Request
-from .result import Error, OutOfBound, PartialFailure, Result, Success
+from .result import Error, OutOfBounds, PartialFailure, Result, Success
 
 __all__ = [
     "PowerDistributingActor",
     "Request",
     "Result",
     "Error",
     "Success",
-    "OutOfBound",
+    "OutOfBounds",
     "PartialFailure",
     "BatteryStatus",
 ]
diff --git a/src/frequenz/sdk/actor/power_distributing/_distribution_algorithm/__init__.py b/src/frequenz/sdk/actor/power_distributing/_distribution_algorithm/__init__.py
diff --git a/src/frequenz/sdk/actor/power_distributing/_distribution_algorithm/_distribution_algorithm.py b/src/frequenz/sdk/actor/power_distributing/_distribution_algorithm/_distribution_algorithm.py
@@ -10,7 +10,7 @@
 
 from frequenz.sdk._internal._math import is_close_to_zero
 
-from ..microgrid.component import BatteryData, InverterData
+from ....microgrid.component import BatteryData, InverterData
 
 _logger = logging.getLogger(__name__)
 
@@ -147,39 +147,16 @@ def __init__(self, distributor_exponent: float = 1) -> None:
             * `Bat1.available_soc = 10`, `Bat2.available_soc = 30`
             * `Bat1.available_soc / Bat2.available_soc = 3`
 
-            We need to distribute 8000W.
+            A request power of 8000W will be distributed as follows, for different
+            values of `distribution_exponent`:
 
-            If `distribution_exponent` is:
+            | distribution_exponent | Bat1 | Bat2 |
+            |-----------------------|------|------|
+            | 0                     | 4000 | 4000 |
+            | 1                     | 2000 | 6000 |
+            | 2                     | 800  | 7200 |
+            | 3                     | 285  | 7715 |
 
-            * `0`: distribution for each battery will be the equal.
-              ```python
-              BAT1_DISTRIBUTION = 4000
-              BAT2_DISTRIBUTION = 4000
-              ```
-
-            * `1`: then `Bat2` will have 3x more power assigned then `Bat1`.
-              ```python
-              # 10 * x + 30 * x = 8000
-              X = 200
-              BAT1_DISTRIBUTION = 2000
-              BAT2_DISTRIBUTION = 6000
-              ```
-
-            * `2`: then `Bat2` will have 9x more power assigned then `Bat1`.
-              ```python
-              # 10^2 * x + 30^2 * x = 8000
-              X = 80
-              BAT1_DISTRIBUTION = 800
-              BAT2_DISTRIBUTION = 7200
-              ```
-
-            * `3`: then `Bat2` will have 27x more power assigned then `Bat1`.
-              ```python
-              # 10^3 * x + 30^3 * x = 8000
-              X = 0.285714286
-              BAT1_DISTRIBUTION = 285
-              BAT2_DISTRIBUTION = 7715
-              ```
 
             # Example 2
 
@@ -189,39 +166,15 @@ def __init__(self, distributor_exponent: float = 1) -> None:
             * `Bat1.available_soc = 30`, `Bat2.available_soc = 60`
             * `Bat1.available_soc / Bat2.available_soc = 2`
 
-            We need to distribute 900W.
-
-            If `distribution_exponent` is:
+            A request power of 900W will be distributed as follows, for different
+            values of `distribution_exponent`.
 
-            * `0`: distribution for each battery will be the same.
-              ```python
-              BAT1_DISTRIBUTION = 4500
-              BAT2_DISTRIBUTION = 450
-              ```
-
-            * `1`: then `Bat2` will have 2x more power assigned then `Bat1`.
-              ```python
-              # 30 * x + 60 * x = 900
-              X = 100
-              BAT1_DISTRIBUTION = 300
-              BAT2_DISTRIBUTION = 600
-              ```
-
-            * `2`: then `Bat2` will have 4x more power assigned then `Bat1`.
-              ```python
-              # 30^2 * x + 60^2 * x = 900
-              X = 0.2
-              BAT1_DISTRIBUTION = 180
-              BAT2_DISTRIBUTION = 720
-              ```
-
-            * `3`: then `Bat2` will have 8x more power assigned then `Bat1`.
-              ```python
-              # 30^3 * x + 60^3 * x = 900
-              X = 0.003703704
-              BAT1_DISTRIBUTION = 100
-              BAT2_DISTRIBUTION = 800
-              ```
+            | distribution_exponent | Bat1 | Bat2 |
+            |-----------------------|------|------|
+            | 0                     | 450  | 450  |
+            | 1                     | 300  | 600  |
+            | 2                     | 180  | 720  |
+            | 3                     | 100  | 800  |
 
             # Example 3
 
@@ -230,26 +183,19 @@ def __init__(self, distributor_exponent: float = 1) -> None:
             * `Bat1.soc = 44` and `Bat2.soc = 64`.
             * `Bat1.available_soc = 36 (80 - 44)`, `Bat2.available_soc = 16 (80 - 64)`
 
-            We need to distribute 900W.
+            A request power of 900W will be distributed as follows, for these values of
+            `distribution_exponent`:
 
             If `distribution_exponent` is:
 
-            * `0`: distribution for each battery will be the equal.
-              ```python
-              BAT1_DISTRIBUTION = 450
-              BAT2_DISTRIBUTION = 450
-              ```
-
-            * `0.5`: then `Bat2` will have 6/4x more power assigned then `Bat1`.
-              ```python
-              # sqrt(36) * x + sqrt(16) * x = 900
-              X = 100
-              BAT1_DISTRIBUTION = 600
-              BAT2_DISTRIBUTION = 400
-              ```
+            | distribution_exponent | Bat1 | Bat2 |
+            |-----------------------|------|------|
+            | 0                     | 450  | 450  |
+            | 0.5                   | 600  | 400  |
 
         Raises:
             ValueError: If distributor_exponent < 0
+
         """
         super().__init__()
 
@@ -401,6 +347,8 @@ def _distribute_power(  # pylint: disable=too-many-arguments
 
         for inverter_id, deficit in deficits.items():
             while not is_close_to_zero(deficit) and deficit < 0.0:
+                if not excess_reserved:
+                    break
                 take_from = max(excess_reserved.items(), key=lambda item: item[1])
                 if is_close_to_zero(take_from[1]) or take_from[1] < 0.0:
                     break
diff --git a/src/frequenz/sdk/actor/power_distributing/power_distributing.py b/src/frequenz/sdk/actor/power_distributing/power_distributing.py
@@ -26,6 +26,7 @@
 import grpc
 from frequenz.channels import Peekable, Receiver, Sender
 
+from ..._internal._math import is_close_to_zero
 from ...actor import ChannelRegistry
 from ...actor._decorator import actor
 from ...microgrid import ComponentGraph, connection_manager
@@ -36,10 +37,14 @@
     ComponentCategory,
     InverterData,
 )
-from ...power import DistributionAlgorithm, DistributionResult, InvBatPair
 from ._battery_pool_status import BatteryPoolStatus, BatteryStatus
+from ._distribution_algorithm import (
+    DistributionAlgorithm,
+    DistributionResult,
+    InvBatPair,
+)
 from .request import Request
-from .result import Error, OutOfBound, PartialFailure, PowerBounds, Result, Success
+from .result import Error, OutOfBounds, PartialFailure, PowerBounds, Result, Success
 
 _logger = logging.getLogger(__name__)
 
@@ -269,56 +274,6 @@ def _get_bounds(
             ),
         )
 
-    def _get_upper_bound(self, batteries: abc.Set[int], include_broken: bool) -> float:
-        """Get total upper bound of power to be set for given batteries.
-
-        Note, output of that function doesn't guarantee that this bound will be
-        the same when the request is processed.
-
-        Args:
-            batteries: List of batteries
-            include_broken: whether all batteries in the batteries set in the
-                request must be used regardless the status.
-
-        Returns:
-            Upper bound for `set_power` operation.
-        """
-        pairs_data: List[InvBatPair] = self._get_components_data(
-            batteries, include_broken
-        )
-        return sum(
-            min(
-                battery.power_inclusion_upper_bound,
-                inverter.active_power_inclusion_upper_bound,
-            )
-            for battery, inverter in pairs_data
-        )
-
-    def _get_lower_bound(self, batteries: abc.Set[int], include_broken: bool) -> float:
-        """Get total lower bound of power to be set for given batteries.
-
-        Note, output of that function doesn't guarantee that this bound will be
-        the same when the request is processed.
-
-        Args:
-            batteries: List of batteries
-            include_broken: whether all batteries in the batteries set in the
-                request must be used regardless the status.
-
-        Returns:
-            Lower bound for `set_power` operation.
-        """
-        pairs_data: List[InvBatPair] = self._get_components_data(
-            batteries, include_broken
-        )
-        return sum(
-            max(
-                battery.power_inclusion_lower_bound,
-                inverter.active_power_inclusion_lower_bound,
-            )
-            for battery, inverter in pairs_data
-        )
-
     async def _send_result(self, namespace: str, result: Result) -> None:
         """Send result to the user.
 
@@ -375,6 +330,7 @@ async def run(self) -> None:
             try:
                 distribution = self._get_power_distribution(request, pairs_data)
             except ValueError as err:
+                _logger.exception("Couldn't distribute power")
                 error_msg = f"Couldn't distribute power, error: {str(err)}"
                 await self._send_result(
                     request.namespace, Error(request=request, msg=str(error_msg))
@@ -521,14 +477,20 @@ def _check_request(
                 return Error(request=request, msg=msg)
 
         bounds = self._get_bounds(pairs_data)
+
+        # Zero power requests are always forwarded to the microgrid API, even if they
+        # are outside the exclusion bounds.
+        if is_close_to_zero(request.power):
+            return None
+
         if request.adjust_power:
             # Automatic power adjustments can only bring down the requested power down
             # to the inclusion bounds.
             #
             # If the requested power is in the exclusion bounds, it is NOT possible to
             # increase it so that it is outside the exclusion bounds.
             if bounds.exclusion_lower < request.power < bounds.exclusion_upper:
-                return OutOfBound(request=request, bound=bounds)
+                return OutOfBounds(request=request, bounds=bounds)
         else:
             in_lower_range = (
                 bounds.inclusion_lower <= request.power <= bounds.exclusion_lower
@@ -537,7 +499,7 @@ def _check_request(
                 bounds.exclusion_upper <= request.power <= bounds.inclusion_upper
             )
             if not (in_lower_range or in_upper_range):
-                return OutOfBound(request=request, bound=bounds)
+                return OutOfBounds(request=request, bounds=bounds)
 
         return None
 
diff --git a/src/frequenz/sdk/actor/power_distributing/result.py b/src/frequenz/sdk/actor/power_distributing/result.py
@@ -88,14 +88,14 @@ class PowerBounds:
 
 
 @dataclasses.dataclass
-class OutOfBound(Result):
+class OutOfBounds(Result):
     """Result returned when the power was not set because it was out of bounds.
 
     This result happens when the originating request was done with
     `adjust_power = False` and the requested power is not within the batteries bounds.
     """
 
-    bound: PowerBounds
+    bounds: PowerBounds
     """The power bounds for the requested batteries.
 
     If the requested power negative, then this value is the lower bound.
diff --git a/tests/actor/power_distributing/__init__.py b/tests/actor/power_distributing/__init__.py
@@ -1,4 +1,4 @@
 # License: MIT
 # Copyright © 2022 Frequenz Energy-as-a-Service GmbH
 
-"""Test power distribution module."""
+"""Tests for the power distributing actor and algorithm."""
diff --git a/tests/actor/power_distributing/test_distribution_algorithm.py b/tests/actor/power_distributing/test_distribution_algorithm.py
@@ -10,11 +10,15 @@
 
 from pytest import approx, raises
 
+from frequenz.sdk.actor.power_distributing._distribution_algorithm import (
+    DistributionAlgorithm,
+    DistributionResult,
+    InvBatPair,
+)
 from frequenz.sdk.actor.power_distributing.result import PowerBounds
 from frequenz.sdk.microgrid.component import BatteryData, InverterData
-from frequenz.sdk.power import DistributionAlgorithm, DistributionResult, InvBatPair
 
-from ..utils.component_data_wrapper import BatteryDataWrapper, InverterDataWrapper
+from ...utils.component_data_wrapper import BatteryDataWrapper, InverterDataWrapper
 
 
 @dataclass
diff --git a/tests/actor/power_distributing/test_power_distributing.py b/tests/actor/power_distributing/test_power_distributing.py
