Track devices via a typed device inventory in the StorageScheduler#2321
Merged
Conversation
Add strict-xfail regression tests for four confirmed device-alignment bugs in multi-device storage scheduling: - A stock-only entry listed before device entries shifts every device's properties (power capacity, efficiencies) by one, because per-device properties are read from the unfiltered flex-model list. - A device without a state-of-charge sensor silently loses its own SoC parameters when a stock-only entry is present, because the synthetic stock keys diverge between _prepare and _build_stock_groups. - Flex-context commitments are assigned device indices by enumerating the unfiltered flex-model list, so stock-only entries receive commitments meant for devices. - A device referencing its power sensor only via a nested consumption/ production output reference is misclassified as stock-only and dropped (carried over from PR #2319, which this branch subsumes). The xfail markers are removed in the commits that fix each bug. Signed-off-by: F.N. Claessen <felix@seita.nl>
Introduce planning/devices.py: every flex-model entry is classified exactly once into a DeviceInventory, which becomes the single source of truth for - entry roles (device / stock-only, extensible to group entries and converter ports), - canonical solver device indices (flexible devices first, then top-level inflexible devices, then per-commodity-context inflexible devices), - stock groups and their SoC parameters (sharing one key space, so parameters can no longer be lost to diverging synthetic keys), and - sensor-id and commodity lookups. Power sensors are resolved from the top-level sensor field or, failing that, from a nested consumption/production output reference, so such devices are no longer misclassified as stock-only entries. The raw deserialized flex-model dicts are kept as-is on each FlexDevice, so downstream code and new flex-model fields need no dataclass changes. Not yet wired into the StorageScheduler; see subsequent commits. Signed-off-by: F.N. Claessen <felix@seita.nl>
Scheduling on the shared module-scoped building fixture is fragile: earlier tests create sensors on it whose legacy attributes (capacity_in_mw, max_soc_in_mwh, ...) get moved into the building's flex-model, which collect_flex_config then injects into the test's flex-model as an extra asset-only device entry, changing the optimization problem. Each alignment test now schedules on its own fresh parent site. Signed-off-by: F.N. Claessen <felix@seita.nl>
Build the DeviceInventory once after flex-config deserialization (with a lazy rebuild for tests that bypass deserialization), and replace _prepare()'s ad-hoc entry classification with it. The local flex_model variable now holds (copies of) the classified device flex-models instead of reverting to the unfiltered flex-model list. This fixes, by construction: - Devices no longer inherit a neighboring entry's properties (power capacity, efficiencies, output sensors) when a stock-only entry precedes them in the flex-model list: all per-device property lists, including the sensors/assets pair, now derive from the same filtered, index-aligned device list. - Devices without a state-of-charge sensor no longer lose their own SoC parameters when a stock-only entry is present (the synthetic stock keys of stock parameters and stock groups now come from one counter). - Flex-context commitments now bind the scheduled devices instead of enumerating the unfiltered flex-model list. - Devices referencing their power sensor only via a nested consumption/ production output reference are scheduled (previously misclassified as stock-only and silently dropped) — subsumes PR #2319. - The dead (and misaligned) stock-groups build in _deserialize_flex_model and the duplicated stock-only detection are gone. The strict-xfail alignment regression tests now pass; their markers are removed. _prepare()'s return value and device_scheduler's interface are unchanged. Signed-off-by: F.N. Claessen <felix@seita.nl>
Both _prepare() and the compute()-side result mapping now take the commodity -> device indices mapping (flexible devices, then top-level inflexible devices, then per-commodity-context inflexible devices) from the device inventory, instead of maintaining two mirrored ~40-line enumerations that had to stay in sync byte for byte. The device constraints for inflexible devices are likewise sized and filled from the inventory's canonical order. _reconstruct_commodity_to_devices keeps a fallback for bare schedulers (e.g. in tests) that classifies the stored device models on the fly. Signed-off-by: F.N. Claessen <felix@seita.nl>
…stock Only a sensor reference can link devices into a shared stock. A state-of-charge field holding timed values (a list of specs) is not a stock identity: classify such devices under their own synthetic stock key, keeping their SoC parameters. On main, this shape crashed _prepare (unhashable stock_models key) if it carried SoC parameters; it only went unnoticed because such flex-models were exercised up to deserialization, never scheduling. Signed-off-by: F.N. Claessen <felix@seita.nl>
Two latent order-dependence bugs, surfaced by running the planning test modules in a new order: - GenericAsset.get_flex_context crashed (TypeError) when an ancestor's flex_context was still None, e.g. a pending asset created without one, before its column default is applied on flush. - The charging station test fixture never flushed, so its assets had no IDs unless an earlier test happened to trigger a flush; scheduler config collection then silently failed to match the flex-model to its asset. The affected test (test_deserialize_storage_soc_at_start_from_ state_of_charge_time_series) failed when run in isolation. Both are now fixed; the test passes in any order. Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
…tory Signed-off-by: F.N. Claessen <felix@seita.nl> # Conflicts: # documentation/changelog.rst
Documentation build overview
7 files changed ·
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a typed DeviceInventory as a single source of truth for classifying flex-model entries (device vs stock-only vs inflexible) and establishing canonical solver device indices, then refactors StorageScheduler to use that inventory to eliminate several index-alignment bugs in multi-device scheduling.
Changes:
- Add
planning/devices.pyimplementingDeviceInventory,FlexDevice, and role classification (including nested output-sensor power references). - Refactor
StorageSchedulerto derive device lists, stock groups/params, and commodity→device mappings from the inventory (removing duplicated enumeration logic). - Add solver/inventory regression tests and update changelog to document the bug fixes and infrastructure change.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| flexmeasures/data/models/planning/tests/test_solver.py | Adds solver-level regression tests covering alignment, stock-only ordering, SoC-param retention, and commitment targeting |
| flexmeasures/data/models/planning/tests/test_device_inventory.py | Adds unit tests for inventory classification and canonical enumeration |
| flexmeasures/data/models/planning/storage.py | Uses DeviceInventory as the canonical source for device/stock/inflexible enumeration and mappings |
| flexmeasures/data/models/planning/devices.py | New typed inventory module for flex-config classification and device index mapping |
| flexmeasures/data/models/planning/init.py | Adds device_inventory as derived scheduler state and deprecates _build_stock_groups in favor of the inventory |
| flexmeasures/data/models/generic_assets.py | Makes flex-context inheritance robust to None flex_context on ancestors |
| flexmeasures/conftest.py | Ensures charging-station asset IDs are assigned deterministically via a flush |
| documentation/changelog.rst | Adds infrastructure/support entry for the inventory and a bugfix entry for the alignment issues |
The fixed alignment bugs only affect flex-model shapes introduced during v1 development (entries without a top-level sensor were rejected outright in v0.33.1), so no released behavior regressed: reference the PR from the multiple-feeders feature entry instead of a separate bugfix entry. Signed-off-by: F.N. Claessen <felix@seita.nl>
Flix6x
commented
Jul 15, 2026
Flix6x
left a comment
Member
Author
There was a problem hiding this comment.
Requesting changes myself.
Address review comments on PR #2321: - Raise a ValidationError when more than one flex-model entry defines state-of-charge parameters for the same stock, instead of silently letting the last entry win (stricter than the historical behavior). - Reflow attribute comments to natural clause breaks, clarify that the flex_model None check is a precaution for inflexible devices, and note why inflexible devices always have a power sensor. - Word choice: scheduling features (not code sites) re-derived entry kinds; pass the sensor name as a keyword argument in the test helper. Signed-off-by: F.N. Claessen <felix@seita.nl>
This was referenced Jul 15, 2026
Closed
Flix6x
added a commit
that referenced
this pull request
Jul 16, 2026
Adapt the sensor-scoped commitment branch to the typed DeviceInventory introduced in PR #2321: resolve the scoped devices' canonical solver indices via inventory.by_sensor_id() instead of re-enumerating the raw flex-model entry list. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015qxM7UZ5wHTz3ftz1Mf9yy Signed-off-by: F.N. Claessen <felix@seita.nl>
Flix6x
added a commit
that referenced
this pull request
Jul 16, 2026
Adapt the CHP converter-port concept to the device inventory: FlexDevice entries carry their coupling name and signed coupling coefficient, and DeviceInventory.coupling_groups replaces Scheduler._build_coupling_groups as the single source of truth for the solver's coupling groups, keyed by canonical device indices. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015qxM7UZ5wHTz3ftz1Mf9yy Signed-off-by: F.N. Claessen <felix@seita.nl>
Flix6x
added a commit
that referenced
this pull request
Jul 16, 2026
…feat/internal-commodity-balance Balance groups now derive from the inventory's canonical commodity enumeration: the internal-node detection runs in the loop over inventory.commodity_to_devices, so each balance group lists the commodity's flexible and inflexible devices by their canonical solver indices. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015qxM7UZ5wHTz3ftz1Mf9yy Signed-off-by: F.N. Claessen <felix@seita.nl>
Flix6x
added a commit
that referenced
this pull request
Jul 16, 2026
Adapt the group feature to the typed DeviceInventory introduced in PR #2321: - Classify group entries in the inventory (new DeviceRole.GROUP), as the first classification step, so a group entry's own power sensor or output sensors cannot make it pass for a device. - Replace the scheduler's ad-hoc _group_models/_group_to_devices derivation with the inventory's group_entries field and group_to_devices resolution (transitive leaf-device resolution with cycle detection). - Drop this branch's own copy of the stock-only misalignment fix (device_sensor_id_to_index and the device_models zip fix), which main now solves by construction via the inventory's canonical device indices. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015qxM7UZ5wHTz3ftz1Mf9yy Signed-off-by: F.N. Claessen <felix@seita.nl>
Flix6x
added a commit
that referenced
this pull request
Jul 17, 2026
…oup (#2324) - storage-efficiency may now be defined on the entry holding a shared stock's SoC parameters (or on exactly one member device) and applies to all members; conflicting definitions fail fast, mirroring how #2321 treats the other SoC parameters. - device_scheduler now models one stock variable and one balance recursion per stock group instead of per device, dropping the redundant secondary recursions, and validates that grouped devices share their storage efficiency and initial stock. - Fix a latent key collision in device_to_group: an ungrouped device whose index equalled a state-of-charge sensor id was silently merged into that stock group, feeding its flows into the wrong stock. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jul 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Device tracking in the
StorageSchedulerwas index-based and fragile: every per-device property lived in a parallel list, each code site re-derived an entry's kind (device / stock-only) from the raw dicts, and a localflex_modelvariable silently flipped between the filtered and unfiltered entry list inside_prepare(). Several in-flight PRs (#2310, #2295, #2276, #2289) each add more entry kinds and more index-keyed bookkeeping on top of this, and two of them (#2276 and #2319) independently fixed the same misalignment bug.This PR sets a more robust basis for those works: a typed device inventory (
planning/devices.py) that classifies every flex-model entry (and the flex-context's inflexible devices) exactly once, right after flex-config deserialization, and serves as the single source of truth for:DEVICE/STOCK_ONLY/INFLEXIBLE, withGROUPandCONVERTER_PORTas documented extension points),The raw deserialized flex-model dicts are kept as-is on each
FlexDevice, so downstream code and new flex-model fields need no dataclass changes._prepare()'s return value anddevice_scheduler's interface are unchanged (linear_optimization.pyis untouched).Bugs fixed by construction
All of these are locked by new solver-level regression tests (added first as strict xfails, then un-xfailed by the fix commit):
{"consumption": {"sensor": N}}(or"production") was misclassified as stock-only and silently unscheduled — one of the two bugs in Fix device misalignment when flex-model entries are filtered from multi-device schedules #2319, which this PR subsumes.convert_to_commitmentsenumerated the unfiltered list, assigning commitments to stock-only entries (and to out-of-range device indices)._prepare(based on the full list length) and_build_stock_groups(based on the filtered list length) whenever a stock-only entry was present, sosoc-at-start/soc-min/… of devices without astate-of-chargesensor were silently dropped._prepare()and_reconstruct_commodity_to_devices()each maintained a ~40-line commodity/device-index enumeration that had to stay in sync byte for byte; both now read the inventory.Relation to in-flight PRs (rebase guide)
soc_value_at_endlist and per-deviceStockCommitmentslot into blocks this PR preserves.enumerate(flex_model)scan withinventory.by_sensor_id(sensor_id).DeviceRole.GROUPas the first classification step; itsdevice_sensor_id_to_indexmap isinventory.by_sensor_id, its_group_models/_group_to_devicesbecome inventory fields; its own copy of fix (1) drops out.commodityis already first-class onFlexDevice; converter ports extend it (coupling, coefficient or aCONVERTER_PORTrole), andcoupling_groups/balance_groupsderive from the inventory.How to test
The full planning suite (
flexmeasures/data/models/planning/tests/) passes; the four new regression tests fail onmain.🤖 Generated with Claude Code
https://claude.ai/code/session_01Fkpk8kwbjHreWaCJN4Mncp