diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index a34f3f767..c3fa12dc0 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -1,15 +1,19 @@ # Frequenz Python SDK Release Notes +## Summary + + + ## Upgrading -* Update microgrid client to v0.18.3+, which fixes a problem with missing steam boilers on formula generation. -* The PV inverter manager now subtracts the power measured on unreachable PV inverters from the distribution target, so the power sent to the reachable inverters can change when some requested PV inverters are unreachable. +* Update the microgrid component graph library to v0.5.0. The per-category formulas (battery, PV, CHP, EV charger, ...) now read the component first and use the meter as the fallback. This is the opposite of the old order, so generated formula strings and power values can change. To keep the old order, pass `ComponentGraphConfig(prefer_meters_in_component_formulas=True)` to `microgrid.initialize()`. + +* Graph validation errors now use a new message format. ## New Features -* The PV inverter manager now accounts for the power measured on unreachable PV inverters when distributing power, so the reachable inverters compensate for it (matching the battery manager's behavior). +* `microgrid.initialize()` has a new keyword argument `component_graph_config`. It takes a `ComponentGraphConfig` and gives full control over how the component graph is built. For example, pass `ComponentGraphConfig(prefer_meters_in_component_formulas=True)` to make the per-category formulas read from the meter first, as in previous releases. `ComponentGraphConfig` and `FormulaOverrides` are re-exported from `frequenz.sdk.microgrid`. ## Bug Fixes -* Fixed a resource leak in the power distributor: the formulas created for unreachable batteries were never stopped, so CPU usage slowly climbed over time until the application was restarted. -* Fixed the grid reactive-power formula being recreated and leaked on every access instead of being reused from the cache. + diff --git a/pyproject.toml b/pyproject.toml index 8201fe864..90a8f2ba2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -30,7 +30,7 @@ dependencies = [ # changing the version # (plugins.mkdocstrings.handlers.python.import) "frequenz-client-microgrid >= 0.18.3, < 0.19.0", - "frequenz-microgrid-component-graph >= 0.4, < 0.5", + "frequenz-microgrid-component-graph >= 0.5.0, < 0.6", "frequenz-client-common >= 0.3.6, < 0.5.0", "frequenz-channels >= 1.6.1, < 2.0.0", "frequenz-quantities[marshmallow] >= 1.0.0, < 2.0.0", diff --git a/src/frequenz/sdk/microgrid/__init__.py b/src/frequenz/sdk/microgrid/__init__.py index a9d049bcd..3bddf8ee5 100644 --- a/src/frequenz/sdk/microgrid/__init__.py +++ b/src/frequenz/sdk/microgrid/__init__.py @@ -370,6 +370,8 @@ from datetime import timedelta +from frequenz.microgrid_component_graph import ComponentGraphConfig, FormulaOverrides + from ..timeseries._resampling._config import ResamplerConfig from . import _data_pipeline, connection_manager from ._data_pipeline import ( @@ -393,6 +395,7 @@ async def initialize( api_power_request_timeout: timedelta = timedelta(seconds=5.0), # pylint: disable-next: line-too-long battery_power_manager_algorithm: PowerManagerAlgorithm = PowerManagerAlgorithm.SHIFTING_MATRYOSHKA, # noqa: E501 + component_graph_config: ComponentGraphConfig | None = None, ) -> None: """Initialize the microgrid connection manager and the data pipeline. @@ -409,8 +412,17 @@ async def initialize( will be unavailable from the corresponding component pools. battery_power_manager_algorithm: The power manager algorithm to use for batteries. + component_graph_config: The configuration for building the component + graph. Use it, for example, to make the per-category formulas + (like the battery or PV formulas) read from the meter first + (`prefer_meters_in_component_formulas`), or to set per-formula + overrides through `FormulaOverrides`. When `None`, the library's + default configuration is used. """ - await connection_manager.initialize(server_url) + await connection_manager.initialize( + server_url, + component_graph_config=component_graph_config, + ) await _data_pipeline.initialize( resampler_config, api_power_request_timeout=api_power_request_timeout, @@ -419,6 +431,8 @@ async def initialize( __all__ = [ + "ComponentGraphConfig", + "FormulaOverrides", "PowerManagerAlgorithm", "initialize", "consumer", diff --git a/src/frequenz/sdk/microgrid/connection_manager.py b/src/frequenz/sdk/microgrid/connection_manager.py index 95fb20618..828f0ff8e 100644 --- a/src/frequenz/sdk/microgrid/connection_manager.py +++ b/src/frequenz/sdk/microgrid/connection_manager.py @@ -18,6 +18,7 @@ MicrogridApiClient, MicrogridInfo, ) +from frequenz.microgrid_component_graph import ComponentGraphConfig from .component_graph import ComponentGraph @@ -27,7 +28,12 @@ class ConnectionManager(ABC): """Creates and stores core features.""" - def __init__(self, server_url: str) -> None: + def __init__( + self, + server_url: str, + *, + component_graph_config: ComponentGraphConfig | None = None, + ) -> None: """Create object instance. Args: @@ -36,9 +42,12 @@ def __init__(self, server_url: str) -> None: where the port should be an int between `0` and `65535` (defaulting to `9090`) and ssl should be a boolean (defaulting to false). For example: `grpc://localhost:1090?ssl=true`. + component_graph_config: The configuration for building the component + graph. When `None`, the library's default configuration is used. """ super().__init__() self._server_url = server_url + self._component_graph_config = component_graph_config @property def server_url(self) -> str: @@ -94,7 +103,12 @@ async def _initialize(self) -> None: class _InsecureConnectionManager(ConnectionManager): """Microgrid Api with insecure channel implementation.""" - def __init__(self, server_url: str) -> None: + def __init__( + self, + server_url: str, + *, + component_graph_config: ComponentGraphConfig | None = None, + ) -> None: """Create and stores core features. Args: @@ -103,8 +117,10 @@ def __init__(self, server_url: str) -> None: where the port should be an int between `0` and `65535` (defaulting to `9090`) and ssl should be a boolean (defaulting to false). For example: `grpc://localhost:1090?ssl=true`. + component_graph_config: The configuration for building the component + graph. When `None`, the library's default configuration is used. """ - super().__init__(server_url) + super().__init__(server_url, component_graph_config=component_graph_config) self._client = MicrogridApiClient(server_url) # To create graph from the API client we need await. # So create empty graph here, and update it in `run` method. @@ -172,14 +188,23 @@ async def _initialize(self) -> None: self._client.list_components(), self._client.list_connections(), ) - self._graph = ComponentGraph(set(components), set(connections)) + if self._component_graph_config is None: + self._graph = ComponentGraph(set(components), set(connections)) + else: + self._graph = ComponentGraph( + set(components), set(connections), self._component_graph_config + ) _CONNECTION_MANAGER: ConnectionManager | None = None """The ConnectionManager singleton instance.""" -async def initialize(server_url: str) -> None: +async def initialize( + server_url: str, + *, + component_graph_config: ComponentGraphConfig | None = None, +) -> None: """Initialize the MicrogridApi. This function should be called only once. Args: @@ -188,6 +213,8 @@ async def initialize(server_url: str) -> None: where the port should be an int between `0` and `65535` (defaulting to `9090`) and ssl should be a boolean (defaulting to false). For example: `grpc://localhost:1090?ssl=true`. + component_graph_config: The configuration for building the component + graph. When `None`, the library's default configuration is used. """ # From Doc: pylint just try to discourage this usage. # That doesn't mean you cannot use it. @@ -197,7 +224,10 @@ async def initialize(server_url: str) -> None: _logger.info("Connecting to microgrid at %s", server_url) - connection_manager = _InsecureConnectionManager(server_url) + connection_manager = _InsecureConnectionManager( + server_url, + component_graph_config=component_graph_config, + ) await connection_manager._initialize() # pylint: disable=protected-access # Check again that _MICROGRID_API is None in case somebody had the great idea of diff --git a/tests/microgrid/test_microgrid_api.py b/tests/microgrid/test_microgrid_api.py index ff546c339..ac2aa73ae 100644 --- a/tests/microgrid/test_microgrid_api.py +++ b/tests/microgrid/test_microgrid_api.py @@ -28,7 +28,7 @@ Meter, ) -from frequenz.sdk.microgrid import connection_manager +from frequenz.sdk.microgrid import ComponentGraphConfig, connection_manager _MICROGRID_ID = MicrogridId(1) @@ -135,6 +135,28 @@ def microgrid(self) -> MicrogridInfo: ), ) + @staticmethod + def _mock_microgrid_client( + components: list[list[Component]], + connections: list[list[ComponentConnection]], + microgrid: MicrogridInfo, + ) -> MagicMock: + """Create a mock microgrid API client. + + Args: + components: components to return, one list per call. + connections: connections to return, one list per call. + microgrid: the information about the microgrid. + + Returns: + the mock client. + """ + client = MagicMock() + client.list_components = AsyncMock(side_effect=components) + client.list_connections = AsyncMock(side_effect=connections) + client.get_microgrid_info = AsyncMock(return_value=microgrid) + return client + @mock.patch("grpc.aio.insecure_channel") async def test_connection_manager( self, @@ -151,10 +173,9 @@ async def test_connection_manager( connections: connections microgrid: the information about the microgrid """ - microgrid_client = MagicMock() - microgrid_client.list_components = AsyncMock(side_effect=components) - microgrid_client.list_connections = AsyncMock(side_effect=connections) - microgrid_client.get_microgrid_info = AsyncMock(return_value=microgrid) + microgrid_client = self._mock_microgrid_client( + components, connections, microgrid + ) with mock.patch( "frequenz.sdk.microgrid.connection_manager.MicrogridApiClient", @@ -214,6 +235,57 @@ async def test_connection_manager( assert api.microgrid_id == microgrid.id assert api.location == microgrid.location + @mock.patch("grpc.aio.insecure_channel") + async def test_component_graph_config( + self, + _insecure_channel_mock: MagicMock, + components: list[list[Component]], + connections: list[list[ComponentConnection]], + microgrid: MicrogridInfo, + ) -> None: + """Test that the component graph config is used to build the graph. + + Args: + _insecure_channel_mock: insecure channel mock from `mock.patch` + components: components + connections: connections + microgrid: the information about the microgrid + """ + microgrid_client = self._mock_microgrid_client( + [components[0]] * 2, [connections[0]] * 2, microgrid + ) + + with mock.patch( + "frequenz.sdk.microgrid.connection_manager.MicrogridApiClient", + return_value=microgrid_client, + ): + # pylint: disable=protected-access + default_manager = connection_manager._InsecureConnectionManager( + "grpc://127.0.0.1:10001" + ) + await default_manager._initialize() + + meters_first_manager = connection_manager._InsecureConnectionManager( + "grpc://127.0.0.1:10001", + component_graph_config=ComponentGraphConfig( + prefer_meters_in_component_formulas=True + ), + ) + await meters_first_manager._initialize() + + batteries = {ComponentId(9), ComponentId(12)} + # By default the battery inverters are the primary source and the + # meters are the fallback. + assert ( + default_manager.component_graph.battery_formula(batteries) + == "COALESCE(#8, #7, 0.0) + COALESCE(#11, #10, 0.0)" + ) + # With `prefer_meters_in_component_formulas` the meters come first. + assert ( + meters_first_manager.component_graph.battery_formula(batteries) + == "COALESCE(#7, #8, 0.0) + COALESCE(#10, #11, 0.0)" + ) + @mock.patch("grpc.aio.insecure_channel") async def test_connection_manager_another_method( self, diff --git a/tests/timeseries/_battery_pool/test_battery_pool.py b/tests/timeseries/_battery_pool/test_battery_pool.py index 39b4a09c3..1c56dba8c 100644 --- a/tests/timeseries/_battery_pool/test_battery_pool.py +++ b/tests/timeseries/_battery_pool/test_battery_pool.py @@ -48,7 +48,7 @@ from ...utils.component_data_streamer import MockComponentDataStreamer from ...utils.component_data_wrapper import BatteryDataWrapper, InverterDataWrapper from ...utils.component_graph_utils import ( - ComponentGraphConfig, + ComponentGraphSpec, create_component_graph_structure, ) from ...utils.mock_microgrid_client import MockMicrogridClient @@ -113,7 +113,7 @@ class SetupArgs: def create_mock_microgrid( - mocker: MockerFixture, config: ComponentGraphConfig + mocker: MockerFixture, config: ComponentGraphSpec ) -> MockMicrogridClient: """Create mock microgrid and initialize it. @@ -143,9 +143,7 @@ async def setup_all_batteries(mocker: MockerFixture) -> AsyncIterator[SetupArgs] Yields: Arguments that are needed in test. """ - mock_microgrid = create_mock_microgrid( - mocker, ComponentGraphConfig(batteries_num=2) - ) + mock_microgrid = create_mock_microgrid(mocker, ComponentGraphSpec(batteries_num=2)) min_update_interval: float = 0.2 # pylint: disable=protected-access microgrid._data_pipeline._DATA_PIPELINE = None @@ -195,9 +193,7 @@ async def setup_batteries_pool(mocker: MockerFixture) -> AsyncIterator[SetupArgs Yields: Arguments that are needed in test. """ - mock_microgrid = create_mock_microgrid( - mocker, ComponentGraphConfig(batteries_num=4) - ) + mock_microgrid = create_mock_microgrid(mocker, ComponentGraphSpec(batteries_num=4)) streamer = MockComponentDataStreamer(mock_microgrid) min_update_interval: float = 0.2 # pylint: disable=protected-access @@ -520,7 +516,21 @@ async def test_battery_pool_power(mocker: MockerFixture) -> None: power_receiver = battery_pool.power.new_receiver() # send meter power [grid_meter, battery1_meter, battery2_meter] + # and battery inverter power [battery1_inverter, battery2_inverter]. + # The inverter values differ from the meter values, to check that + # the inverters are the primary source. + await mockgrid.mock_resampler.send_meter_power([100.0, 20.0, 30.0]) + await mockgrid.mock_resampler.send_bat_inverter_power([21.0, 31.0]) + assert (await power_receiver.receive()).value == Power.from_watts(52.0) + + # When the inverters have no value, the formula falls back to the + # meters. The fallback subscription starts after the first `None`, + # so send twice and skip one output. + await mockgrid.mock_resampler.send_meter_power([100.0, 20.0, 30.0]) + await mockgrid.mock_resampler.send_bat_inverter_power([None, None]) await mockgrid.mock_resampler.send_meter_power([100.0, 20.0, 30.0]) + await mockgrid.mock_resampler.send_bat_inverter_power([None, None]) + await power_receiver.receive() assert (await power_receiver.receive()).value == Power.from_watts(50.0) diff --git a/tests/timeseries/_ev_charger_pool/test_ev_charger_pool.py b/tests/timeseries/_ev_charger_pool/test_ev_charger_pool.py index e7a7e1804..b406ca313 100644 --- a/tests/timeseries/_ev_charger_pool/test_ev_charger_pool.py +++ b/tests/timeseries/_ev_charger_pool/test_ev_charger_pool.py @@ -46,8 +46,11 @@ async def test_ev_power( # pylint: disable=too-many-locals ev_pool = microgrid.new_ev_charger_pool(priority=5) power_receiver = ev_pool.power.new_receiver() + # The charger values sum to 15 W, not the 16 W on the meter, to + # check that the chargers are the primary source. await mockgrid.mock_resampler.send_meter_power([16.0]) - assert (await power_receiver.receive()).value == Power.from_watts(16.0) + await mockgrid.mock_resampler.send_evc_power([2.0, 6.0, 7.0]) + assert (await power_receiver.receive()).value == Power.from_watts(15.0) async def test_power_status_same_instance_subscriptions_work( diff --git a/tests/timeseries/_formulas/test_composition.py b/tests/timeseries/_formulas/test_composition.py index e81e3a289..040714070 100644 --- a/tests/timeseries/_formulas/test_composition.py +++ b/tests/timeseries/_formulas/test_composition.py @@ -232,13 +232,13 @@ async def test_formula_composition_min_max(self, mocker: MockerFixture) -> None: assert ( str(formula_min) == "[grid_power_min](" - + "MIN([grid_power](#4), [chp_power](COALESCE(#7, #5, 0.0)))" + + "MIN([grid_power](#4), [chp_power](COALESCE(#5, #7, 0.0)))" + ")" ) assert ( str(formula_max) == "[grid_power_max](" - + "MAX([grid_power](#4), [chp_power](COALESCE(#7, #5, 0.0)))" + + "MAX([grid_power](#4), [chp_power](COALESCE(#5, #7, 0.0)))" + ")" ) diff --git a/tests/timeseries/test_logical_meter.py b/tests/timeseries/test_logical_meter.py index fe148190b..a2464b21b 100644 --- a/tests/timeseries/test_logical_meter.py +++ b/tests/timeseries/test_logical_meter.py @@ -49,5 +49,8 @@ async def test_pv_power(self, mocker: MockerFixture) -> None: stack.push_async_callback(pv_pool.stop) pv_power_receiver = pv_pool.power.new_receiver() + # The inverter values differ from the meter values, to check + # that the inverters are the primary source. await mockgrid.mock_resampler.send_meter_power([-10.0, -20.0]) - assert (await pv_power_receiver.receive()).value == Power.from_watts(-30.0) + await mockgrid.mock_resampler.send_pv_inverter_power([-9.0, -19.0]) + assert (await pv_power_receiver.receive()).value == Power.from_watts(-28.0) diff --git a/tests/utils/component_graph_utils.py b/tests/utils/component_graph_utils.py index 74979b493..68e439ee2 100644 --- a/tests/utils/component_graph_utils.py +++ b/tests/utils/component_graph_utils.py @@ -21,7 +21,7 @@ @dataclass -class ComponentGraphConfig: +class ComponentGraphSpec: """Config with information how the component graph should be created.""" grid_side_meter: bool = True @@ -44,12 +44,12 @@ class ComponentGraphConfig: def create_component_graph_structure( - component_graph_config: ComponentGraphConfig, + component_graph_spec: ComponentGraphSpec, ) -> tuple[set[Component], set[ComponentConnection]]: """Create structure of components graph. Args: - component_graph_config: config that tells what graph should have. + component_graph_spec: spec that tells what the graph should have. Returns: Create set of components and set of connections between them. @@ -72,11 +72,11 @@ def create_component_graph_structure( connections = {ComponentConnection(source=grid_id, destination=main_meter_id)} junction_id = grid_id - if component_graph_config.grid_side_meter: + if component_graph_spec.grid_side_meter: junction_id = main_meter_id start_idx = 3 - for _ in range(component_graph_config.batteries_num): + for _ in range(component_graph_spec.batteries_num): meter_id = ComponentId(start_idx) inv_id = ComponentId(int(start_idx) + 1) battery_id = ComponentId(start_idx + 2) @@ -90,7 +90,7 @@ def create_component_graph_structure( connections.add(ComponentConnection(source=meter_id, destination=inv_id)) connections.add(ComponentConnection(source=inv_id, destination=battery_id)) - for _ in range(component_graph_config.solar_inverters_num): + for _ in range(component_graph_spec.solar_inverters_num): meter_id = ComponentId(start_idx) inv_id = ComponentId(start_idx + 1) start_idx += 2 @@ -100,7 +100,7 @@ def create_component_graph_structure( connections.add(ComponentConnection(source=junction_id, destination=meter_id)) connections.add(ComponentConnection(source=meter_id, destination=inv_id)) - for _ in range(component_graph_config.ev_chargers): + for _ in range(component_graph_spec.ev_chargers): ev_id = ComponentId(start_idx) start_idx += 1