Skip to content

Conversation

@shsms
Copy link
Contributor

@shsms shsms commented May 2, 2025

Previously, when all proposals for a set of components were withdrawn with pool.propose_power(None), the power manager would just stop setting powers for those components, and the last set powers remain until they get reset by the API service or some other process. This is not desirable, and instead the components should get set back to their respective default powers, according to the table below. This is implemented in this PR.

component category default power
Battery 0.0
PV Minimum power (aka max production power)
EV Chargers Maximum power (aka max consumption power)

shsms added 6 commits April 29, 2025 13:47
The power manager in some cases keeps resending the bounds even when
there are no changes.  And these tests were relying on that behaviour.
This is changed now, so that the power manager's behaviour can change
as well.

Signed-off-by: Sahas Subramanian <[email protected]>
With this, the power manager algorithms basically start ignoring the
`must_send` flag and always return a target power value if available.

Signed-off-by: Sahas Subramanian <[email protected]>
@Copilot Copilot AI review requested due to automatic review settings May 2, 2025 16:51
@shsms shsms requested a review from a team as a code owner May 2, 2025 16:51
@shsms shsms requested review from llucax and removed request for a team May 2, 2025 16:51
@github-actions github-actions bot added part:tests Affects the unit, integration and performance (benchmarks) tests part:microgrid Affects the interactions with the microgrid labels May 2, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR changes the behavior of power proposals so that when all proposals for a set of components are withdrawn (i.e. when propose_power is set to None), the components are reset to their respective default powers as defined by the component category. Key changes include updating the tests for PV pools, EV charger pools, and battery pools to reflect the new default power reset behavior; passing a new default_power parameter through several power management classes; and modifying both the ShiftingMatryoshka and Matryoshka algorithms to return default power values when no valid target power can be calculated.

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tests/timeseries/_pv_pool/test_pv_pool_control_methods.py Refactored report assertion and receiver handling for default power resets in PV pools.
tests/timeseries/_ev_charger_pool/test_ev_charger_pool_control_methods.py Updated expected power values for EV chargers to reflect default power (max consumption).
tests/timeseries/_battery_pool/test_battery_pool_control_methods.py Modified battery pool tests to verify that default power (zero) is applied on proposal reset.
tests/actor/_power_managing/test_shifting_matryoshka.py and test_matryoshka.py Adjusted expected outcomes in the Matryoshka algorithm tests to match default power behavior.
src/frequenz/sdk/microgrid/_power_wrapper.py, _shifting_matryoshka.py, _matryoshka.py, _power_managing_actor.py, _base_classes.py, _data_pipeline.py Introduced and propagated the default_power parameter and logic to reset target power when proposals are withdrawn.
Comments suppressed due to low confidence (1)

src/frequenz/sdk/microgrid/_data_pipeline.py:125

  • Double-check that setting DefaultPower.MIN for PV inverters complies with the PR description (i.e. PV default power should be the minimum production power). Ensuring consistency with both the documented behavior and external documentation will help avoid confusion.
default_power=DefaultPower.MIN,

Signed-off-by: Sahas Subramanian <[email protected]>
@github-actions github-actions bot added the part:docs Affects the documentation label May 2, 2025
llucax
llucax previously approved these changes May 5, 2025
async def _recv_reports_until(
self,
bounds_rx: Receiver[EVChargerPoolReport],
check: typing.Callable[[EVChargerPoolReport], bool],
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick: check -> predicate? (oh, I see this is in tests, so nitpick³)

@github-project-automation github-project-automation bot moved this from To do to Review approved in Python SDK Roadmap May 5, 2025
@llucax
Copy link
Contributor

llucax commented May 5, 2025

One more thing, maybe this is worth mentioning in the docs too, in the part explaining the power manager and how proposing power works.

@shsms
Copy link
Contributor Author

shsms commented May 5, 2025

One more thing, maybe this is worth mentioning in the docs too, in the part explaining the power manager and how proposing power works.

True, I've added a new commit.

@shsms shsms enabled auto-merge May 5, 2025 09:06
@shsms shsms added this pull request to the merge queue May 5, 2025
Merged via the queue into frequenz-floss:v1.x.x with commit 92a268f May 5, 2025
5 checks passed
@shsms shsms deleted the power-manager-reset-power branch May 5, 2025 10:28
@github-project-automation github-project-automation bot moved this from Review approved to Done in Python SDK Roadmap May 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

part:docs Affects the documentation part:microgrid Affects the interactions with the microgrid part:tests Affects the unit, integration and performance (benchmarks) tests

Projects

Development

Successfully merging this pull request may close these issues.

2 participants