Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion RELEASE_NOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,4 +52,4 @@

## Bug Fixes

<!-- Here goes notable bug fixes that are worth a special mention or explanation -->
- 0W power requests are now not adjusted to exclusion bounds by the `PowerManager` and `PowerDistributor`, and are sent over to the microgrid API directly.
2 changes: 1 addition & 1 deletion src/frequenz/sdk/actor/_power_managing/_bounds.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ def clamp_to_bounds( # pylint: disable=too-many-return-statements

# If the given value is within the exclusion bounds and the exclusion bounds are
# within the given bounds, clamp the given value to the closest exclusion bound.
if exclusion_bounds is not None:
if exclusion_bounds is not None and not value.isclose(Power.zero()):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the power is close to zero, shouldn't we use Power.zero() instead of value? What happens if the API receives something that is close to zero but not exactly zero?

I understand in general we need to compare using is close comparison because of floats work, but if we treat zero specially we might run into problems too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The adjusting to zero is done in the PowerDistributor. In the PowerManager, we do this check just to decide if the value needs should be clamped to the exclusion bounds or not.

if exclusion_bounds.lower < value < exclusion_bounds.upper:
return exclusion_bounds.lower, exclusion_bounds.upper

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -685,7 +685,16 @@ def distribute_power(
Returns:
Distribution result
"""
if power >= 0.0:
if is_close_to_zero(power):
return DistributionResult(
distribution={
inverter.component_id: 0.0
for _, inverters in components
for inverter in inverters
},
remaining_power=0.0,
)
Comment on lines +688 to +696
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case, if I understand correctly, you are setting it to zero if it is close to zero, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup, this is the value that gets sent to the microgrid API.

if power > 0.0:
return self._distribute_consume_power(power, components)
return self._distribute_supply_power(power, components)

Expand Down
7 changes: 7 additions & 0 deletions tests/actor/_power_managing/test_matryoshka.py
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,13 @@ async def test_matryoshka_with_excl_3() -> None:
)

tester = StatefulTester(batteries, system_bounds)
tester.tgt_power(priority=2, power=10.0, bounds=(None, None), expected=30.0)
tester.tgt_power(priority=2, power=-10.0, bounds=(None, None), expected=-30.0)
tester.tgt_power(priority=2, power=0.0, bounds=(None, None), expected=0.0)
tester.tgt_power(priority=3, power=20.0, bounds=(None, None), expected=None)
tester.tgt_power(priority=1, power=-20.0, bounds=(None, None), expected=-30.0)
tester.tgt_power(priority=3, power=None, bounds=(None, None), expected=None)
tester.tgt_power(priority=1, power=None, bounds=(None, None), 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))
Expand Down
2 changes: 1 addition & 1 deletion tests/actor/_power_managing/test_report.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ def test_adjust_to_bounds() -> None:
inclusion_bounds=(-200.0, 200.0),
exclusion_bounds=(-30.0, 30.0),
)
tester.case(0.0, -30.0, 30.0)
tester.case(0.0, 0.0, 0.0)
tester.case(-210.0, -200.0, None)
tester.case(220.0, None, 200.0)
tester.case(-20.0, -30.0, 30.0)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1039,6 +1039,11 @@ def test_scenario_1(self) -> None:

algorithm = BatteryDistributionAlgorithm()

self.assert_result(
algorithm.distribute_power(0, components),
DistributionResult({1: 0, 3: 0, 5: 0}, remaining_power=0.0),
)

self.assert_result(
algorithm.distribute_power(-300, components),
DistributionResult({1: -100, 3: -100, 5: -100}, remaining_power=0.0),
Expand Down
120 changes: 117 additions & 3 deletions tests/timeseries/_battery_pool/test_battery_pool_control_methods.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,11 @@ async def _patch_battery_pool_status(
)
)

async def _init_data_for_batteries(self, mocks: Mocks) -> None:
async def _init_data_for_batteries(
self, mocks: Mocks, *, exclusion_bounds: tuple[float, float] | None = None
) -> None:
excl_lower = exclusion_bounds[0] if exclusion_bounds else 0.0
excl_upper = exclusion_bounds[1] if exclusion_bounds else 0.0
now = datetime.now(tz=timezone.utc)
for battery_id in mocks.microgrid.battery_ids:
mocks.streamer.start_streaming(
Expand All @@ -119,8 +123,8 @@ async def _init_data_for_batteries(self, mocks: Mocks) -> None:
soc=50.0,
soc_lower_bound=10.0,
soc_upper_bound=90.0,
power_exclusion_lower_bound=0.0,
power_exclusion_upper_bound=0.0,
power_exclusion_lower_bound=excl_lower,
power_exclusion_upper_bound=excl_upper,
power_inclusion_lower_bound=-1000.0,
power_inclusion_upper_bound=1000.0,
capacity=2000.0,
Expand Down Expand Up @@ -368,3 +372,113 @@ async def test_case_3(self, mocks: Mocks, mocker: MockerFixture) -> None:
assert sorted(set_power.call_args_list) == [
mocker.call(inv_id, 0.0) for inv_id in mocks.microgrid.battery_inverter_ids
]

async def test_case_4(self, mocks: Mocks, mocker: MockerFixture) -> None:
"""Test case 4.

- single battery pool with all batteries.
- all batteries are working, but have exclusion bounds.
"""
set_power = typing.cast(
AsyncMock, microgrid.connection_manager.get().api_client.set_power
)
await self._patch_battery_pool_status(mocks, mocker)
await self._init_data_for_batteries(mocks, exclusion_bounds=(-100.0, 100.0))
await self._init_data_for_inverters(mocks)

battery_pool = microgrid.battery_pool()
bounds_rx = battery_pool.power_status.new_receiver()

self._assert_report(
await bounds_rx.receive(), power=None, lower=-4000.0, upper=4000.0
)

await battery_pool.propose_power(Power.from_watts(1000.0))

self._assert_report(
await bounds_rx.receive(), power=1000.0, lower=-4000.0, upper=4000.0
)
assert set_power.call_count == 4
assert sorted(set_power.call_args_list) == [
mocker.call(inv_id, 250.0)
for inv_id in mocks.microgrid.battery_inverter_ids
]
self._assert_report(
await bounds_rx.receive(),
power=1000.0,
lower=-4000.0,
upper=4000.0,
expected_result_pred=lambda result: isinstance(
result, power_distributing.Success
),
)

set_power.reset_mock()

# Non-zero power but within the exclusion bounds should get adjusted to nearest
# available power.
await battery_pool.propose_power(Power.from_watts(50.0))

self._assert_report(
await bounds_rx.receive(), power=400.0, lower=-4000.0, upper=4000.0
)
assert set_power.call_count == 4
assert sorted(set_power.call_args_list) == [
mocker.call(inv_id, 100.0)
for inv_id in mocks.microgrid.battery_inverter_ids
]
self._assert_report(
await bounds_rx.receive(),
power=400.0,
lower=-4000.0,
upper=4000.0,
expected_result_pred=lambda result: isinstance(
result, power_distributing.Success
),
)

set_power.reset_mock()

# Zero power should be allowed, even if there are exclusion bounds.
await battery_pool.propose_power(Power.from_watts(0.0))

self._assert_report(
await bounds_rx.receive(), power=0.0, lower=-4000.0, upper=4000.0
)
assert set_power.call_count == 4
assert sorted(set_power.call_args_list) == [
mocker.call(inv_id, 0.0) for inv_id in mocks.microgrid.battery_inverter_ids
]
self._assert_report(
await bounds_rx.receive(),
power=0.0,
lower=-4000.0,
upper=4000.0,
expected_result_pred=lambda result: isinstance(
result, power_distributing.Success
),
)

set_power.reset_mock()

# Non-zero power but within the exclusion bounds should get adjusted to nearest
# available power.
await battery_pool.propose_power(Power.from_watts(-150.0))

self._assert_report(
await bounds_rx.receive(), power=-400.0, lower=-4000.0, upper=4000.0
)
assert set_power.call_count == 4
assert sorted(set_power.call_args_list) == [
mocker.call(inv_id, -100.0)
for inv_id in mocks.microgrid.battery_inverter_ids
]
self._assert_report(
await bounds_rx.receive(),
power=-400.0,
lower=-4000.0,
upper=4000.0,
expected_result_pred=lambda result: isinstance(
result, power_distributing.Success
),
)