Skip to content

Bind a regular commitment to its commodity's aggregate, not each device (fixes #2379) - #2380

Merged
Flix6x merged 3 commits into
mainfrom
fix/2379-commitment-commodity-aggregate
Aug 3, 2026
Merged

Bind a regular commitment to its commodity's aggregate, not each device (fixes #2379)#2380
Flix6x merged 3 commits into
mainfrom
fix/2379-commitment-commodity-aggregate

Conversation

@Flix6x

@Flix6x Flix6x commented Aug 3, 2026

Copy link
Copy Markdown
Member

Description

Fixes #2379: a regular (unscoped) flex-context commitment binds each device individually against its baseline, instead of the aggregate flow of all its commodity's devices. Regression from #1946 (multi-commodity), where convert_to_commitments went from one EMS-level FlowCommitment(**spec) (device=None, aggregate) to a for d in enumerate(flex_model): device=d loop (per-device).

Fix

  • In convert_to_commitments, bind one FlowCommitment per commitment over all of its commodity's devices: device=<the commodity's device indices>, device_group=<commodity> — mirroring the internal "<commodity> net energy" commitment.
  • Take those indices from self.device_inventory.commodity_to_devices (canonical, and including the commodity's inflexible devices), instead of re-enumerating the raw flex_model list — the positional-index reuse was a second latent bug.
  • Drop the now-unused flex_model parameter of convert_to_commitments.

Tests

  • Updated the direct-convert unit tests to set device_inventory and assert the aggregate device set (they previously used one device per commodity, which couldn't distinguish per-device from aggregate).
  • Added test_unscoped_commitment_binds_commodity_aggregate: two 8 MW heaters under a 10 MW baseline reach a combined 10 MW — a level neither could carry alone, and impossible under per-device binding.

Changelog

Appended this PR to the existing multi-commodity changelog entry (the one that introduced the regression), rather than adding a new entry.

Relation to #2355 / #2326

Distinct bug. #2326/#2355 is about the solver's ems_flow_commitment_equalities returning an unbounded (None, expr, None) for EMS-level (device=None) commitments — affecting direct device_scheduler callers. This PR fixes the StorageScheduler's per-device conversion, and its commitments use device=<list> (the grouped_commitment_equalities path), so they don't go through the device=None code #2355 fixes. The two are complementary. Heads-up: both touch test_commitments.py, so whichever merges second will need a small rebase there (#2355 is currently CONFLICTING anyway).

How to test

pytest flexmeasures/data/models/planning/tests/test_commitments.py

🤖 Generated with Claude Code

… 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>
@read-the-docs-community

read-the-docs-community Bot commented Aug 3, 2026

Copy link
Copy Markdown

Documentation build overview

📚 flexmeasures | 🛠️ Build #33886635 | 📁 Comparing cf180db against latest (fecf13a)

  🔍 Preview build  

3 files changed
± changelog.html
± _autosummary/flexmeasures.data.models.planning.storage.html
± api/v3_0.html

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>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes regression #2379 where an unscoped (regular) flex-context commitment was incorrectly bound per-device instead of binding the aggregate flow across all devices of the commitment’s commodity, aligning user commitments with the scheduler’s internal “<commodity> net energy” aggregate commitment behavior.

Changes:

  • Update StorageScheduler.convert_to_commitments to create one aggregate FlowCommitment per unscoped commitment, binding the commodity’s full device set via self.device_inventory.commodity_to_devices (and drop the now-unused flex_model parameter).
  • Adjust unit tests to initialize device_inventory explicitly for direct convert_to_commitments calls and assert against aggregate device sets.
  • Add regression test covering a two-device same-commodity scenario that distinguishes aggregate binding from per-device binding.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
flexmeasures/data/models/planning/storage.py Fix unscoped commitment conversion to bind commodity-aggregate devices using canonical device inventory indices; remove flex_model parameter.
flexmeasures/data/models/planning/tests/test_commitments.py Update direct-conversion tests to set device_inventory, and add regression coverage ensuring unscoped commitments bind the commodity aggregate.

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>
@Flix6x

Flix6x commented Aug 3, 2026

Copy link
Copy Markdown
Member Author

CI caught two existing tests that encoded the pre-fix per-device commitment behaviour; updated both to the aggregate semantics this PR restores (cf180dbe4):

  • test_flex_context_commitments_target_devices_not_stock_only_entries — a regular commitment now produces one aggregate commitment binding the scheduled devices (indices 0 and 1), instead of one per raw flex-model entry. The stock-only exclusion it guards against is unchanged.
  • test_create_simultaneous_jobs — its fixture's "sample commitment rewarding supply" is commented as not expected to affect the schedule. Under per-device binding it wrongly rewarded the battery's supply and biased the EV/battery split; bound to the site aggregate it stays inactive while the site is net-consuming, so total cost is unchanged and only the EV/battery split shifts (EV €2.3125 → €2.2375). The fix makes that fixture comment finally true.

Both verified locally before pushing.

@Flix6x Flix6x self-assigned this Aug 3, 2026
@Flix6x Flix6x added bug Something isn't working Scheduling labels Aug 3, 2026
Comment on lines +137 to +141
# 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.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Finally we understand why these shares kept changing.

@Flix6x Flix6x added this to the 1.0.0 milestone Aug 3, 2026
@Flix6x
Flix6x merged commit 5f2cb5e into main Aug 3, 2026
13 checks passed
@Flix6x
Flix6x deleted the fix/2379-commitment-commodity-aggregate branch August 3, 2026 11:29
Flix6x added a commit to Sanjays2402/flexmeasures that referenced this pull request Aug 3, 2026
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>
Flix6x added a commit to Sanjays2402/flexmeasures that referenced this pull request Aug 3, 2026
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>
Flix6x added a commit that referenced this pull request Aug 3, 2026
* fix(scheduling): bind EMS-level flow commitments

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>

* Do not bind a device-scoped commitment at EMS level as well

Giving ems_flow_commitment_equalities bounds exposed an overlap with
grouped_commitment_equalities that did not matter while the rows were free.

PR #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 #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>

---------

Signed-off-by: Sanjay Santhanam <51058514+Sanjays2402@users.noreply.github.com>
Signed-off-by: F.N. Claessen <claessen@seita.nl>
Co-authored-by: F.N. Claessen <claessen@seita.nl>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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
…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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working Scheduling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Regular (unscoped) commitments bind each device individually instead of the commodity aggregate (regression from #1946)

2 participants