From c5e15aac8c89217bcf660411b7e545f910d1a262 Mon Sep 17 00:00:00 2001 From: Fabian Date: Fri, 25 Sep 2026 07:26:39 +0200 Subject: [PATCH 1/3] fix(copy): keep frozen constraints frozen in Model.copy (#981) --- doc/release_notes.rst | 1 + linopy/io.py | 40 ++++++++++++++++++++++++++---------- test/test_csr.py | 48 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 78 insertions(+), 11 deletions(-) diff --git a/doc/release_notes.rst b/doc/release_notes.rst index d3ff4dd8..e7f4bdd7 100644 --- a/doc/release_notes.rst +++ b/doc/release_notes.rst @@ -81,6 +81,7 @@ Upcoming Version * ``CSRConstraint.loc``, ``update`` and ``from_rule``, and assigning to ``coeffs``, ``vars`` or ``sign``, now raise an ``AttributeError`` saying the operation is not supported on a frozen constraint and naming ``.mutable()`` as the way out (for ``from_rule``: build with ``Constraint.from_rule`` and ``.freeze()`` the result), like the read-only ``rhs``, ``lhs`` and ``scaling`` setters, instead of a bare ``AttributeError``. (`#963 `__) * ``linopy.merge`` with ``join="outer"``, ``"left"`` or ``"right"`` no longer raises an xarray ``MergeError`` when only one operand carries an auxiliary coordinate on a joined dimension. The constant was reindexed with a fill value of ``0`` that was also written into the auxiliary coordinate, so it no longer matched the coefficients' copy. The coordinate now follows its rows onto the joined grid and is NaN where no operand provides it, under both semantics. The same holds for ``.add`` / ``.sub`` / ``.mul`` / ``.div`` with a constant and a reindexing ``join=``. * A frozen constraint caches the label-to-position mapping of its matrix columns by weak reference and only rebuilds it when the constraint or the set of variables changes. While a persistent snapshot holds the arrays, repeated matrix assembly on an unchanged model returns the same objects, so the snapshot diff can again skip the comparison of untouched frozen constraints by object identity; one-off exports such as ``to_file`` retain no extra memory. (`#933 `__) +* ``Model.copy`` (and the ``copy.copy``/``copy.deepcopy`` protocols) keeps a frozen ``CSRConstraint`` frozen instead of rebuilding it as a dense ``Constraint``, with its matrix, grid, sign, rhs, scaling and slack; ``deep=False`` shares its buffers and duals are copied only with ``include_solution=True``. A CSR-backed objective is now deep-copied with ``deep=True`` as well. (`#981 `__) * ``Solver.close()`` no longer leaves dangling native handles behind. The solver model is now dropped before the environment that owns it, instead of after. And the COPT and MindOpt file interfaces no longer hand back a model they already disposed: after a file-based COPT or MindOpt solve, ``model.solver_model`` is ``None`` rather than a handle into freed memory. (`#899 `__) **Breaking Changes** diff --git a/linopy/io.py b/linopy/io.py index 718757e5..a89519cc 100644 --- a/linopy/io.py +++ b/linopy/io.py @@ -22,6 +22,7 @@ import numpy as np import pandas as pd import polars as pl +import scipy.sparse import xarray as xr from tqdm import tqdm @@ -1335,7 +1336,12 @@ def copy(m: Model, include_solution: bool = False, deep: bool = True) -> Model: Model A deep or shallow copy of the model. """ - from linopy.constraints import Constraint, ConstraintBase, Constraints + from linopy.constraints import ( + Constraint, + ConstraintBase, + Constraints, + CSRConstraint, + ) from linopy.expressions import Expressions, LinearExpression, QuadraticExpression from linopy.model import Model, Objective from linopy.variables import Variable, Variables @@ -1378,17 +1384,24 @@ def _copy_expr( new_model, ) - def _copy_con_data(con: ConstraintBase) -> xr.Dataset: - d = con.mutable().data - if include_solution: - return d.copy(deep=deep) - return d[con.data_attrs].copy(deep=deep) + def _buffer(value: Any) -> Any: + is_buffer = isinstance(value, np.ndarray | scipy.sparse.sparray) + return value.copy() if deep and is_buffer else value + + def _copy_con(name: str, con: ConstraintBase) -> ConstraintBase: + if isinstance(con, CSRConstraint): + kwargs = {k: _buffer(v) for k, v in con._init_kwargs().items()} + kwargs["model"] = new_model + if not include_solution: + kwargs["dual"] = None + return con._replace(**kwargs) + d = con.data + if not include_solution: + d = d[con.data_attrs] + return Constraint(d.copy(deep=deep), new_model, name) new_model._constraints = Constraints( - { - name: Constraint(_copy_con_data(con), new_model, name) - for name, con in m.constraints.items() - }, + {name: _copy_con(name, con) for name, con in m.constraints.items()}, new_model, ) @@ -1397,7 +1410,12 @@ def _copy_con_data(con: ConstraintBase) -> xr.Dataset: obj_expr = ( type(expr)(expr.data.copy(deep=deep), new_model) if csr is None - else LinearExpression._from_csr(replace(csr, model=new_model), new_model) + else LinearExpression._from_csr( + replace( + csr, csr=_buffer(csr.csr), const=_buffer(csr.const), model=new_model + ), + new_model, + ) ) new_model._objective = Objective( obj_expr, new_model, m.objective.sense, m.objective.scaling diff --git a/test/test_csr.py b/test/test_csr.py index d24e9a8a..7a30fcd8 100644 --- a/test/test_csr.py +++ b/test/test_csr.py @@ -2203,3 +2203,51 @@ def test_soften_rejects(error: str, freeze: bool) -> None: call, exc, match = SOFTEN_ERRORS[error] with pytest.raises(exc, match=match): call(con) + + +def frozen_model(soften: bool) -> tuple[Model, CSRConstraint]: + c = base_model() + for var in (c.gen_p, c.flow): + var.update(lower=0, upper=1) + with no_densify(): + c.m.add_objective((1.0 * c.gen_p).groupby(c.gbus).sum(sparse=True).sum()) + con = c.m.add_constraints( + c.balance_lhs(sparse=True), ">=", c.load, name="c", freeze=True + ) + if soften: + con.soften(penalty=2.0, max_violation=20.0) + assert isinstance(con, CSRConstraint) + return c.m, con + + +@pytest.mark.skipif("highs" not in linopy.available_solvers, reason="needs highs") +@pytest.mark.parametrize("include_solution", [True, False]) +@pytest.mark.parametrize("deep", [True, False]) +def test_copy_keeps_frozen_constraints(deep: bool, include_solution: bool) -> None: + require_v1() + m, con = frozen_model(soften=True) + m.solve("highs") + with no_densify(): + c = m.copy(deep=deep, include_solution=include_solution) + copied = c.constraints["c"] + assert isinstance(copied, CSRConstraint) + assert copied.model is c and copied.name == "c" + assert c.objective.expression.is_sparse + assert np.shares_memory(con.active_positions, copied.active_positions) != deep + assert_frozen_equal(con, copied) + assert con.slack is not None and copied.slack is not None + assert copied.slack.positive.labels.equals(con.slack.positive.labels) + assert copied.slack.positive.model is c + assert ("dual" in copied.mutable().data) == include_solution + + +@pytest.mark.parametrize("deep", [True, False]) +def test_softening_frozen_copy_leaves_original(deep: bool) -> None: + require_v1() + m, con = frozen_model(soften=False) + with no_densify(): + copied = m.copy(deep=deep) + copied.constraints["c"].soften(penalty=2.0) + assert con.slack is None and "c_slack_pos" not in m.variables + assert m.objective.expression.nterm < copied.objective.expression.nterm + assert con.nterm < copied.constraints["c"].nterm From 5188f9b1ba482cbd98b84b127392b4da635fcbe2 Mon Sep 17 00:00:00 2001 From: Fabian Date: Fri, 25 Sep 2026 07:39:06 +0200 Subject: [PATCH 2/3] fix(copy): read-only grid aux coords, semantics-independent copy test --- doc/release_notes.rst | 11 +---------- linopy/csr.py | 14 ++++++++++++++ linopy/io.py | 22 ++++++++++++++-------- test/test_csr.py | 38 +++++++++++++++++++++++++++++++++++--- 4 files changed, 64 insertions(+), 21 deletions(-) diff --git a/doc/release_notes.rst b/doc/release_notes.rst index e7f4bdd7..660e97bf 100644 --- a/doc/release_notes.rst +++ b/doc/release_notes.rst @@ -72,16 +72,7 @@ Upcoming Version * ``sum()`` over a dimension no longer raises when another dimension of the expression has size 0; it returns an expression without terms over the kept coordinates, as summing over the empty dimension itself already did. (https://github.com/PyPSA/linopy/issues/906) * A multi-key ``groupby`` now returns its groups sorted by key tuple, like the single-key path. The key combinations were numbered by iterating a ``set``, so the group order was arbitrary and changed between processes with ``PYTHONHASHSEED``. * The ``linopy.options`` context manager now restores the values that were active on entry instead of resetting all options to their defaults. -* ``Model.copy`` (and the ``copy.copy``/``copy.deepcopy`` protocols) no longer downgrades a quadratic objective to a linear one. The objective was rebuilt as a ``LinearExpression`` regardless of its type, so the copy silently solved a different problem. ``linopy.testing.assert_model_equal`` now compares the objective by expression type as well, which it could not do before. (`#903 `__) -* ``Model.to_netcdf``/``linopy.read_netcdf`` downgraded a quadratic objective the same way; the expression type is now stored alongside the objective and restored on read. Files written by earlier versions are read as before. (`#903 `__) -* ``linopy.read_netcdf`` now restores variables, expressions and constraints in their original insertion order instead of alphabetically, so ``matrices.A`` of a round-tripped model is no longer a row/column permutation of the original. The order is stored in the file; files written by earlier versions still load in sorted order. ``linopy.testing.assert_model_equal`` now also compares container order. (`#934 `__) -* Adding or removing variables after a constraint was added with ``freeze=True`` no longer breaks ``model.matrices``. The frozen constraint stored dense variable positions as its matrix columns, so blocks frozen at different times disagreed on their width and stacking them raised ``ValueError: inconsistent shapes``. Raw variable labels are stored instead and mapped to positions when the matrix is assembled. Frozen constraints in netCDF files written by earlier versions are read as before. (`#926 `__) -* A ``CSRConstraint`` built from a CSR-backed expression now keeps its auxiliary coordinates, e.g. the key levels of a multi-key ``groupby(..., observed=True).sum(sparse=True)`` or a scalar coordinate left by a scalar selection, so its ``coords`` equal those of the dense constraint. They are kept by ``to_dense()``/``mutable()`` and written to and read from netcdf, and ``Constraint.freeze()`` keeps them as well. (`#941 `__) -* ``linopy.read_netcdf`` no longer fails for a whole model when a frozen constraint has a dimension or MultiIndex level whose name contains ``-``: the coordinate metadata of a ``CSRConstraint`` is now stored under positional variable names with the names as attributes. Files written by earlier versions are read as before. -* ``CSRConstraint.loc``, ``update`` and ``from_rule``, and assigning to ``coeffs``, ``vars`` or ``sign``, now raise an ``AttributeError`` saying the operation is not supported on a frozen constraint and naming ``.mutable()`` as the way out (for ``from_rule``: build with ``Constraint.from_rule`` and ``.freeze()`` the result), like the read-only ``rhs``, ``lhs`` and ``scaling`` setters, instead of a bare ``AttributeError``. (`#963 `__) -* ``linopy.merge`` with ``join="outer"``, ``"left"`` or ``"right"`` no longer raises an xarray ``MergeError`` when only one operand carries an auxiliary coordinate on a joined dimension. The constant was reindexed with a fill value of ``0`` that was also written into the auxiliary coordinate, so it no longer matched the coefficients' copy. The coordinate now follows its rows onto the joined grid and is NaN where no operand provides it, under both semantics. The same holds for ``.add`` / ``.sub`` / ``.mul`` / ``.div`` with a constant and a reindexing ``join=``. -* A frozen constraint caches the label-to-position mapping of its matrix columns by weak reference and only rebuilds it when the constraint or the set of variables changes. While a persistent snapshot holds the arrays, repeated matrix assembly on an unchanged model returns the same objects, so the snapshot diff can again skip the comparison of untouched frozen constraints by object identity; one-off exports such as ``to_file`` retain no extra memory. (`#933 `__) -* ``Model.copy`` (and the ``copy.copy``/``copy.deepcopy`` protocols) keeps a frozen ``CSRConstraint`` frozen instead of rebuilding it as a dense ``Constraint``, with its matrix, grid, sign, rhs, scaling and slack; ``deep=False`` shares its buffers and duals are copied only with ``include_solution=True``. A CSR-backed objective is now deep-copied with ``deep=True`` as well. (`#981 `__) +* ``Model.copy`` (and the ``copy.copy``/``copy.deepcopy`` protocols) keeps frozen constraints as ``CSRConstraint`` instead of rebuilding them as dense ``Constraint``; ``deep=True`` copies their arrays, and duals are copied only with ``include_solution=True``. A CSR-backed objective is now deep-copied with ``deep=True`` as well. The auxiliary coordinates of a CSR grid are stored read-only, so a copy can share them safely. (`#981 `__) * ``Solver.close()`` no longer leaves dangling native handles behind. The solver model is now dropped before the environment that owns it, instead of after. And the COPT and MindOpt file interfaces no longer hand back a model they already disposed: after a file-based COPT or MindOpt solve, ``model.solver_model`` is ``None`` rather than a handle into freed memory. (`#899 `__) **Breaking Changes** diff --git a/linopy/csr.py b/linopy/csr.py index ea5d0da7..38901a36 100644 --- a/linopy/csr.py +++ b/linopy/csr.py @@ -77,6 +77,11 @@ class Grid: indexes: dict[str, pd.Index] aux: AuxCoords = field(default_factory=dict) + def __post_init__(self) -> None: + object.__setattr__( + self, "aux", {n: (d, _readonly(v)) for n, (d, v) in self.aux.items()} + ) + @classmethod def from_coords(cls, coords: Iterable[pd.Index]) -> Grid: """Build from one index per dimension, each named after its dim.""" @@ -749,6 +754,15 @@ def _densify_notice(reason: str) -> None: ) +def _readonly(values: np.ndarray) -> np.ndarray: + """``values`` as a read-only array, copied first if it is writable.""" + if not values.flags.writeable: + return values + values = values.copy() + values.setflags(write=False) + return values + + def _aux_coords(ds: Dataset | DataArray, dims: set[str]) -> AuxCoords: """Scalar and one-dimensional auxiliary coordinates of ``ds`` lying on ``dims``.""" aux: AuxCoords = {} diff --git a/linopy/io.py b/linopy/io.py index a89519cc..4d008979 100644 --- a/linopy/io.py +++ b/linopy/io.py @@ -17,7 +17,7 @@ from io import BufferedWriter from pathlib import Path from tempfile import TemporaryDirectory -from typing import TYPE_CHECKING, Any +from typing import TYPE_CHECKING, Any, TypeVar import numpy as np import pandas as pd @@ -35,6 +35,8 @@ from linopy.objective import Objective, linear_part from linopy.scaling import constraint_scaling_lookup, variable_scaling_lookup +Buffer = TypeVar("Buffer", np.ndarray, scipy.sparse.sparray) + if TYPE_CHECKING: from cuopt.linear_programming import DataModel as cuoptDataModel from cupdlpx import Model as cupdlpxModel @@ -1384,17 +1386,21 @@ def _copy_expr( new_model, ) - def _buffer(value: Any) -> Any: - is_buffer = isinstance(value, np.ndarray | scipy.sparse.sparray) - return value.copy() if deep and is_buffer else value + def _buffer(value: Buffer) -> Buffer: + return value.copy() if deep else value def _copy_con(name: str, con: ConstraintBase) -> ConstraintBase: if isinstance(con, CSRConstraint): - kwargs = {k: _buffer(v) for k, v in con._init_kwargs().items()} - kwargs["model"] = new_model + buffer_types = (np.ndarray, scipy.sparse.sparray) + changes: dict[str, Any] = {"model": new_model} + if deep: + kwargs = con._init_kwargs().items() + changes |= { + k: v.copy() for k, v in kwargs if isinstance(v, buffer_types) + } if not include_solution: - kwargs["dual"] = None - return con._replace(**kwargs) + changes["dual"] = None + return con._replace(**changes) d = con.data if not include_solution: d = d[con.data_attrs] diff --git a/test/test_csr.py b/test/test_csr.py index 7a30fcd8..b38ed35c 100644 --- a/test/test_csr.py +++ b/test/test_csr.py @@ -2230,11 +2230,8 @@ def test_copy_keeps_frozen_constraints(deep: bool, include_solution: bool) -> No with no_densify(): c = m.copy(deep=deep, include_solution=include_solution) copied = c.constraints["c"] - assert isinstance(copied, CSRConstraint) assert copied.model is c and copied.name == "c" assert c.objective.expression.is_sparse - assert np.shares_memory(con.active_positions, copied.active_positions) != deep - assert_frozen_equal(con, copied) assert con.slack is not None and copied.slack is not None assert copied.slack.positive.labels.equals(con.slack.positive.labels) assert copied.slack.positive.model is c @@ -2251,3 +2248,38 @@ def test_softening_frozen_copy_leaves_original(deep: bool) -> None: assert con.slack is None and "c_slack_pos" not in m.variables assert m.objective.expression.nterm < copied.objective.expression.nterm assert con.nterm < copied.constraints["c"].nterm + + +@pytest.mark.parametrize("deep", [True, False]) +def test_copy_frozen_matrices(deep: bool) -> None: + m = Model(freeze_constraints=True) + i = pd.RangeIndex(4, name="i") + x = m.add_variables(lower=0, coords=[i], name="x") + b = m.add_variables(coords=[i], binary=True, name="b") + lhs = (2 * x).assign_coords(aux=("i", [1.0, 2, 3, 4])) + mask = xr.DataArray([True, False, True, True], coords=[i]) + scaling = xr.DataArray([1.0, 2, 3, 4], coords=[i]) + m.add_constraints(lhs >= 1, name="c", mask=mask, scaling=scaling) + m.add_indicator_constraints(b, 1, x <= 5, name="ind") + c = m.copy(deep=deep) + for name, con in m.constraints.items(): + copied = c.constraints[name] + assert isinstance(con, CSRConstraint) and isinstance(copied, CSRConstraint) + assert np.shares_memory(con._csr.data, copied._csr.data) != deep + got, want = c.matrices, m.matrices + assert got.A is not None and want.A is not None + assert got.indicator_A is not None and want.indicator_A is not None + pairs = [ + (got.A.toarray(), want.A.toarray()), + (got.b, want.b), + (got.sense, want.sense), + (got.clabels, want.clabels), + (got.indicator_A.toarray(), want.indicator_A.toarray()), + (got.indicator_b, want.indicator_b), + (got.indicator_binvar, want.indicator_binvar), + ] + for g, w in pairs: + assert np.array_equal(g, w) + with pytest.raises(ValueError, match="read-only"): + c.constraints["c"].coords["aux"].values[0] = -1 + assert c.constraints["c"].coords["aux"].equals(m.constraints["c"].coords["aux"]) From d8db6108a5b64ef2bd647792c97bbce649a3a417 Mon Sep 17 00:00:00 2001 From: Fabian Date: Fri, 25 Sep 2026 07:39:35 +0200 Subject: [PATCH 3/3] docs: restore release notes dropped in copy fix --- doc/release_notes.rst | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/doc/release_notes.rst b/doc/release_notes.rst index 660e97bf..dccad9b8 100644 --- a/doc/release_notes.rst +++ b/doc/release_notes.rst @@ -72,6 +72,15 @@ Upcoming Version * ``sum()`` over a dimension no longer raises when another dimension of the expression has size 0; it returns an expression without terms over the kept coordinates, as summing over the empty dimension itself already did. (https://github.com/PyPSA/linopy/issues/906) * A multi-key ``groupby`` now returns its groups sorted by key tuple, like the single-key path. The key combinations were numbered by iterating a ``set``, so the group order was arbitrary and changed between processes with ``PYTHONHASHSEED``. * The ``linopy.options`` context manager now restores the values that were active on entry instead of resetting all options to their defaults. +* ``Model.copy`` (and the ``copy.copy``/``copy.deepcopy`` protocols) no longer downgrades a quadratic objective to a linear one. The objective was rebuilt as a ``LinearExpression`` regardless of its type, so the copy silently solved a different problem. ``linopy.testing.assert_model_equal`` now compares the objective by expression type as well, which it could not do before. (`#903 `__) +* ``Model.to_netcdf``/``linopy.read_netcdf`` downgraded a quadratic objective the same way; the expression type is now stored alongside the objective and restored on read. Files written by earlier versions are read as before. (`#903 `__) +* ``linopy.read_netcdf`` now restores variables, expressions and constraints in their original insertion order instead of alphabetically, so ``matrices.A`` of a round-tripped model is no longer a row/column permutation of the original. The order is stored in the file; files written by earlier versions still load in sorted order. ``linopy.testing.assert_model_equal`` now also compares container order. (`#934 `__) +* Adding or removing variables after a constraint was added with ``freeze=True`` no longer breaks ``model.matrices``. The frozen constraint stored dense variable positions as its matrix columns, so blocks frozen at different times disagreed on their width and stacking them raised ``ValueError: inconsistent shapes``. Raw variable labels are stored instead and mapped to positions when the matrix is assembled. Frozen constraints in netCDF files written by earlier versions are read as before. (`#926 `__) +* A ``CSRConstraint`` built from a CSR-backed expression now keeps its auxiliary coordinates, e.g. the key levels of a multi-key ``groupby(..., observed=True).sum(sparse=True)`` or a scalar coordinate left by a scalar selection, so its ``coords`` equal those of the dense constraint. They are kept by ``to_dense()``/``mutable()`` and written to and read from netcdf, and ``Constraint.freeze()`` keeps them as well. (`#941 `__) +* ``linopy.read_netcdf`` no longer fails for a whole model when a frozen constraint has a dimension or MultiIndex level whose name contains ``-``: the coordinate metadata of a ``CSRConstraint`` is now stored under positional variable names with the names as attributes. Files written by earlier versions are read as before. +* ``CSRConstraint.loc``, ``update`` and ``from_rule``, and assigning to ``coeffs``, ``vars`` or ``sign``, now raise an ``AttributeError`` saying the operation is not supported on a frozen constraint and naming ``.mutable()`` as the way out (for ``from_rule``: build with ``Constraint.from_rule`` and ``.freeze()`` the result), like the read-only ``rhs``, ``lhs`` and ``scaling`` setters, instead of a bare ``AttributeError``. (`#963 `__) +* ``linopy.merge`` with ``join="outer"``, ``"left"`` or ``"right"`` no longer raises an xarray ``MergeError`` when only one operand carries an auxiliary coordinate on a joined dimension. The constant was reindexed with a fill value of ``0`` that was also written into the auxiliary coordinate, so it no longer matched the coefficients' copy. The coordinate now follows its rows onto the joined grid and is NaN where no operand provides it, under both semantics. The same holds for ``.add`` / ``.sub`` / ``.mul`` / ``.div`` with a constant and a reindexing ``join=``. +* A frozen constraint caches the label-to-position mapping of its matrix columns by weak reference and only rebuilds it when the constraint or the set of variables changes. While a persistent snapshot holds the arrays, repeated matrix assembly on an unchanged model returns the same objects, so the snapshot diff can again skip the comparison of untouched frozen constraints by object identity; one-off exports such as ``to_file`` retain no extra memory. (`#933 `__) * ``Model.copy`` (and the ``copy.copy``/``copy.deepcopy`` protocols) keeps frozen constraints as ``CSRConstraint`` instead of rebuilding them as dense ``Constraint``; ``deep=True`` copies their arrays, and duals are copied only with ``include_solution=True``. A CSR-backed objective is now deep-copied with ``deep=True`` as well. The auxiliary coordinates of a CSR grid are stored read-only, so a copy can share them safely. (`#981 `__) * ``Solver.close()`` no longer leaves dangling native handles behind. The solver model is now dropped before the environment that owns it, instead of after. And the COPT and MindOpt file interfaces no longer hand back a model they already disposed: after a file-based COPT or MindOpt solve, ``model.solver_model`` is ``None`` rather than a handle into freed memory. (`#899 `__)