From a189a80d95554c7d3cd090da904b98642717d102 Mon Sep 17 00:00:00 2001 From: "F.N. Claessen" Date: Mon, 3 Aug 2026 12:39:12 +0200 Subject: [PATCH 1/3] fix: bind a regular commitment to its commodity's aggregate, not each device Fixes #2379 (unreleased regression from #1946). convert_to_commitments emitted one FlowCommitment per device (device=d, from re-enumerating the raw flex-model list), so a regular commitment held each device to the baseline individually. Bind one commitment per commodity over all its devices instead (device=, device_group=commodity), mirroring the internal ' net energy' commitment; this also drops the fragile raw-flex-model enumeration in favour of the device inventory. The now-unused flex_model parameter is removed; the direct-convert tests set device_inventory and assert the aggregate device set. Adds a two-devices-of-one-commodity regression test (the combined flow reaches a baseline neither device could carry alone). Distinct from #2326/#2355 (that is the solver's EMS-level device=None constraint being unbound, affecting direct device_scheduler callers); this path uses device=, so it does not go through that code. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01TaMepbxRYzJxQKFtu6Hq4p Signed-off-by: F.N. Claessen --- flexmeasures/data/models/planning/storage.py | 42 +++++--- .../models/planning/tests/test_commitments.py | 102 ++++++++++++++++-- 2 files changed, 119 insertions(+), 25 deletions(-) diff --git a/flexmeasures/data/models/planning/storage.py b/flexmeasures/data/models/planning/storage.py index e4d6765c24..70cb4a01d3 100644 --- a/flexmeasures/data/models/planning/storage.py +++ b/flexmeasures/data/models/planning/storage.py @@ -379,7 +379,6 @@ def _prepare(self, skip_validation: bool = False) -> tuple: # noqa: C901 query_window=(start, end), resolution=resolution, beliefs_before=belief_time, - flex_model=flex_model, ) index = initialize_index(start, end, resolution) @@ -1634,7 +1633,6 @@ def _build_scoped_commitment( def convert_to_commitments( self, - flex_model, **timing_kwargs, ) -> list[FlowCommitment | StockCommitment]: """Convert list of commitment specifications (dicts) to a list of FlowCommitments. @@ -1644,6 +1642,10 @@ def convert_to_commitments( context's commodity (matching how the UI editor scopes commitments per commodity tab). + An unscoped commitment binds the aggregate flow of all its commodity's devices; + a scoped commitment (a ``sensors`` list or ``group`` reference) binds a subset. + Device indices come from the device inventory, never from the raw flex-model list. + User-given commitment names are kept as is, but the resulting commitments are tagged with provenance "custom", so cost reporting can tell them apart from the commitments the scheduler sets up internally (e.g. "electricity net energy"). @@ -1711,20 +1713,14 @@ def convert_to_commitments( if scoped is not None: commitments.append(scoped) continue - bound_device_count = 0 - for d, flex_model_d in enumerate(flex_model): - device_commodity = flex_model_d.get("commodity", "electricity") - if device_commodity != commitment_commodity: - continue - commitment = FlowCommitment( - device=d, - device_group=device_commodity, - provenance="custom", - **commitment_spec, - ) - commitments.append(commitment) - bound_device_count += 1 - if bound_device_count == 0: + # A regular (unscoped) commitment binds the *aggregate* flow of all the commitment commodity's devices as one commitment (issue #2379), + # mirroring the internal " net energy" commitment. + # Device indices come from the device inventory (canonical, and including the commodity's inflexible devices), + # never from re-enumerating the raw flex-model list. + commodity_devices = self.device_inventory.commodity_to_devices.get( + commitment_commodity, [] + ) + if not commodity_devices: current_app.logger.warning( f"Commitment '{commitment_spec.get('name')}' has commodity" f" '{commitment_commodity}', which matches none of the devices" @@ -1732,6 +1728,20 @@ def convert_to_commitments( " (check for a typo in the commitment's `commodity` field, or in" " a device's `commodity` field in the flex-model)." ) + continue + index = commitment_spec["index"] + commitments.append( + FlowCommitment( + device=pd.Series( + [tuple(commodity_devices)] * len(index), + index=index, + name="device", + ), + device_group=commitment_commodity, + provenance="custom", + **commitment_spec, + ) + ) return commitments diff --git a/flexmeasures/data/models/planning/tests/test_commitments.py b/flexmeasures/data/models/planning/tests/test_commitments.py index ca0fc154f3..7a57f96cfe 100644 --- a/flexmeasures/data/models/planning/tests/test_commitments.py +++ b/flexmeasures/data/models/planning/tests/test_commitments.py @@ -1825,8 +1825,12 @@ def test_user_commitment_names_and_provenance(app): } flex_model = [{"commodity": "electricity"}] + from flexmeasures.data.models.planning.devices import DeviceInventory + + scheduler.device_inventory = DeviceInventory.from_flex_config( + flex_model, scheduler.flex_context + ) commitments = scheduler.convert_to_commitments( - flex_model, query_window=(start, end), resolution=resolution, beliefs_before=start, @@ -2060,8 +2064,12 @@ def test_commitment_commodity_does_not_bind_other_commodity_devices(): end = pd.Timestamp("2024-01-01T03:00:00+01:00") resolution = pd.Timedelta("1h") + from flexmeasures.data.models.planning.devices import DeviceInventory + + scheduler.device_inventory = DeviceInventory.from_flex_config( + flex_model, scheduler.flex_context + ) commitments = scheduler.convert_to_commitments( - flex_model=flex_model, query_window=(start, end), resolution=resolution, beliefs_before=None, @@ -2076,15 +2084,90 @@ def test_commitment_commodity_does_not_bind_other_commodity_devices(): # The gas commitment binds only the gas device (index 1), not the electricity # device (index 0). - assert (gas_commitment.device == 1).all() + assert set(gas_commitment.device.iloc[0]) == {1} assert set(gas_commitment.device_group.unique()) == {"gas"} # The electricity commitment (commodity defaulting to "electricity") binds only # the electricity device (index 0), not the gas device (index 1). - assert (electricity_commitment.device == 0).all() + assert set(electricity_commitment.device.iloc[0]) == {0} assert set(electricity_commitment.device_group.unique()) == {"electricity"} +def test_unscoped_commitment_binds_commodity_aggregate(app, db): + """Regression (#2379): a regular (unscoped) commitment binds the *aggregate* flow of its commodity's devices, + not each device individually. + Two 8 MW heaters under a 10 MW baseline reach a combined 10 MW (a level neither could carry alone); + a per-device binding would instead hold each heater to 10 MW, pushing each to its 8 MW cap (combined 16). + """ + heater_type = get_or_create_model(GenericAssetType, name="e-heater") + site = GenericAsset( + name="Aggregate commitment site", generic_asset_type=heater_type + ) + db.session.add(site) + db.session.flush() + + resolution = pd.Timedelta("1h") + start = pd.Timestamp("2026-02-01T00:00:00+01:00") + end = pd.Timestamp("2026-02-01T04:00:00+01:00") + + def sensor(name): + s = Sensor( + name=name, unit="MW", event_resolution=resolution, generic_asset=site + ) + db.session.add(s) + return s + + heater_1 = sensor("aggregate heater 1") + heater_2 = sensor("aggregate heater 2") + db.session.flush() + + flex_model = [ + { + "sensor": heater_1.id, + "power-capacity": "8 MW", + "consumption-capacity": "8 MW", + "production-capacity": "0 kW", + }, + { + "sensor": heater_2.id, + "power-capacity": "8 MW", + "consumption-capacity": "8 MW", + "production-capacity": "0 kW", + }, + ] + flex_context = { + "consumption-price": "50 EUR/MWh", + "production-price": "50 EUR/MWh", + "site-power-capacity": "1 GW", + "commitments": [ + { + # Unscoped: binds the aggregate of the electricity devices. + "name": "aggregate band", + "baseline": "10 MW", + "down-price": "-10000 EUR/MWh", + } + ], + } + + scheduler = StorageScheduler( + asset_or_sensor=site, + start=start, + end=end, + resolution=resolution, + belief_time=start, + flex_model=flex_model, + flex_context=flex_context, + return_multiple=True, + ) + results = scheduler.compute(skip_validation=True) + schedules = { + r["sensor"]: r["data"] for r in results if r.get("name") == "storage_schedule" + } + combined = schedules[heater_1] + schedules[heater_2] + # Aggregate binding: the two heaters together reach exactly the 10 MW baseline. + np.testing.assert_allclose(combined.iloc[:-1], 10.0, rtol=1e-4) + + def test_sensor_scoped_commitment_binds_aggregate_of_selected_devices(app, db): """A commitment scoped to specific sensors (here: two e-heaters) binds their aggregate flow as one commitment: a baseline of 10 MW with a steep penalty on downward deviation keeps their combined consumption at 10 MW, @@ -2323,7 +2406,6 @@ def commitment_devices(scope): flex_model, scheduler.flex_context ) commitments = scheduler.convert_to_commitments( - flex_model, query_window=(start, end), resolution=resolution, beliefs_before=start, @@ -2374,7 +2456,6 @@ def test_scoped_commitment_pins_commodity_to_scoped_devices(app): ) commitments = scheduler.convert_to_commitments( - flex_model, query_window=(start, end), resolution=resolution, beliefs_before=start, @@ -2422,7 +2503,6 @@ def test_scoped_commitment_with_no_matching_devices_warns_and_binds_nothing( with caplog.at_level(logging.WARNING): commitments = scheduler.convert_to_commitments( - flex_model, query_window=(start, end), resolution=resolution, beliefs_before=start, @@ -2467,15 +2547,19 @@ def test_commitments_in_commodity_contexts_are_converted(app): # Flexible devices: 0 = electricity, 1 = gas. flex_model = [{"commodity": "electricity"}, {"commodity": "gas"}] + from flexmeasures.data.models.planning.devices import DeviceInventory + + scheduler.device_inventory = DeviceInventory.from_flex_config( + flex_model, scheduler.flex_context + ) commitments = scheduler.convert_to_commitments( - flex_model, query_window=(start, end), resolution=resolution, beliefs_before=start, ) assert len(commitments) == 2 gas_commitment = next(c for c in commitments if c.name == "gas context commitment") - assert (gas_commitment.device == 1).all() + assert set(gas_commitment.device.iloc[0]) == {1} assert set(gas_commitment.device_group.unique()) == {"gas"} # The original specs (including the nested ones) are not mutated. From 8f5cbdb89a752205cfeffe2ea0f76ad6c5d7df86 Mon Sep 17 00:00:00 2001 From: "F.N. Claessen" Date: Mon, 3 Aug 2026 12:44:18 +0200 Subject: [PATCH 2/3] docs: append PR reference to the multi-commodity changelog entry The regular-commitment aggregation regression was introduced by the multi-commodity work; append this PR to that existing changelog entry rather than adding a new one. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01TaMepbxRYzJxQKFtu6Hq4p Signed-off-by: F.N. Claessen --- documentation/changelog.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/documentation/changelog.rst b/documentation/changelog.rst index 9b0682e1d6..3db27c7dab 100644 --- a/documentation/changelog.rst +++ b/documentation/changelog.rst @@ -32,7 +32,7 @@ New features * Sensor references in flex-model and flex-context support various ways of filtering by source [see `PR #2209 `_] * Let storage scheduling infer missing ``power-capacity`` from directional device capacities before falling back to site capacity, and default the missing opposite capacity to zero when only a non-zero ``consumption-capacity`` or ``production-capacity`` is configured [see `PR #2222 `_] * Support multiple feeders to a shared storage [see `PR #2001 `_, `PR #2321 `_, `PR #2322 `_ and `PR #2325 `_] -* The flex-context can now define multiple commodities, each specifying their own prices and grid capacities [see `PR #1946 `_, `PR #2172 `_, `PR #2235 `_ and `PR #2271 `_] +* The flex-context can now define multiple commodities, each specifying their own prices and grid capacities [see `PR #1946 `_, `PR #2172 `_, `PR #2235 `_, `PR #2271 `_ and `PR #2380 `_] * In the UI, the flex-context editor supports editing commitments (name, baseline and deviation prices, each accepting a fixed value or a sensor), also within each commodity context; the commitment's commodity follows the commodity tab being edited, and new commitments start with zero-valued baseline and prices [see `PR #2287 `_] * A commitment in the flex-context now requires a ``baseline`` and at least one deviation price (``up-price`` and/or ``down-price``), as already documented; commitment costs are reported in the scheduling results under the user-given name (with a ``(custom)`` suffix in the rare case the name collides with a scheduler-internal commitment name) [see `PR #2287 `_] * Commodity contexts that omit grid-connection fields (prices and site capacities) now get smart defaults instead of failing or silently leaving the grid unconstrained — for instance, a bare ``{"commodity": "gas"}`` is treated as having no grid connection; see :ref:`commodity_context_defaults` for the full rules [see `PR #2272 `_] From cf180dbe4c0ceba4c8a7446abde4b83e393e0d30 Mon Sep 17 00:00:00 2001 From: "F.N. Claessen" Date: Mon, 3 Aug 2026 13:06:42 +0200 Subject: [PATCH 3/3] test: update commitment tests for aggregate (unscoped) semantics Two existing tests encoded the pre-#2379 per-device commitment behaviour: - test_flex_context_commitments_target_devices_not_stock_only_entries: a regular commitment now yields a single aggregate commitment binding the scheduled devices (indices 0 and 1), not one commitment per raw flex-model entry. The stock-only exclusion it guards against is unchanged. - test_create_simultaneous_jobs: the sample commitment rewarding supply binds the site aggregate, so it stays inactive while the site is net-consuming and no longer biases the EV/battery split (EV costs 2.3125 -> 2.2375). Total cost is unchanged, matching the fixture's stated intent that the commitment not affect the schedule. Signed-off-by: F.N. Claessen --- .../data/models/planning/tests/test_solver.py | 22 +++++++++++-------- .../tests/test_scheduling_simultaneous.py | 7 +++++- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/flexmeasures/data/models/planning/tests/test_solver.py b/flexmeasures/data/models/planning/tests/test_solver.py index c6a9c1b009..e1a4ecdd31 100644 --- a/flexmeasures/data/models/planning/tests/test_solver.py +++ b/flexmeasures/data/models/planning/tests/test_solver.py @@ -3468,10 +3468,11 @@ def test_flex_context_commitments_target_devices_not_stock_only_entries( ): """Flex-context commitments must bind the scheduled devices, not stock-only entries. - With a stock-only entry listed first, a flex-context commitment should still yield - one commitment per scheduled device (indices 0 and 1), rather than one per - flex-model entry (indices 0, 1 and 2, of which index 2 does not exist as a - flexible device). + A regular (unscoped) commitment binds the *aggregate* flow of its commodity's + devices as a single commitment (issue #2379). With a stock-only entry listed + first, that single commitment should bind the scheduled devices (indices 0 and 1), + and never the stock-only entry (which is not a flexible device), rather than one + commitment per raw flex-model entry (indices 0, 1 and 2). """ battery_type = setup_generic_asset_types["battery"] site = _add_parent_site(db, building, "commitment test site") @@ -3528,15 +3529,18 @@ def test_flex_context_commitments_target_devices_not_stock_only_entries( test_commitments = [c for c in commitments if c.name == "test commitment"] num_devices = 2 - assert len(test_commitments) == num_devices, ( - f"Expected one commitment per scheduled device ({num_devices}), " + assert len(test_commitments) == 1, ( + f"Expected a single aggregate commitment binding all scheduled devices, " f"got {len(test_commitments)} (one per flex-model entry, including the " "stock-only entry)." ) - commitment_devices = {int(d) for c in test_commitments for d in c.device.unique()} + # The aggregate commitment binds all of its commodity's devices at once, + # so each row of its device column is the tuple of scheduled device indices. + commitment_devices = {int(d) for d in test_commitments[0].device.iloc[0]} assert commitment_devices == set(range(num_devices)), ( - f"Commitments target device indices {sorted(commitment_devices)}, " - f"expected {sorted(range(num_devices))}." + f"Commitment targets device indices {sorted(commitment_devices)}, " + f"expected {sorted(range(num_devices))} (the scheduled devices, not the " + "stock-only entry)." ) diff --git a/flexmeasures/data/tests/test_scheduling_simultaneous.py b/flexmeasures/data/tests/test_scheduling_simultaneous.py index b5469d0e6b..ebf4c4cd8e 100644 --- a/flexmeasures/data/tests/test_scheduling_simultaneous.py +++ b/flexmeasures/data/tests/test_scheduling_simultaneous.py @@ -134,7 +134,12 @@ def test_create_simultaneous_jobs( # Define expected costs based on resolution expected_total_cost = -3.2775 - expected_ev_costs = 2.3125 + # Aggregate (unscoped) commitment semantics (issue #2379): the sample commitment + # rewarding supply binds the site's *aggregate* flow, so it stays inactive while + # the site is net-consuming and does not bias the per-device dispatch. Under the + # earlier per-device binding it wrongly rewarded the battery's supply, shifting the + # EV/battery split (EV costs were €2.3125); the total cost is unchanged either way. + expected_ev_costs = 2.2375 expected_battery_costs = expected_total_cost - expected_ev_costs # Check costs