diff --git a/doc/release_notes.rst b/doc/release_notes.rst index fc0d88f8..9210ff80 100644 --- a/doc/release_notes.rst +++ b/doc/release_notes.rst @@ -40,6 +40,14 @@ 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. + +* ``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/linopy/spec/accessor.py b/linopy/spec/accessor.py index bb588a2a..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) @@ -116,6 +118,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 +163,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/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_accessor.py b/test/test_spec_accessor.py index ed8457d5..91605cec 100644 --- a/test/test_spec_accessor.py +++ b/test/test_spec_accessor.py @@ -133,6 +133,30 @@ 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_( + 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"): 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( 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])))