From de9c7c1886d726e0863b17dae31aa62c3de75e81 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 19 Aug 2026 10:54:59 +0000 Subject: [PATCH 1/2] refactor(runner): drop the dead scalar carry-over branch, enforce the contract MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 5 of the problem builder carried two pinning paths: a windowed one for `initial_values` arrays with a `time` dimension, and a scalar fallback pinning only `var[time=0]`. The scalar path was compatibility with the array shape `SimulationSession._extract_carry_over` produced *before* multi-timestep stitching; since that producer now always keeps a `time` dimension, and it is the only producer in the tree, the branch is unreachable (confirmed by running the full suite with an assertion in its body). Remove it, and make the shape a checked precondition instead of a silent fallback: `_validate_initial_values` rejects a value without a `time` dimension at construction, before any phase runs, so a stale caller fails loudly rather than getting a single-timestep pin it did not ask for. Phase 5 is then a plain windowed pin with one guard. Also record the design decision the review settled on — time-independent variables (`structure.time = False`) are never carried over — in the phase comment, `_extract_carry_over`, and the user guide, alongside the clarification that the carry-over is plain variable fixing at matching absolute timesteps, not an initial-condition mechanism reaching across the block boundary. The builder docstring claimed 4 phases while Phase 5 already existed below it; it now lists 5. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011K1uKpkPN2sB5yo1yfeDXn --- docs/user-guide/optim-config.md | 18 ++++ src/gems_runner/session/session.py | 5 + src/gems_runner/simulation/optimization.py | 76 +++++++++------ .../test_sequential_carry_over_length.py | 92 +++++++++++++++++++ 4 files changed, 165 insertions(+), 26 deletions(-) diff --git a/docs/user-guide/optim-config.md b/docs/user-guide/optim-config.md index 25537693..8e5733f9 100644 --- a/docs/user-guide/optim-config.md +++ b/docs/user-guide/optim-config.md @@ -305,6 +305,24 @@ tooling decides which block's version of a shared timestep is authoritative; `carry-over-length` only controls how much two consecutive blocks may *disagree* on that shared window. +**What the carry-over pins.** The mechanism is plain *variable fixing*: for +block *N+1*, every time-dependent variable whose block-relative timestep falls +in `[0, carry-over-length[` is fixed to the value block *N* computed for the +**same absolute timestep**. Two consequences are worth spelling out: + +- It is **not** an initial-condition mechanism. Block *N+1*'s problem is not + given the value of the timestep *preceding* its window, so a `t-1` time-shift + operator at the block's first timestep still resolves against that block's own + border condition (cyclic by default) rather than reaching into block *N*. +- It applies to **all** time-dependent variables of all models, not only + state-like ones such as a storage level. Finer, per-model granularity can be + added later if a use case needs it. + +**Time-independent** variables (`structure.time = False`, e.g. an investment +capacity) are never carried over — nothing links their values across blocks, so +each block sizes them independently. Sequential mode is therefore not suited to +investment problems; use `frontal` or `benders-decomposition` for those. + ### `parallel-subproblems` diff --git a/src/gems_runner/session/session.py b/src/gems_runner/session/session.py index 8f6c622a..66e56918 100644 --- a/src/gems_runner/session/session.py +++ b/src/gems_runner/session/session.py @@ -257,6 +257,11 @@ def _extract_carry_over( block's variables. The window is clamped to the solved block's actual horizon (a truncated final block can be shorter than ``block_length``), so fewer than *length* values may be carried over. + + Only variables carrying a ``time`` dimension are extracted. + Time-independent variables (e.g. an investment capacity) are + deliberately left free in every block, so each block re-optimizes them + independently. """ carry_over: Dict[Tuple[str, str], xr.DataArray] = {} if length <= 0 or problem.linopy_model.solution is None: diff --git a/src/gems_runner/simulation/optimization.py b/src/gems_runner/simulation/optimization.py index fd297d4e..76bbc109 100644 --- a/src/gems_runner/simulation/optimization.py +++ b/src/gems_runner/simulation/optimization.py @@ -467,13 +467,39 @@ def get_variable_solution( # --------------------------------------------------------------------------- +def _validate_initial_values( + initial_values: Optional[Dict[Tuple[str, str], xr.DataArray]], +) -> Dict[Tuple[str, str], xr.DataArray]: + """Check the carry-over contract on *initial_values* and return them. + + Every value must carry a ``time`` dimension indexed ``0 .. k-1`` so that it + aligns with the leading timesteps of the block being built. + """ + values = initial_values or {} + for (mk, var_name), init_val in values.items(): + if "time" not in init_val.dims: + raise ValueError( + f"initial_values[{mk!r}, {var_name!r}] must carry a 'time' " + f"dimension indexed 0..k-1; got dims {tuple(init_val.dims)}" + ) + return values + + class _OptimizationProblemBuilder: """ - Builds the linopy problem in 4 phases: + Builds the linopy problem in 5 phases: 1. Build parameter DataArrays for all models. 2. Create all linopy Variables (uses param arrays for bounds). 3. Build port arrays via incidence matrices. 4. Add constraints and objectives to the linopy model. + 5. Add the carry-over constraints of *initial_values* (sequential mode): + each time-dependent variable is *fixed*, over the block's first ``k`` + timesteps, to the value the previous block computed for the same + absolute timestep. Time-independent variables are never pinned. + + ``initial_values`` maps ``(model_id, var_name)`` to an ``xr.DataArray`` + carrying a ``time`` dimension indexed ``0 .. k-1``; the shape is checked at + construction by :func:`_validate_initial_values`. """ def __init__( @@ -492,7 +518,7 @@ def __init__( self.scenario_ids = scenario_ids self._location_filter = location_filter self._oob_filter = oob_filter - self._initial_values = initial_values or {} + self._initial_values = _validate_initial_values(initial_values) self.block_length = len(block.timesteps) self.time_coord = list(range(self.block_length)) @@ -534,27 +560,24 @@ def build(self) -> OptimizationProblem: model, port_arrays_for_model, total_obj ) - # Phase 5: carry-over constraints (sequential mode only) + # Phase 5: carry-over constraints (sequential mode only). + # Only time-dependent variables are pinned: time-independent ones + # (structure.time = False, e.g. an investment capacity) are deliberately + # left free in every block — see the user guide, `sequential-subproblems`. for (mk, var_name), init_val in self._initial_values.items(): linopy_var = self.linopy_vars.get((mk, var_name)) - if linopy_var is not None and "time" in linopy_var.dims: - safe = f"{mk}__{var_name}".replace("-", "_") - if "time" in init_val.dims: - # Pin the first len(init_val.time) timesteps, clamped to - # this block's horizon (a truncated final block can be - # shorter than the carried window). - pin_length = min(init_val.sizes["time"], self.block_length) - self.linopy_model.add_constraints( - linopy_var.isel(time=slice(0, pin_length)) - == init_val.isel(time=slice(0, pin_length)), # type: ignore[arg-type] - name=f"carry_over__{safe}", - ) - else: - # Scalar (no time dim): legacy form, pin the first timestep. - self.linopy_model.add_constraints( - linopy_var.isel(time=0) == init_val, # type: ignore[arg-type] - name=f"carry_over__{safe}", - ) + if linopy_var is None or "time" not in linopy_var.dims: + continue + # Pin the first len(init_val.time) timesteps, clamped to this + # block's horizon (a truncated final block can be shorter than the + # carried window). + pin_length = min(init_val.sizes["time"], self.block_length) + safe = f"{mk}__{var_name}".replace("-", "_") + self.linopy_model.add_constraints( + linopy_var.isel(time=slice(0, pin_length)) + == init_val.isel(time=slice(0, pin_length)), # type: ignore[arg-type] + name=f"carry_over__{safe}", + ) # Extract constant objective contribution (linopy cannot hold pure constants). objective_constant = 0.0 @@ -1005,12 +1028,13 @@ def build_problem( Label for the linopy model. initial_values: Optional carry-over values keyed by ``(model_id, var_name)``. Each - value is an ``xr.DataArray``; when it carries a ``time`` dimension of - length ``k`` (indexed ``0 .. k-1``), constraints - ``var[time=i] == value[i]`` are added for the block's first ``k`` + value must be an ``xr.DataArray`` carrying a ``time`` dimension of + length ``k`` indexed ``0 .. k-1``; constraints + ``var[time=i] == value[i]`` are then added for the block's first ``k`` timesteps, overriding the cyclic border condition on that window. A - value without a ``time`` dimension pins only ``var[time=0]`` (legacy - single-timestep form). + value without a ``time`` dimension raises ``ValueError`` before the + problem is built. Entries whose variable is time-independent, or is + absent from this block, are ignored. """ study.check_consistency() diff --git a/tests/e2e/functional/test_sequential_carry_over_length.py b/tests/e2e/functional/test_sequential_carry_over_length.py index 28b3bfa6..34b14be8 100644 --- a/tests/e2e/functional/test_sequential_carry_over_length.py +++ b/tests/e2e/functional/test_sequential_carry_over_length.py @@ -41,8 +41,15 @@ import pandas as pd import pytest +import xarray as xr +from gems_craft.expression.expression import literal, param, var +from gems_craft.model import Constraint, float_parameter, float_variable, model +from gems_craft.study import ConstantData, DataBase, Study, System, create_component +from gems_runner.simulation import TimeBlock, build_problem +from gems_runner.simulation.optimization import _validate_initial_values from gems_runner.study.runner import run_study +from tests.e2e.functional.libs.standard import CONSTANT _STUDY_SRC = Path(__file__).parent / "studies" / "rolling_horizon_suboptimality" @@ -239,3 +246,88 @@ def test_zero_overlap_blocks_fully_independent(tmp_path: Path) -> None: f"sequential (block-overlap: 0) and parallel modes disagree at " f"t={t} for {component}.{output}: {v_seq} != {v_par}" ) + + +# --------------------------------------------------------------------------- +# `initial_values` contract, at the `build_problem` level +# +# The tests above are run-level: they observe the pinned values, not the +# constraints that produced them. These two build a problem directly to check +# *which* variables the carry-over pins, and that a malformed `initial_values` +# is rejected before anything is built. +# --------------------------------------------------------------------------- + + +def _one_time_dependent_one_constant_study() -> Study: + """A single-component study whose model has one time-dependent variable + (`gen`) and one time-independent one (`cap`).""" + plant = model( + id="PLANT", + parameters=[float_parameter("cost", CONSTANT)], + variables=[ + float_variable("gen", lower_bound=literal(0), upper_bound=literal(10)), + float_variable( + "cap", + lower_bound=literal(0), + upper_bound=literal(10), + structure=CONSTANT, + ), + ], + constraints=[ + Constraint(name="Max generation", expression=var("gen") <= var("cap")) + ], + objective_contributions={ + "operational": (param("cost") * var("gen")).time_sum().expec() + }, + ) + database = DataBase() + database.add_data("P", "cost", ConstantData(1)) + system = System("carry_over_contract") + system.add_component(create_component(model=plant, id="P")) + return Study(system, database) + + +def _time_da(values: list[float]) -> xr.DataArray: + """Carry-over array in the shape `_extract_carry_over` produces: a `time` + dimension indexed 0..k-1.""" + return xr.DataArray( + values, dims=["time"], coords={"time": list(range(len(values)))} + ) + + +def test_carry_over_skips_time_independent_variables() -> None: + """Only time-dependent variables are pinned. A time-independent variable + (`structure.time = False`) is left free in every block even when the caller + passes a value for it, so consecutive blocks size it independently.""" + problem = build_problem( + _one_time_dependent_one_constant_study(), + TimeBlock(1, [0, 1, 2]), + [0], + initial_values={ + ("PLANT", "gen"): _time_da([2.0, 3.0]), + ("PLANT", "cap"): _time_da([7.0]), + }, + ) + + constraint_names = set(problem.linopy_model.constraints) + assert "carry_over__PLANT__gen" in constraint_names + assert "carry_over__PLANT__cap" not in constraint_names + + +def test_initial_values_without_time_dim_rejected() -> None: + """A value with no `time` dimension — the shape carried over before + multi-timestep stitching existed — is rejected outright rather than + silently reinterpreted as a single-timestep pin.""" + with pytest.raises(ValueError, match="must carry a 'time' dimension"): + build_problem( + _one_time_dependent_one_constant_study(), + TimeBlock(1, [0, 1, 2]), + [0], + initial_values={("PLANT", "gen"): xr.DataArray(2.0)}, + ) + + # The check is a precondition on the argument, so it does not need a study: + with pytest.raises(ValueError, match="must carry a 'time' dimension"): + _validate_initial_values({("PLANT", "gen"): xr.DataArray(2.0)}) + + assert _validate_initial_values(None) == {} From f6cb0d9d568d3825854337668d5b99824befb0fd Mon Sep 17 00:00:00 2001 From: "Antoine Oustry, PhD" <58943406+aoustry@users.noreply.github.com> Date: Wed, 19 Aug 2026 15:48:42 +0200 Subject: [PATCH 2/2] Apply suggestion from @aoustry --- src/gems_runner/simulation/optimization.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/gems_runner/simulation/optimization.py b/src/gems_runner/simulation/optimization.py index 76bbc109..ab64789a 100644 --- a/src/gems_runner/simulation/optimization.py +++ b/src/gems_runner/simulation/optimization.py @@ -496,10 +496,6 @@ class _OptimizationProblemBuilder: each time-dependent variable is *fixed*, over the block's first ``k`` timesteps, to the value the previous block computed for the same absolute timestep. Time-independent variables are never pinned. - - ``initial_values`` maps ``(model_id, var_name)`` to an ``xr.DataArray`` - carrying a ``time`` dimension indexed ``0 .. k-1``; the shape is checked at - construction by :func:`_validate_initial_values`. """ def __init__(