Fix EMS-level flow commitment constraints - #2355
Merged
Flix6x merged 3 commits intoAug 3, 2026
Merged
Conversation
EMS-level commitments currently create an unbounded constraint tuple, leaving their deviation variables uncoupled from device flow. Mirror grouped commitments when selecting bounds and cover aggregate scheduling with a regression test. Signed-off-by: Sanjay Santhanam <51058514+Sanjays2402@users.noreply.github.com>
Documentation build overview
59 files changed ·
|
Flix6x
added a commit
that referenced
this pull request
Aug 3, 2026
…ce (fixes #2379) (#2380) * 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=<commodity's inventory indices>, device_group=commodity), mirroring the internal '<commodity> 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=<list>, so it does not go through that code. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TaMepbxRYzJxQKFtu6Hq4p Signed-off-by: F.N. Claessen <felix@seita.nl> * 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TaMepbxRYzJxQKFtu6Hq4p Signed-off-by: F.N. Claessen <felix@seita.nl> * 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 <felix@seita.nl> --------- Signed-off-by: F.N. Claessen <felix@seita.nl> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
The bug being fixed here (from PR FlexMeasures#1946) has not been released: it sits in the unreleased v1.0.0 section. A separate Bugfixes entry would tell readers about a regression they were never exposed to, so this appends PR FlexMeasures#2355 to the existing multi-commodity feature entry instead, as PR FlexMeasures#2380 did. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X2jLjsQzwztDc7Nxz1ozmQ Signed-off-by: F.N. Claessen <claessen@seita.nl>
Giving ems_flow_commitment_equalities bounds exposed an overlap with grouped_commitment_equalities that did not matter while the rows were free. PR FlexMeasures#2380, merged after this PR was opened, binds a regular flex-context commitment to the aggregate of its commodity's devices by giving it a "device" and "device_group". Such a commitment is therefore already bound, once per device group. Binding it here as well constrains the same commitment_downwards_deviation/commitment_upwards_deviation variables a second time, against a different device set, which over-constrains the problem: 9 device-, group- and stock-scoped commitment tests failed, including the one PR FlexMeasures#2380 added. Skip commitments that have a device group, so this constraint family applies only to commitments naming no device -- the EMS-level case this PR is about. The test added here still exercises that case, as an EMS-level FlowCommitment gets no device_group_lookup entry. Co-authored-by: Sanjay Santhanam <51058514+Sanjays2402@users.noreply.github.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X2jLjsQzwztDc7Nxz1ozmQ Signed-off-by: F.N. Claessen <claessen@seita.nl>
Member
|
Thanks for this, @Sanjays2402 — you found a real bug, and your fix is the right one. Those rows were built without bounds, so the constraint family never actually constrained anything. I've pushed two commits to your branch (thanks for leaving Allow edits by maintainers on). 1. Changelog entry folded into the multi-commodity one I moved your entry out of Bugfixes and appended this PR to the existing "The flex-context can now define multiple commodities…" feature entry. The reason: the bug came in with #1946, which is still sitting in the unreleased Worth knowing for next time: when a bug traces back to unreleased work, it's worth a quick word with the maintainers before opening a PR. Occasionally the better home for the fix is the branch that introduced it, and it can save you a rebase. 2. A follow-up fix — this one wasn't your doing #2380 merged a few hours after you opened this. It binds a regular flex-context commitment to the aggregate of its commodity's devices, by giving the commitment a Once your change gives So I added a skip for commitments that carry a device group, leaving this constraint family to the genuinely EMS-level case your test covers (an EMS-level if device_group_lookup.get(c):
return Constraint.SkipWith that, the full planning suite is green (280 passed, 3 xfailed), and your One thing to flag for context: #2364 adds a second, Pyomo-free scheduling backend and makes it the default. That backend currently skips this constraint family precisely because the rows were free, so it will need to build the row now that yours binds. That's our job over in #2364, not something for you to pick up here. Merging once CI is green. Thanks again — nice catch. |
Flix6x
approved these changes
Aug 3, 2026
Flix6x
added a commit
that referenced
this pull request
Aug 3, 2026
The dispatch to the direct HiGHS backend listed its keyword arguments by hand. That is a trap for the branches currently adding scheduling parameters: whoever adds the next one (coupling_groups in #2218, balance_groups in #2289) works on the Pyomo model further down the file, and a parameter missing from the dispatch list would not fail. It would simply never reach the backend, producing a schedule computed as if the constraint had never been requested -- and since this PR makes "highspy" the default solver, that would be silently wrong. Forward by name instead, mapping device_scheduler's signature onto the backend's. An argument the backend does not model raises NotImplementedError naming it, but only when the caller actually set it, so leaving a future parameter at its default stays free. The signature comparison is cached on the two function objects (~70 us once per process, 2 us per call after). Also record, in the highspy module docstring, that the deliberate omission of ems_flow_commitment_equalities stops being harmless once #2355 gives those rows bounds, and that #2380 routes unscoped flex-context commitments through grouped_commitment_equalities (which this backend does build). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X2jLjsQzwztDc7Nxz1ozmQ Signed-off-by: F.N. Claessen <claessen@seita.nl>
Flix6x
added a commit
that referenced
this pull request
Aug 3, 2026
The two backends build the same model in two representations, so the model construction is necessarily written twice. Everything around it was too: 199 lines were byte-identical between linear_optimization.py and highspy_optimization.py -- argument normalisation, stock groups and their validation, legacy commitment conversion, the sub-commitment split, device_group_lookup, the convex-curve check, the Big-Ms, band validation, the HiGHS option profile, and the cost/schedule assembly. None of it has a solver in it, and keeping it twice meant the two paths could drift apart on input handling, which the equivalence tests are not aimed at. Move it to a new scheduling_problem module: prepare_scheduling_problem() returns a SchedulingProblem that both backends unpack, plus solver_options() and the result-assembly helpers. Duplication between the backends drops from 199 to 40 lines, and those 40 are the shared signature and the call itself. Two deliberate changes while moving: - commodity_devices becomes a cached_property. It is a per-row scan that only the Pyomo ems_flow_commitment_equalities needs, so computing it eagerly would put a real cost on the fast path. Pyomo pays what it did before; the direct backend pays nothing until it needs it (see #2355). - initial_stock_of() casts its index to int, as the direct backend already did. The Pyomo version raised TypeError on the numpy float device indices a commitment's "device" column can carry. The empty-commitments case also stops raising on pd.concat([]), which previously made the Pyomo path crash where the direct path coped. Verified: 265 passed, 3 xfailed across the planning suite under all three solver parameters, plus the scheduling-job and API schedule tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X2jLjsQzwztDc7Nxz1ozmQ Signed-off-by: F.N. Claessen <claessen@seita.nl>
Flix6x
added a commit
that referenced
this pull request
Aug 3, 2026
This backend skipped ems_flow_commitment_equalities because the Pyomo path built those rows without bounds, so they could never bind and HiGHS dropped them anyway. PR #2355 gives them the same one-sided bounds that grouped_commitment_equalities uses, so they bind now, and skipping them here would silently ignore an EMS-level commitment under this backend -- which this PR makes the default. The row is the grouped one with a different summation set, so rather than write it twice, the existing loop's body is extracted into _active_rows (the commitment's active time steps and bounds) and _add_commitment_rows (bind a commitment to the summed flow or stock of a set of devices). The EMS-level loop then sums over every device, or over the commitment's commodity's devices, and mirrors the Pyomo path in skipping commitments that name a device group -- those are already bound per group, and binding them twice over-constrains the problem. Two equivalence scenarios cover this, so it runs under both backends: ems_level_flow_commitment (names no device, binds all devices) and ems_level_commodity_commitment (binds only its commodity's devices, which also exercises the commodity_devices lookup). Both were checked to fail with the new rows disabled, so they cannot pass vacuously. Also drops the now-inaccurate deviation note from the module docstring. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X2jLjsQzwztDc7Nxz1ozmQ Signed-off-by: F.N. Claessen <claessen@seita.nl>
Flix6x
added a commit
that referenced
this pull request
Aug 3, 2026
…n) (#2364) * feat: direct HiGHS (highspy) implementation of the device scheduler Add flexmeasures/data/models/planning/highspy_optimization.py, which builds the device scheduler's LP/MILP directly with the HiGHS Python API (highspy), bypassing Pyomo's model construction and solution-ingestion overhead. The Pyomo implementation (linear_optimization.device_scheduler) remains the semantic reference; the module docstring prominently documents that the two models must be kept in sync, as well as the (verified) deviations: - ems_flow_commitment_equalities are not built: on the Pyomo path they are bound-less, i.e. free rows without any effect on the solution - rows no finite assignment can satisfy (bounds involving +/-inf quantities) are skipped, mirroring HiGHS rejecting such rows when appsi adds them - solver results and model are lightweight shims exposing the attributes callers consume (termination_condition/status strings, commitment_costs, commodity_costs, costs, and indexed variable views) The model is built with vectorized numpy arrays (addVars/addRows/ changeColsCost/changeColsIntegrality), constructing and solving typical battery problems in milliseconds, where the Pyomo layer needs seconds. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WtuVTVfL4fQ9QSqbLXmAGD Signed-off-by: F.N. Claessen <claessen@seita.nl> * feat: expose the direct HiGHS backend as solver choice "highspy" When FLEXMEASURES_LP_SOLVER is set to "highspy", device_scheduler delegates to device_scheduler_highspy with the same inputs and the same return contract. All other solver names keep using the Pyomo path unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WtuVTVfL4fQ9QSqbLXmAGD Signed-off-by: F.N. Claessen <claessen@seita.nl> * tests: prove equivalence of the highspy and Pyomo scheduler backends - Add "highspy" to the app_with_each_solver fixture params, so tests using it also run against the direct backend. - Add test_highspy_equivalence.py, running representative scenarios (battery with prices; soc targets incl. storage efficiency and stock delta; site capacity with breach and peak prices; two devices with a StockCommitment; an infeasible case) through both appsi_highs and highspy, asserting near-identical schedules and costs, and matching termination handling. - Make test case 2 of test_multiple_devices_simultaneous_scheduler assert solver-independent properties (aggregate schedule, total costs, total unmet demand): the problem has multiple optima, and only the site-level schedule is unique, while the per-device slot allocation is an arbitrary tie-break that depends on the solver backend. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WtuVTVfL4fQ9QSqbLXmAGD Signed-off-by: F.N. Claessen <claessen@seita.nl> * feat: make the direct HiGHS backend the default LP solver Flip the FLEXMEASURES_LP_SOLVER default from "appsi_highs" to "highspy" and update the configuration, installation and deployment docs accordingly. Any Pyomo-based solver remains available as before. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WtuVTVfL4fQ9QSqbLXmAGD Signed-off-by: F.N. Claessen <claessen@seita.nl> * docs: point the changelog entry at the actual PR number Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WtuVTVfL4fQ9QSqbLXmAGD Signed-off-by: F.N. Claessen <claessen@seita.nl> * perf: vectorize the subcommitment conversion Hoist convert_commitments_to_subcommitments to module level (it is solver-agnostic and closure-free) and share it between both scheduler backends, removing the duplicated copy from the highspy module. Splitting a commitment into per-group subcommitments now uses a single groupby pass (in order of first appearance, like pd.unique) instead of filtering the DataFrame once per group, and the price non-uniqueness checks are vectorized across all groups. This removes a cost that scaled quadratically with the number of time steps (each time step often forms its own group), benefiting both backends: a 2-device, 192-step benchmark drops from 0.47s to 0.28s via appsi_highs and from 0.37s to 0.12s via highspy. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WtuVTVfL4fQ9QSqbLXmAGD Signed-off-by: F.N. Claessen <claessen@seita.nl> * review: address Copilot comments on solver output and solver docs - Do not disable HiGHS output unconditionally in the direct backend; only force output_flag false when LOGGING_LEVEL is "INFO", mirroring exactly how the Pyomo path builds its solver options profile, so verbose logging modes can still see solver output. Operator-configured FLEXMEASURES_LP_SOLVER_OPTIONS are still applied last and can override. - Clarify in the configuration docs that a separate solver installation is only needed for external solvers such as cbc; both HiGHS-based choices (highspy and appsi_highs) rely on the bundled highspy package. - Remove the now-inconsistent "pip install highspy" instruction from the deployment docs, which already state that highspy ships with FlexMeasures. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WtuVTVfL4fQ9QSqbLXmAGD Signed-off-by: F.N. Claessen <claessen@seita.nl> * review: remove dead commodity_devices lookup from the direct backend The commodity -> device indices lookup was only consumed by the Pyomo path's ems_flow_commitment_equalities, which the direct backend deliberately does not build (they are free rows). A breadcrumb comment keeps pointing readers at that documented skip. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WtuVTVfL4fQ9QSqbLXmAGD Signed-off-by: F.N. Claessen <claessen@seita.nl> * Forward device_scheduler arguments to the highspy backend by name The dispatch to the direct HiGHS backend listed its keyword arguments by hand. That is a trap for the branches currently adding scheduling parameters: whoever adds the next one (coupling_groups in #2218, balance_groups in #2289) works on the Pyomo model further down the file, and a parameter missing from the dispatch list would not fail. It would simply never reach the backend, producing a schedule computed as if the constraint had never been requested -- and since this PR makes "highspy" the default solver, that would be silently wrong. Forward by name instead, mapping device_scheduler's signature onto the backend's. An argument the backend does not model raises NotImplementedError naming it, but only when the caller actually set it, so leaving a future parameter at its default stays free. The signature comparison is cached on the two function objects (~70 us once per process, 2 us per call after). Also record, in the highspy module docstring, that the deliberate omission of ems_flow_commitment_equalities stops being harmless once #2355 gives those rows bounds, and that #2380 routes unscoped flex-context commitments through grouped_commitment_equalities (which this backend does build). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X2jLjsQzwztDc7Nxz1ozmQ Signed-off-by: F.N. Claessen <claessen@seita.nl> * refactor: share the scheduler's solver-agnostic input handling The two backends build the same model in two representations, so the model construction is necessarily written twice. Everything around it was too: 199 lines were byte-identical between linear_optimization.py and highspy_optimization.py -- argument normalisation, stock groups and their validation, legacy commitment conversion, the sub-commitment split, device_group_lookup, the convex-curve check, the Big-Ms, band validation, the HiGHS option profile, and the cost/schedule assembly. None of it has a solver in it, and keeping it twice meant the two paths could drift apart on input handling, which the equivalence tests are not aimed at. Move it to a new scheduling_problem module: prepare_scheduling_problem() returns a SchedulingProblem that both backends unpack, plus solver_options() and the result-assembly helpers. Duplication between the backends drops from 199 to 40 lines, and those 40 are the shared signature and the call itself. Two deliberate changes while moving: - commodity_devices becomes a cached_property. It is a per-row scan that only the Pyomo ems_flow_commitment_equalities needs, so computing it eagerly would put a real cost on the fast path. Pyomo pays what it did before; the direct backend pays nothing until it needs it (see #2355). - initial_stock_of() casts its index to int, as the direct backend already did. The Pyomo version raised TypeError on the numpy float device indices a commitment's "device" column can carry. The empty-commitments case also stops raising on pd.concat([]), which previously made the Pyomo path crash where the direct path coped. Verified: 265 passed, 3 xfailed across the planning suite under all three solver parameters, plus the scheduling-job and API schedule tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X2jLjsQzwztDc7Nxz1ozmQ Signed-off-by: F.N. Claessen <claessen@seita.nl> * perf: build device_group_lookup from column arrays Sub-commitments are usually one row each (every time step tends to form its own commitment group), so this loop slices a fresh two-column DataFrame, runs dropna() on it and calls iterrows() once per time step. The pandas per-call overhead, not the work, dominated: profiling a 4-device x 192-step problem showed 192 dropna() calls accounting for ~50 ms of a ~135 ms prepare_scheduling_problem, against a model build measured in single-digit milliseconds. Read the two columns as arrays once and loop over them instead, replacing dropna() with an explicit missing-value check that also handles the collection-valued "device" entries pd.isna would answer element-wise. The loop itself goes from 49 ms to 1.0 ms (device+device_group) and 65 ms to 0.8 ms (device only) at 192 sub-commitments; prepare_scheduling_problem as a whole drops from 135 ms to 22 ms. Equivalence was checked against the previous implementation over device-only and grouped frames, NaN/None/pd.NA in either column, list/tuple/ndarray device entries, mixed group key types, stock-scoped and empty frames: same groups, same members, same insertion order. Reading the column array yields numpy scalars where iterrows() yielded Python floats on mixed-dtype frames; the two are interchangeable as set members and dict keys (equal hash and equality) and both survive the int() casts applied downstream. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X2jLjsQzwztDc7Nxz1ozmQ Signed-off-by: F.N. Claessen <claessen@seita.nl> * Build the EMS-level flow commitment rows in the direct HiGHS backend This backend skipped ems_flow_commitment_equalities because the Pyomo path built those rows without bounds, so they could never bind and HiGHS dropped them anyway. PR #2355 gives them the same one-sided bounds that grouped_commitment_equalities uses, so they bind now, and skipping them here would silently ignore an EMS-level commitment under this backend -- which this PR makes the default. The row is the grouped one with a different summation set, so rather than write it twice, the existing loop's body is extracted into _active_rows (the commitment's active time steps and bounds) and _add_commitment_rows (bind a commitment to the summed flow or stock of a set of devices). The EMS-level loop then sums over every device, or over the commitment's commodity's devices, and mirrors the Pyomo path in skipping commitments that name a device group -- those are already bound per group, and binding them twice over-constrains the problem. Two equivalence scenarios cover this, so it runs under both backends: ems_level_flow_commitment (names no device, binds all devices) and ems_level_commodity_commitment (binds only its commodity's devices, which also exercises the commodity_devices lookup). Both were checked to fail with the new rows disabled, so they cannot pass vacuously. Also drops the now-inaccurate deviation note from the module docstring. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X2jLjsQzwztDc7Nxz1ozmQ Signed-off-by: F.N. Claessen <claessen@seita.nl> * review: name the solver-results shim after Pyomo's own class "Stanza" was an unhelpful coinage. The attribute this shim stands in for holds a pyomo.opt.results.solver.SolverInformation, so name it _SolverInformation and say where the name comes from. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X2jLjsQzwztDc7Nxz1ozmQ Signed-off-by: F.N. Claessen <claessen@seita.nl> * style: reflow docstrings and comments to break only after punctuation CLAUDE.md asks that docstrings and comments break lines only after punctuation, never mid-phrase, with max-line-length 160 and E501 ignored, so that review comments and text search stay stable. Copilot flagged nine places in this PR where I had not followed it, and it was right. Reflowed the docstrings and comments this PR adds, plus the ones it moved into the new scheduling_problem module. Doing that here rather than in the refactor commit keeps that commit verifiable as a verbatim move. Text only. No behaviour, names or logic changed; 290 passed, 3 xfailed across the planning suite under all three solvers. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X2jLjsQzwztDc7Nxz1ozmQ Signed-off-by: F.N. Claessen <claessen@seita.nl> --------- Signed-off-by: F.N. Claessen <claessen@seita.nl> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
3 tasks
Flix6x
added a commit
that referenced
this pull request
Aug 3, 2026
* Run the scheduler constraint tests under both backends Since #2364 the schedulers build the same model twice, once through Pyomo and once directly in HiGHS, so a test of scheduler behaviour only means something under one backend if the two agree -- which is the thing that cannot be assumed. Only test_solver.py was parametrized, so most constraint behaviour was verified under the configured default and no other. That is how a fix could pass on the Pyomo path and fail on the direct one, as happened on #2355. A module now opts in with RUN_UNDER_EACH_SOLVER = True and an autouse fixture in conftest does the switching, so no test signature changes. Enabled on test_group_constraints.py and test_operation_modes.py (27 tests, now 54). test_commitments.py and test_storage.py are NOT enabled, and the reason is worth recording: they build assets with fixed names, so running each test twice in one fixture scope violates generic_asset's unique-name constraint. Parametrizing them means making those fixtures unique per parameter first, which is a larger change than this one. Rather than leave that hole silent, test_solver_coverage.py asserts every planning test module either opts in or appears on an EXEMPT list with a reason, and that no EXEMPT entry is stale. A new module is then a decision someone makes on purpose instead of a gap nobody notices. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X2jLjsQzwztDc7Nxz1ozmQ Signed-off-by: F.N. Claessen <claessen@seita.nl> * review: show the exempt modules pass under the other backend too The exempt modules could not be parametrized in-process, but that left their behaviour under the non-default backend simply unknown, which is a weaker position than it needed to be. Adds a --lp-solver option pinning a whole run to one backend, so those modules can be run again under the other one. (An environment variable would not do: TestingConfig does not read FLEXMEASURES_LP_SOLVER.) Result: test_commitments.py, test_storage.py and test_process.py -- 68 tests -- all pass under appsi_highs, the non-default backend. So the exemption costs per-test granularity, not coverage. Checked the flag actually bites rather than silently doing nothing: running with --lp-solver=definitely_not_a_solver fails with Pyomo's UnknownSolver, so the option does reach solver selection. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X2jLjsQzwztDc7Nxz1ozmQ Signed-off-by: F.N. Claessen <claessen@seita.nl> * review: reflow this PR's docstrings and comments Every docstring and comment block added here broke lines mid-phrase, against the convention this repo documents and that #2384 had just restated -- "run once per / backend", "under one backend / if the two agree", "set from the / environment". Reflowed so each physical line ends after punctuation. Text only. Checked mechanically rather than by eye this time: inside a multi-line docstring or comment block, every line but the last must end in punctuation. The only remaining hits in the added lines are the two continuation lines of an example shell command, which end in a backslash by nature. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X2jLjsQzwztDc7Nxz1ozmQ Signed-off-by: F.N. Claessen <claessen@seita.nl> * review: drop the comment pointing at the PR description A code comment telling the reader to consult a pull request description ages badly and does not belong in the tree. Replaced with the fact it was pointing at: these modules pass under the other backend when a run is pinned with --lp-solver. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X2jLjsQzwztDc7Nxz1ozmQ Signed-off-by: F.N. Claessen <claessen@seita.nl> --------- Signed-off-by: F.N. Claessen <claessen@seita.nl> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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
2 participants
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.
Closes #2326
Description
documentation/changelog.rstLook & Feel
N/A
How to test
Run
pytest flexmeasures/data/models/planning/tests/test_commitments.py.Further Improvements
N/A
Related Items
Issue #2326
Sign-off