Use the additive ShiftingMatryoshka algorithm only for batteries
#1202
Conversation
And revert back to plain Matryoshka for PV and EV chargers. Signed-off-by: Sahas Subramanian <[email protected]>
Signed-off-by: Sahas Subramanian <[email protected]>
shsms
requested review from
daniel-zullo-frequenz
and removed request for
a team
github-actions
bot
added
part:docs
There was a problem hiding this comment.
Pull Request Overview
This PR reverses the previously introduced change where the additive ShiftingMatryoshka algorithm was applied universally by restoring the plain Matryoshka algorithm for PV and EV chargers while retaining the shifting algorithm exclusively for batteries.
- Modifies the PowerWrapper init to accept a power management algorithm
- Updates the PowerManagingActor to receive the algorithm parameter instead of using a default
- Adjusts the instantiation of PowerWrapper in the data pipeline and updates the release notes accordingly
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/frequenz/sdk/microgrid/_power_wrapper.py | Introduces a new parameter "power_manager_algorithm" in the constructor and passes it to the actor. |
| src/frequenz/sdk/microgrid/_power_managing/_power_managing_actor.py | Removes the default algorithm and accepts the algorithm parameter explicitly. |
| src/frequenz/sdk/microgrid/_data_pipeline.py | Updates algorithm selection: batteries use ShiftingMatryoshka; PV and EV use Matryoshka. |
| RELEASE_NOTES.md | Updates the release notes to reflect that the algorithm selection has been reverted for PV and EV chargers. |
Comments suppressed due to low confidence (1)
src/frequenz/sdk/microgrid/_power_managing/_power_managing_actor.py:42
- [nitpick] Consider renaming the parameter from 'algorithm' to 'power_manager_algorithm' for consistency with the PowerWrapper init signature and to reduce potential confusion.
algorithm: Algorithm,
github-project-automation
bot
moved this from To do
to Review approved
in Python SDK Roadmap
github-project-automation
bot
moved this from Review approved
to Done
in Python SDK Roadmap
| self._ev_power_wrapper = PowerWrapper( | ||
| self._channel_registry, | ||
| api_power_request_timeout=api_power_request_timeout, | ||
| power_manager_algorithm=Algorithm.MATRYOSHKA, | ||
| component_category=ComponentCategory.EV_CHARGER, | ||
| ) | ||
| self._pv_power_wrapper = PowerWrapper( | ||
| self._channel_registry, | ||
| api_power_request_timeout=api_power_request_timeout, | ||
| power_manager_algorithm=Algorithm.MATRYOSHKA, | ||
| component_category=ComponentCategory.INVERTER, | ||
| component_type=InverterType.SOLAR, | ||
| ) |
There was a problem hiding this comment.
If I remember correctly, original Matryoshka algorithm had something like operating point adjustment.
How it works now?
There was a problem hiding this comment.
That was like a partition, so all actors that were in the operating point group would shift the other actors or something like that. I think now is like the operating point group is always empty. But maybe @shsms can check I'm not mistaken.
There was a problem hiding this comment.
That was mainly for battery use cases and is gone now. We only have a single matryoshka instance for PV and EV chargers.
There was a problem hiding this comment.
OK, thanks a lot! :)
3 participants
And revert back to plain Matryoshka for PV and EV chargers.