From b683713672010bab7aca5b1aa60f33b0ce1f45e6 Mon Sep 17 00:00:00 2001 From: FBumann <117816358+FBumann@users.noreply.github.com> Date: Tue, 8 Sep 2026 17:11:09 +0200 Subject: [PATCH 1/4] fix(io): restamp only the containers built on the master coordinates `restamp_coords` puts a spec's master coordinates back on every container carrying a dimension of that name, so the whole model agrees on one dtype per dimension however the netcdf engine returned it. It asked only whether the dtypes differed, not whether the labels were the same -- and a spec-built model may hold hand-added variables of its own. So a variable added on `snapshot = [10, 11, 12]` beside a spec built on `[0, 1, 2]` came back relabelled to the spec's, silently; and one on a `snapshot` of another length failed the read outright with `conflicting sizes for dimension 'snapshot'`. The `CSRConstraint` branch replaced its `Grid` indexes unconditionally, without even the dtype guard. Both went unseen on netcdf4, which preserves the dtype; scipy narrowing int64 to int32 is what makes an index look stale. An index is now restamped only where it holds the master's own labels, which is what `Index.equals` asks -- it compares labels and not dtypes, so a narrowed int and a widened bool still match. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015ZfHfTFENUy6WxnFFri5Td --- doc/release_notes.rst | 2 ++ linopy/io.py | 33 +++++++++++++++++++++++++++------ test/test_spec_io.py | 18 ++++++++++++++++++ 3 files changed, 47 insertions(+), 6 deletions(-) diff --git a/doc/release_notes.rst b/doc/release_notes.rst index fc0d88f8..96edfce3 100644 --- a/doc/release_notes.rst +++ b/doc/release_notes.rst @@ -40,6 +40,8 @@ Upcoming Version *Other* +* ``read_netcdf`` no longer rewrites the coordinates of a container that merely shares a dimension's *name* with a spec-built model's master coordinates. A hand-added variable on its own labels kept them; before, it was silently relabelled onto the master ones, or the read failed outright when the two lengths differed. + * ``add_piecewise_formulation`` gained a ``mask`` parameter declaring which breakpoint slots hold a real breakpoint. It is needed for **ragged** curves — entities with different numbers of breakpoints — which are stored densely with the surplus slots left absent. Under v1 that absence must be declared (``mask=x_pts.notnull()``) rather than read off the NaN padding. (https://github.com/PyPSA/linopy/issues/884) *Internal* diff --git a/linopy/io.py b/linopy/io.py index 275cb2b7..22781750 100644 --- a/linopy/io.py +++ b/linopy/io.py @@ -1107,7 +1107,14 @@ def restore_dtypes(ds: xr.Dataset) -> xr.Dataset: def restamp_coords(m: Model, coords: Mapping[str, pd.Index]) -> None: - """Put *coords* on every container of *m* that carries one of those dimensions.""" + """ + Put *coords* on every container of *m* that was built on them. + + Only on those: a container may carry a dimension of that name and its own + labels -- a hand-added variable beside a spec-built one -- and restamping + it would rewrite labels it never had, or fail outright over a length the + master coordinate does not share. + """ from linopy.constraints import Constraint, CSRConstraint from linopy.csr import Grid @@ -1122,7 +1129,7 @@ def restamp_coords(m: Model, coords: Mapping[str, pd.Index]) -> None: elif isinstance(constraint, CSRConstraint): constraint._grid = Grid( { - d: coords.get(d, index) + d: _restamped(index, coords.get(str(d))) for d, index in constraint._grid.indexes.items() } ) @@ -1130,15 +1137,29 @@ def restamp_coords(m: Model, coords: Mapping[str, pd.Index]) -> None: def _stamped(data: xr.Dataset, coords: Mapping[str, pd.Index]) -> xr.Dataset: """*data* with *coords* in place of the ones a dtype narrowed.""" - indexes = data.indexes stale = { - dim: index - for dim, index in coords.items() - if dim in indexes and indexes[dim].dtype != index.dtype + str(dim): restamped + for dim, index in data.indexes.items() + if (restamped := _restamped(index, coords.get(str(dim)))) is not index } return data.assign_coords(stale) if stale else data +def _restamped(found: pd.Index, master: pd.Index | None) -> pd.Index: + """ + *master* where *found* is it as a netcdf type gave it back, else *found* itself. + + A narrowed int or a widened bool holds the same labels at another dtype + and is the one to replace -- which is what ``Index.equals`` asks, since it + compares labels and not dtypes. An index of another length, or of other + labels entirely, belongs to a container that was never built on *master* + and is left alone. + """ + if master is None or found.dtype == master.dtype: + return found + return master if found.equals(master) else found + + def to_netcdf(m: Model, *args: Any, **kwargs: Any) -> None: """ Write out the model to a netcdf file. diff --git a/test/test_spec_io.py b/test/test_spec_io.py index 30139a14..4dddf94e 100644 --- a/test/test_spec_io.py +++ b/test/test_spec_io.py @@ -190,6 +190,24 @@ def test_a_parameter_keeps_its_dtype(tmp_path: Path, engine: str, name: str) -> assert p.spec.parameters[name].dtype == m.spec.parameters[name].dtype +@pytest.mark.parametrize("engine", ENGINES) +@pytest.mark.parametrize( + "labels", [[10, 11, 12], [0, 1]], ids=["relabelled", "shorter"] +) +def test_a_hand_added_variable_keeps_its_own_labels( + tmp_path: Path, engine: str, labels: list[int] +) -> None: + """A container sharing a master dimension's name but not its labels is left alone.""" + own = pd.Index(labels, name="snapshot") + m = Model.from_spec(EXAMPLE_DISPATCH, DISPATCH_DATA, retain="all") + m.add_variables(coords=[own], name="side") + p = roundtrip(m, tmp_path, engine) + + assert p.variables["side"].indexes["snapshot"].equals(own) + assert p.spec.coords["snapshot"].equals(m.spec.coords["snapshot"]) + assert p.variables["p"].indexes["snapshot"].equals(m.spec.coords["snapshot"]) + + @pytest.mark.parametrize("engine", ENGINES) @pytest.mark.parametrize("frozen", [False, True], ids=["dataset", "csr"]) def test_every_container_shares_the_master_coordinate_dtypes( From ff54274e9433ad026548f0b2b4a378eeda1af35a Mon Sep 17 00:00:00 2001 From: FBumann <117816358+FBumann@users.noreply.github.com> Date: Tue, 8 Sep 2026 17:12:39 +0200 Subject: [PATCH 2/4] fix(spec): a lookup that maps nothing groups to the empty sum `grouped_sum` promises that "a group no member reaches holds the empty sum, which is 0 and not an absence", and delivers it by filtering the operand down to its mapped members and grouping what is left. Where a lookup maps no member at all, what is left is empty -- and an empty dimension is one xarray refuses to group over, so the promise came out as `ValueError: s must not be empty` from inside the groupby, on the fold and at build time alike. The empty case is now answered directly, the way `sum_over` already answers a term beside an empty dimension: zeros over the operand's remaining dimensions and every declared label of the group, as data or as a constant term. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015ZfHfTFENUy6WxnFFri5Td --- doc/release_notes.rst | 2 ++ linopy/spec/operators.py | 36 ++++++++++++++++++++++++++++++++++++ test/test_spec_operators.py | 23 +++++++++++++++++++++++ 3 files changed, 61 insertions(+) diff --git a/doc/release_notes.rst b/doc/release_notes.rst index 96edfce3..cbb8d25d 100644 --- a/doc/release_notes.rst +++ b/doc/release_notes.rst @@ -40,6 +40,8 @@ Upcoming Version *Other* +* A grouped sum through a lookup that maps no member at all now holds the empty sum, ``0``, on every declared group, as its documented rule says. It raised xarray's ``ValueError: must not be empty`` instead. + * ``read_netcdf`` no longer rewrites the coordinates of a container that merely shares a dimension's *name* with a spec-built model's master coordinates. A hand-added variable on its own labels kept them; before, it was silently relabelled onto the master ones, or the read failed outright when the two lengths differed. * ``add_piecewise_formulation`` gained a ``mask`` parameter declaring which breakpoint slots hold a real breakpoint. It is needed for **ragged** curves — entities with different numbers of breakpoints — which are stored densely with the surplus slots left absent. Under v1 that absence must be declared (``mask=x_pts.notnull()``) rather than read off the NaN padding. (https://github.com/PyPSA/linopy/issues/884) diff --git a/linopy/spec/operators.py b/linopy/spec/operators.py index 17fd2d46..7b86565a 100644 --- a/linopy/spec/operators.py +++ b/linopy/spec/operators.py @@ -81,6 +81,8 @@ def grouped_sum( mappings = _renamed(mappings, into) present = _present(mappings) dim = str(mappings[0].dims[0]) + if not bool(present.any()): + return _empty_groups(array, dim, into=into, labels=labels) if not bool(present.all()): keep = present.to_numpy() mappings = tuple(m.isel({dim: keep}) for m in mappings) @@ -92,6 +94,40 @@ def grouped_sum( return summed.reindex({d: labels[d] for d in into}).fillna(0.0) +def _empty_groups( + array: Array, + dim: str, + *, + into: tuple[str, ...], + labels: Mapping[str, pd.Index], +) -> Array: + """ + The grouped sum of an operand no member of *dim* is mapped out of. + + Every declared group holds the empty sum, which is 0. Grouping cannot say + so itself: filtering the operand down to its mapped members leaves nothing, + and an empty dimension is one xarray refuses to group over. + """ + kept = [d for d in _coord_dims(array) if d != dim] + zeros = xr.DataArray( + np.zeros([array.sizes[d] for d in kept] + [len(labels[d]) for d in into]), + coords={ + **{d: array.indexes[d] for d in kept}, + **{d: labels[d] for d in into}, + }, + dims=kept + list(into), + ) + if isinstance(array, xr.DataArray): + return zeros + return LinearExpression.from_constant(array.model, zeros) + + +def _coord_dims(array: Array) -> list[str]: + """The dimensions the operand is labelled over, without a term's own ``_term``.""" + dims = array.dims if isinstance(array, xr.DataArray) else array.coord_dims + return [str(d) for d in dims] + + @overload def at( array: xr.DataArray, mappings: tuple[xr.DataArray, ...], *, into: tuple[str, ...] diff --git a/test/test_spec_operators.py b/test/test_spec_operators.py index 4e470eed..bd9a1b45 100644 --- a/test/test_spec_operators.py +++ b/test/test_spec_operators.py @@ -202,6 +202,29 @@ def test_an_empty_group_on_the_constant_side_is_a_zero_and_not_a_gap() -> None: assert float(m.solution["imports"].sel(bus="south")) == pytest.approx(0.0) +def test_a_lookup_that_maps_nothing_leaves_every_group_at_the_empty_sum() -> None: + """Filtering to the mapped members leaves nothing, and nothing is what xarray will not group.""" + spec = with_( + GROUPED_SPEC, + variables={ + "out": { + "foreach": ["generator"], + "bounds": {"lower": 0, "upper": "capacity"}, + } + }, + expressions={"per_bus": "sum(out, by=gen_bus)"}, + ) + sources = grouped_sources(pd.Series([3.0, 4.0], index=GENS)) + sources["gen_bus"] = pd.Series([], dtype=object) + m = solved(spec, sources) + + assert m.objective.value == pytest.approx(0.0) + per_bus = m.spec.expressions["per_bus"] + assert per_bus.expression.nterm == 0 + assert per_bus.solution.indexes["bus"].tolist() == ["north", "south"] + np.testing.assert_allclose(per_bus.solution.values, [0.0, 0.0]) + + def test_a_member_with_no_value_is_still_refused_through_a_group() -> None: with pytest.raises(SpecDataError, match="parameter 'capacity' covers 1 fewer"): Model.from_spec(GROUPED_SPEC, grouped_sources(pd.Series([3.0], index=GENS[:1]))) From ebf73f0563d85d4cc6302f01fbc3a4c784004530 Mon Sep 17 00:00:00 2001 From: FBumann <117816358+FBumann@users.noreply.github.com> Date: Tue, 8 Sep 2026 17:13:40 +0200 Subject: [PATCH 3/4] fix(spec): repr a declared dimension nothing reaches `attach` lets a dimension the spec declares but no declaration reaches go without a source, deliberately, so it never lands in `coords`. The `ModelSpec` repr indexed `coords[d]` for every declared dimension anyway, so a model that built perfectly well could not be looked at: `repr(model.spec)` raised `KeyError` in a REPL or a notebook. Such a dimension is now shown as `unreached`. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015ZfHfTFENUy6WxnFFri5Td --- doc/release_notes.rst | 2 ++ linopy/spec/accessor.py | 7 ++++++- test/test_spec_accessor.py | 11 +++++++++++ 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/doc/release_notes.rst b/doc/release_notes.rst index cbb8d25d..9f4085a3 100644 --- a/doc/release_notes.rst +++ b/doc/release_notes.rst @@ -40,6 +40,8 @@ Upcoming Version *Other* +* ``repr(model.spec)`` no longer raises ``KeyError`` for a dimension the spec declares but nothing reaches, which needs no source and so has no coordinates; it is shown as ``unreached``. + * A grouped sum through a lookup that maps no member at all now holds the empty sum, ``0``, on every declared group, as its documented rule says. It raised xarray's ``ValueError: must not be empty`` instead. * ``read_netcdf`` no longer rewrites the coordinates of a container that merely shares a dimension's *name* with a spec-built model's master coordinates. A hand-added variable on its own labels kept them; before, it was silently relabelled onto the master ones, or the read failed outright when the two lengths differed. diff --git a/linopy/spec/accessor.py b/linopy/spec/accessor.py index bb588a2a..f9ab1817 100644 --- a/linopy/spec/accessor.py +++ b/linopy/spec/accessor.py @@ -116,6 +116,11 @@ def _source(spec: SpecLike) -> tuple[str, ms.Program]: return loaded.to_yaml(), to_program(loaded) +def _dimension(dim: str, coords: Mapping[str, pd.Index]) -> str: + """A dimension and how many labels it holds; a declared one nothing reaches holds none.""" + return f"{dim} ({len(coords[dim])})" if dim in coords else f"{dim} (unreached)" + + def _row(label: str, items: list[str], cap: int = 8) -> str: """One aligned summary line, capped with a ``(+N more)`` tail.""" shown = items[:cap] @@ -156,7 +161,7 @@ def __repr__(self) -> str: head = f"ModelSpec: {self.description}" if self.description else "ModelSpec" rows = [ head, - _row("Dimensions", [f"{d} ({len(coords[d])})" for d in p.dimensions]), + _row("Dimensions", [_dimension(d, coords) for d in p.dimensions]), _row("Variables", list(p.variables)), _row("Constraints", list(p.constraints)), ] diff --git a/test/test_spec_accessor.py b/test/test_spec_accessor.py index ed8457d5..eadf9907 100644 --- a/test/test_spec_accessor.py +++ b/test/test_spec_accessor.py @@ -133,6 +133,17 @@ def test_the_spec_keeps_its_parameters_off_the_model() -> None: assert m.spec.parameters["cost"].dims == ("generator",) +def test_a_declared_dimension_with_no_source_still_reprs() -> None: + """A dimension nothing reaches needs no source, so the repr must do without its labels.""" + spec = with_( + yaml_dict(), dimensions={"spare": {"dtype": "int", "description": "unreached"}} + ) + m = Model.from_spec(spec, DISPATCH_DATA) + + assert "spare (unreached)" in repr(m.spec) + assert "snapshot (3)" in repr(m.spec) + + def test_an_unknown_expression_is_a_key_error_with_a_hint() -> None: m = Model.from_spec(yaml_dict(), DISPATCH_DATA) with pytest.raises(KeyError, match="unknown named expression 'spent'.*spend"): From 435bb277a23983a05a83780ee4e313e864e0cb00 Mon Sep 17 00:00:00 2001 From: FBumann <117816358+FBumann@users.noreply.github.com> Date: Tue, 8 Sep 2026 17:14:36 +0200 Subject: [PATCH 4/4] fix(spec): resolve the retained parameters before building `retain="all"` reaches every declared parameter, including ones no declaration reads, and it did so after `build()` had already put the variables and constraints on the model. A parameter with no source therefore raised with the model half-built and `_spec` still unset -- and the corrected retry was then refused by the guard that `add_spec` builds into an empty model, so the only way on was a fresh model. The retained set is resolved first. It needs nothing the build produces, so the failure now lands before anything is added and the same model takes the corrected sources. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015ZfHfTFENUy6WxnFFri5Td --- doc/release_notes.rst | 2 ++ linopy/spec/accessor.py | 4 +++- test/test_spec_accessor.py | 13 +++++++++++++ 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/doc/release_notes.rst b/doc/release_notes.rst index 9f4085a3..9210ff80 100644 --- a/doc/release_notes.rst +++ b/doc/release_notes.rst @@ -40,6 +40,8 @@ Upcoming Version *Other* +* ``Model.add_spec`` resolves the parameters it retains before it builds. A ``retain="all"`` build that could not read a parameter no declaration uses raised after the variables and constraints were already added, leaving a model that the "builds into an empty model" guard then refused to build into again. + * ``repr(model.spec)`` no longer raises ``KeyError`` for a dimension the spec declares but nothing reaches, which needs no source and so has no coordinates; it is shown as ``unreached``. * A grouped sum through a lookup that maps no member at all now holds the empty sum, ``0``, on every declared group, as its documented rule says. It raised xarray's ``ValueError: must not be empty`` instead. diff --git a/linopy/spec/accessor.py b/linopy/spec/accessor.py index f9ab1817..b8cec579 100644 --- a/linopy/spec/accessor.py +++ b/linopy/spec/accessor.py @@ -84,8 +84,10 @@ def attach( ) text, program = _source(spec) attached: Attached = attach_data(program, sources, retain=retain) - build(model, attached) + # Resolved before the build, so a parameter no declaration reads cannot fail + # halfway through one and leave a model too full to build into again. parameters = attached.retained().assign_coords(dict(attached.coords)) + build(model, attached) return ModelSpec(model, program, text, parameters, attached) diff --git a/test/test_spec_accessor.py b/test/test_spec_accessor.py index eadf9907..91605cec 100644 --- a/test/test_spec_accessor.py +++ b/test/test_spec_accessor.py @@ -133,6 +133,19 @@ def test_the_spec_keeps_its_parameters_off_the_model() -> None: assert m.spec.parameters["cost"].dims == ("generator",) +def test_a_build_that_cannot_retain_leaves_the_model_buildable() -> None: + """retain='all' reaches parameters no declaration does, and must not half-build on one.""" + spec = with_(yaml_dict(), parameters={"spare": {"dims": ["generator"]}}) + m = Model() + with pytest.raises(SpecDataError, match="no data provided for parameter 'spare'"): + m.add_spec(spec, DISPATCH_DATA, retain="all") + assert not len(m.variables) and not len(m.constraints) + + spare = pd.Series([1.0, 2.0], index=GENERATOR) + m.add_spec(spec, {**DISPATCH_DATA, "spare": spare}, retain="all") + assert "spare" in m.spec.parameters + + def test_a_declared_dimension_with_no_source_still_reprs() -> None: """A dimension nothing reaches needs no source, so the repr must do without its labels.""" spec = with_(