Skip to content

Conversation

@Marenz
Copy link
Contributor

@Marenz Marenz commented Oct 18, 2023

This specifically focuses on the power forumla for the battery pool.

  • test with no battery
  • test with no inverter
  • test with requests that don't specify all batteries behind the inverters

refs #501

@github-actions github-actions bot added part:tests Affects the unit, integration and performance (benchmarks) tests part:data-pipeline Affects the data pipeline labels Oct 18, 2023
@Marenz Marenz force-pushed the nminverter branch 2 times, most recently from b4ae13c to f7d4078 Compare October 18, 2023 15:20
@llucax llucax added this to the v1.0.0-rc5 milestone Jan 29, 2024
@Marenz Marenz force-pushed the nminverter branch 2 times, most recently from 0e714d9 to 751463d Compare January 30, 2024 14:38
@github-actions github-actions bot added the part:docs Affects the documentation label Jan 30, 2024
@Marenz Marenz marked this pull request as ready for review January 30, 2024 16:49
@Marenz Marenz requested a review from a team as a code owner January 30, 2024 16:49
@Marenz Marenz requested a review from shsms January 30, 2024 16:49
component_graph.predecessors(bat_id),
)
)
for bat_id in component_ids
Copy link
Contributor

Choose a reason for hiding this comment

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

in a setup with 1 inverter and 2 batteries, there will be 2 frozensets with the same inverter. that would lead to 2x power reporting, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. Added a test to catch this and fixed it, too

Comment on lines +97 to +99
for idx, comp in enumerate(
inverter for inverters in battery_inverters for inverter in inverters
):
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe the inverter ids can be collected into a set before enumerating, to deduplicate?

Suggested change
for idx, comp in enumerate(
inverter for inverters in battery_inverters for inverter in inverters
):
for idx, comp in enumerate(
set(inverter for inverters in battery_inverters for inverter in inverters)
):

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I made battery_inverters directly to a frozenset instead

component_graph = connection_manager.get().component_graph

battery_inverters = list(
battery_inverters = frozenset(
Copy link
Contributor

Choose a reason for hiding this comment

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

was this supposed to go into a different commit?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed >:|

@shsms shsms changed the title More updates for n:m support More updates for n:m support - battery pool's power formula Feb 22, 2024
@Marenz Marenz added this pull request to the merge queue Feb 22, 2024
Merged via the queue into frequenz-floss:v1.x.x with commit b0cf5e2 Feb 22, 2024
@Marenz Marenz deleted the nminverter branch February 22, 2024 13:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

part:data-pipeline Affects the data pipeline part:docs Affects the documentation part:tests Affects the unit, integration and performance (benchmarks) tests

Projects

Development

Successfully merging this pull request may close these issues.

3 participants