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: 2 additions & 0 deletions RELEASE_NOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
- Allow multiplying and dividing any `Quantity` by a `float`. This just scales the `Quantity` value.
- Allow dividing any `Quantity` by another quaintity of the same type. This just returns a ration between both quantities.

- The battery pool `power` method now supports scenarios where one or more inverters can have multiple batteries connected to it and one or more batteries can have multiple inverters connected to it.

## Bug Fixes

- Fix grid current formula generator to add the operator `+` to the engine only when the component category is handled.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
from ._formula_generator import (
NON_EXISTING_COMPONENT_ID,
ComponentNotFound,
FormulaGenerationError,
FormulaGenerator,
)

Expand Down Expand Up @@ -40,6 +41,8 @@ def generate(
they don't have an inverter as a predecessor.
FormulaGenerationError: If a battery has a non-inverter predecessor
in the component graph.
FormulaGenerationError: If not all batteries behind a set of inverters
have been requested.
"""
builder = self._get_builder(
"battery-power", ComponentMetricId.ACTIVE_POWER, Power.from_watts
Expand All @@ -61,16 +64,39 @@ def generate(

component_graph = connection_manager.get().component_graph

battery_inverters = list(
next(iter(component_graph.predecessors(bat_id))) for bat_id in component_ids
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 >:|

frozenset(
filter(
component_graph.is_battery_inverter,
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

)

if len(component_ids) != len(battery_inverters):
if not all(battery_inverters):
raise ComponentNotFound(
"Can't find inverters for all batteries from the component graph."
"All batteries must have at least one inverter as a predecessor."
)

all_connected_batteries = set()
for inverters in battery_inverters:
for inverter in inverters:
all_connected_batteries.update(
component_graph.successors(inverter.component_id)
)

if len(all_connected_batteries) != len(component_ids):
raise FormulaGenerationError(
"All batteries behind a set of inverters must be requested."
)

for idx, comp in enumerate(battery_inverters):
builder.push_oper("(")
builder.push_oper("(")
# Iterate over the flattened list of inverters
for idx, comp in enumerate(
inverter for inverters in battery_inverters for inverter in inverters
):
Comment on lines +97 to +99
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

if idx > 0:
builder.push_oper("+")
builder.push_component_metric(comp.component_id, nones_are_zeros=True)
Expand Down
39 changes: 2 additions & 37 deletions tests/actor/power_distributing/test_power_distributing.py
Original file line number Diff line number Diff line change
Expand Up @@ -537,21 +537,7 @@ async def test_battery_two_inverters(self, mocker: MockerFixture) -> None:
graph = gen.to_graph(
(
ComponentCategory.METER,
[
(
ComponentCategory.METER,
[
(
ComponentCategory.INVERTER,
bat_component,
),
(
ComponentCategory.INVERTER,
bat_component,
),
],
),
],
[gen.battery_with_inverter(bat_component, 2)],
)
)

Expand Down Expand Up @@ -600,28 +586,7 @@ async def test_two_batteries_three_inverters(self, mocker: MockerFixture) -> Non
batteries = gen.components(*[ComponentCategory.BATTERY] * 2)

graph = gen.to_graph(
(
ComponentCategory.METER,
[
(
ComponentCategory.METER,
[
(
ComponentCategory.INVERTER,
[*batteries],
),
(
ComponentCategory.INVERTER,
[*batteries],
),
(
ComponentCategory.INVERTER,
[*batteries],
),
],
),
],
)
(ComponentCategory.METER, gen.batteries_with_inverter(batteries, 3))
)

async with _mocks(mocker, graph=graph) as mocks:
Expand Down
106 changes: 105 additions & 1 deletion tests/timeseries/_battery_pool/test_battery_pool.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@
)
from frequenz.sdk.timeseries._base_types import SystemBounds
from frequenz.sdk.timeseries.battery_pool import BatteryPool
from frequenz.sdk.timeseries.formula_engine._formula_generators._formula_generator import (
FormulaGenerationError,
)
from tests.utils.graph_generator import GraphGenerator

from ...timeseries.mock_microgrid import MockMicrogrid
from ...utils.component_data_streamer import MockComponentDataStreamer
Expand Down Expand Up @@ -488,10 +492,110 @@ async def run_test_battery_status_channel( # pylint: disable=too-many-arguments


async def test_battery_pool_power(mocker: MockerFixture) -> None:
"""Test `BatteryPool.{,production,consumption}_power` methods."""
"""Test `BatteryPool.power` method."""
mockgrid = MockMicrogrid(grid_meter=True, mocker=mocker)
mockgrid.add_batteries(2)
await mockgrid.start(mocker)
await _test_battery_pool_power(mockgrid)


async def test_battery_pool_power_two_inverters_per_battery(
mocker: MockerFixture,
) -> None:
"""Test power method with two inverters per battery."""
gen = GraphGenerator()
bat = gen.component(ComponentCategory.BATTERY)
mockgrid = MockMicrogrid(
graph=gen.to_graph((ComponentCategory.METER, gen.battery_with_inverter(bat, 2)))
)
await mockgrid.start(mocker)
await _test_battery_pool_power(mockgrid)


async def test_batter_pool_power_two_batteries_per_inverter(
mocker: MockerFixture,
) -> None:
"""Test power method with two batteries per inverter."""
gen = GraphGenerator()
mockgrid = MockMicrogrid(
graph=gen.to_graph(
[
ComponentCategory.METER,
(
ComponentCategory.INVERTER,
[ComponentCategory.BATTERY, ComponentCategory.BATTERY],
),
ComponentCategory.METER,
(
ComponentCategory.INVERTER,
[ComponentCategory.BATTERY, ComponentCategory.BATTERY],
),
]
)
)
await mockgrid.start(mocker)
await _test_battery_pool_power(mockgrid)


async def test_batter_pool_power_no_batteries(mocker: MockerFixture) -> None:
"""Test power method with no batteries."""
mockgrid = MockMicrogrid(
graph=GraphGenerator().to_graph(
(
ComponentCategory.METER,
[ComponentCategory.INVERTER, ComponentCategory.INVERTER],
)
)
)
await mockgrid.start(mocker)
battery_pool = microgrid.battery_pool()
power_receiver = battery_pool.power.new_receiver()

await mockgrid.mock_resampler.send_non_existing_component_value()
assert (await power_receiver.receive()).value == Power.from_watts(0)


async def test_battery_pool_power_with_no_inverters(mocker: MockerFixture) -> None:
"""Test power method with no inverters."""
mockgrid = MockMicrogrid(
graph=GraphGenerator().to_graph(
(ComponentCategory.METER, ComponentCategory.BATTERY)
)
)
await mockgrid.start(mocker)

with pytest.raises(RuntimeError):
microgrid.battery_pool()


async def test_battery_pool_power_incomplete_bat_request(mocker: MockerFixture) -> None:
"""Test power method when not all requested ids are behind the same inverter."""
gen = GraphGenerator()
bats = gen.components(
ComponentCategory.BATTERY, ComponentCategory.BATTERY, ComponentCategory.BATTERY
)

mockgrid = MockMicrogrid(
graph=gen.to_graph(
(
ComponentCategory.METER,
gen.batteries_with_inverter(bats, 2),
)
)
)
await mockgrid.start(mocker)

with pytest.raises(FormulaGenerationError):
# Request only two of the three batteries behind the inverters
battery_pool = microgrid.battery_pool(
battery_ids=set([bats[1].component_id, bats[0].component_id])
)
power_receiver = battery_pool.power.new_receiver()
await mockgrid.mock_resampler.send_bat_inverter_power([2.0])
assert (await power_receiver.receive()).value == Power.from_watts(2.0)


async def _test_battery_pool_power(mockgrid: MockMicrogrid) -> None:
async with mockgrid:
battery_pool = microgrid.battery_pool()
power_receiver = battery_pool.power.new_receiver()
Expand Down
58 changes: 57 additions & 1 deletion tests/utils/graph_generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,68 @@ def new_id(self) -> dict[ComponentCategory, int]:
self._id_increment += 1
return id_per_category

def battery_with_inverter(self, battery: Component, num_inverters: int) -> Any:
"""Add a meter and inverters to the given battery.

Args:
battery: the battery component.
num_inverters: the number of inverters to create.

Returns:
connected graph components for the given battery.
"""
assert battery.category == ComponentCategory.BATTERY
return self._battery_with_inverter(battery, num_inverters)

def batteries_with_inverter(
self, one_or_more_batteries: list[Component], num_inverters: int
) -> Any:
"""Add a meter and inverters to the given batteries.

Args:
one_or_more_batteries: the battery components.
num_inverters: the number of inverters to create.

Returns:
connected graph components for the given batteries.
"""
assert all(
b.category == ComponentCategory.BATTERY for b in one_or_more_batteries
)
return self._battery_with_inverter(one_or_more_batteries, num_inverters)

def _battery_with_inverter(
self, one_or_more_batteries: Component | list[Component], num_inverters: int
) -> Any:
"""Add a meter and inverters to the given battery or batteries.

Args:
one_or_more_batteries: the battery component or components.
num_inverters: the number of inverters to create.

Returns:
connected graph components for the given battery or batteries.
"""
return (
ComponentCategory.METER,
[
(
self.component(ComponentCategory.INVERTER, InverterType.BATTERY),
one_or_more_batteries,
)
for _ in range(num_inverters)
],
)

@overload
def component(self, other: Component) -> Component:
def component(
self, other: Component, comp_type: ComponentType | None = None
) -> Component:
"""Just return the given component.

Args:
other: the component to return.
comp_type: the component type to set, ignored

Returns:
the given component.
Expand Down