fix: support adding/removing variables after adding frozen constraint by koen-vg · Pull Request #806 · PyPSA/linopy

koen-vg · 2026-07-01T18:16:18Z

I like the new CSRConstraint and the memory benefits it comes with. The current implementation, however, breaks when you add or remove variables after adding a frozen constraint. This is because existing CSRConstraints record the number of active variables (matrix columns) in a shape parameter, which becomes stale when a new variable is added. When a variable (column) is removed, the CSR indices change and have to be rebuilt.

This PR makes it so CSR representations are updated when variables are added and removed. Especially in the case of adding variables, this is very low-impact, since the actual CSR data doesn't change; only the "shape" does.

I guess there is an argument to be made for a stricter kind of behavior: that no variables may be added or removed after any CSRConstraint has been added to the model. I believe that may be overly strict since the solution in this PR seems to work fine, but happy to discuss the alternative if anyone else has any insights. In any case, though, if the intention is that no variables may be added or removed when CSRConstaints are present, then this should be enforced properly at variable addition/removal. (Currently, you just get an obscure matrix alignment error when passing the model to a solver.)

Code and tests were written with the help of Claude Code.

Changes proposed in this Pull Request

Added a function to reshape CSR matrix returned by to_matrix in case any variables were added and matrix shape is stale.
Added a function to remap CSR columns for when a variable is removed.
Added tests for different orders of adding variables and CSRConstraints.

Checklist

AI-generated content is marked (see AGENTS.md).
Code changes are sufficiently documented; i.e. new functions contain docstrings and further explanations may be given in doc.
Unit tests for new features were added (if applicable).
A note for the release notes doc/release_notes.rst of the upcoming release is included.
I consent to the release of this PR's code under the MIT license.

codspeed-hq · 2026-07-01T18:19:41Z

Merging this PR will not alter performance

✅ 173 untouched benchmarks
⏩ 173 skipped benchmarks¹

_{Comparing koen-vg:fix/frozen-csr-active-vars (433847a) with master (c2607b6)}

173 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

Frozen (CSR-backed) constraints cache their matrix at freeze time, sized against the active-variable set as it was then. Adding variables afterwards (e.g. an auxiliary variable introduced by a constraint added after this one was frozen, common under Model(freeze_constraints=True) when building incrementally) grows the active-variable count, but CSRConstraint.to_matrix and to_matrix_with_rhs returned the cached CSR verbatim, ignoring the VariableLabelIndex passed in. The earlier-frozen blocks stay narrower than later ones, so stacking them fails with "ValueError: incompatible dimensions for axis 1". This breaks the invariant documented in PyPSA#630 -- to_matrix on both the mutable and frozen constraint "always returns a csr matrix of shape (n_active_cons, n_active_vars)" -- which the VariableLabelIndex exists precisely to uphold. Widen the cached CSR to the current n_active_vars in a shared _csr_for_index helper used by both to_matrix (matrix-accessor path) and to_matrix_with_rhs (direct-solve path). Variable positions are assigned in encounter order and are stable under additions, so widening only appends empty trailing columns. Adds differential tests (freeze vs mutable must agree) covering the baseline, variables added after freeze, and constraints linking later-added variables.

Frozen (CSR-backed) constraints store absolute dense variable positions from freeze time. Removing a variable renumbers the positions of every later variable, but Model.remove_variables left surviving frozen constraints with those stale positions -- both the wrong width and the wrong columns -- producing a corrupt constraint matrix (e.g. shape (2, 5) instead of (2, 2)) or an IndexError at solve time. The mutable Constraint path handles removal correctly, so this was a frozen-vs-mutable parity gap. remove_variables now captures the pre-removal variable ordering and, once the variable (and any constraints referencing it) are gone, remaps each surviving frozen constraint's CSR columns through variable labels via the new CSRConstraint._remap_columns. The old ordering is still available at the mutation site, so no per-constraint label storage is needed; a frozen constraint that referenced the removed variable has already been dropped, so every remaining column maps to a valid new position. Extends the differential tests with removal scenarios: first/middle-variable removal, straddling references, sequential removals, remove-then-add, and the drop-referencing-constraint parity case.

coroa

Oh, damn. I fucked up. I don't know anymore why i suggested active variables. I had an intermediate representation where i was going for the variables shape being mapped to the full xCounter range and then somehow ended up with changing to the active constraints first and then variables, but this remapping need is bad.

Thanks for seeing that @koen-vg .

I don't think the n.remove_variable is a good place for the remapping to happen. In principle you could also imagine variable mask changes (how do we do those?) to have the same effect! Should we have a short call how to fix this, @FabianHofmann ?

coroa · 2026-07-16T07:54:45Z

+        if frozen_cons:
+            assert old_vlabels is not None  # captured whenever frozen_cons is non-empty
+            new_label_index = self.variables.label_index
+            for con in frozen_cons:
+                con._remap_columns(old_vlabels, new_label_index)
+


Not the right place for the remapping, this will lead to spaghetti eventually.

coroa · 2026-07-16T08:17:24Z

I suggest that we hold something like a reference to the vlabels on the CSRConstraint and when accessing _csr we do:

if self.vlabels is self.model.variables.index.vlabels:
   return self._csr

csr = self._csr
if (self.vlabels != self.model.variables.index.vlabels[:self.shape[1]]).any():
   # more changes than additions, ie. remap
   csr = ...

csr = sp.sparse.csr_matrix(self._csr..., ..., shape=(self.shape[0], self.self.model.variables.index.vlabels[:]))
self._csr = csr

return csr

koen-vg · 2026-07-16T16:03:13Z

@coroa thanks for picking up this thread. I will admit that I was just trying to apply a simple fix that made my local modelling work, and wasn't too sure about the wider context and "morally correct" place to apply the fix.

Happy to update the PR along the lines you suggest though! I thought a little bit about it, and it seems to make sense. But I'm not super familiar with linopy internals. If you want to work on this yourself, feel free.

coroa · 2026-07-16T16:22:11Z

@coroa thanks for picking up this thread. I will admit that I was just trying to apply a simple fix that made my local modelling work, and wasn't too sure about the wider context and "morally correct" place to apply the fix.

Happy to update the PR along the lines you suggest though! I thought a little bit about it, and it seems to make sense. But I'm not super familiar with linopy internals. If you want to work on this yourself, feel free.

@FabianHofmann assigned the review to himself, so i was kinda hoping with the limited breadcrumbs i left, he'd whip up an amazing solution 😀

coroa · 2026-07-16T16:28:02Z

but feel free to give it a go yourself. the only additional comment i have would be that it probably makes sense to test two/three remapping strategies and/or think about whether there is an easy remapping cache strategy.

coroa · 2026-07-16T16:43:32Z

my intuition would be that something like

new_colinds = np.searchsorted(old_vlabels, vlabels)[colinds]

would be the most efficient remapping and even more so if you cache this intermediate

Replace the two site-specific fixes (widening in to_matrix, eager column remap in Model.remove_variables) with a single lazy reconciliation on the CSRConstraint itself. The frozen CSR records the active-variable basis (label_index.vlabels) its column indices refer to; every consumer that interprets those indices goes through _reconciled_csr(), which - returns the stored CSR unchanged while the basis object is unchanged (the common case, O(1)); - widens the shape when the stored basis is a prefix of the current one (variables were only added; positions are stable); - otherwise remaps each stored position through its variable label to the new position (variables were removed, possibly plus additions), raising a clear error if a still-referenced variable is gone. This covers all mutation paths (including ones that bypass Model.remove_variables) and all consumers - matrix assembly, .data reconstruction, repr, to_polars, iterate_slices, has_variable and netcdf export - without hooks in model.py. Also fixes repr of frozen constraints decoding CSR column positions as variable labels, which crashed or printed wrong variables for models with masked variables, and makes Constraints.reset_dual preserve the indicator data and basis of frozen constraints. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GcUKeYa1GvsxyMQsLgf7y1

koen-vg · 2026-07-17T20:32:59Z

Okay so this was my attempt at the suggestion solution. I think it holds up.

One comment, in the new implementation there is this ValueError:

+            referenced = old_vlabels[csr.indices]
+            new_positions = label_index.label_to_pos[referenced]
+            if (new_positions < 0).any():
+                missing = np.unique(referenced[new_positions < 0])
+                raise ValueError(
+                    f"Frozen constraint '{self._name}' references variables that "
+                    f"are no longer part of the model (labels {missing.tolist()}). "
+                    "Use `Model.remove_variables` to remove variables; it also "
+                    "removes the constraints referencing them."
+                )

Now, this isn't really supposed to happen, but if a user were to call Variables.remove (instead of Model.remove_variables), this could happen. It's probably a good idea to rename Variables.remove to Variables._remove (or delete the function entirely and inline it; it's only called from remove_variables right?) so nobody thinks this is a public function (even though it's not documented).

koen-vg added 2 commits July 1, 2026 11:27

koen-vg force-pushed the fix/frozen-csr-active-vars branch from b690853 to b2b622c Compare July 1, 2026 18:28

FabianHofmann self-requested a review July 13, 2026 11:10

coroa reviewed Jul 16, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: support adding/removing variables after adding frozen constraint#806

fix: support adding/removing variables after adding frozen constraint#806
koen-vg wants to merge 3 commits into
PyPSA:masterfrom
koen-vg:fix/frozen-csr-active-vars

koen-vg commented Jul 1, 2026

Uh oh!

codspeed-hq Bot commented Jul 1, 2026 •

edited

Loading

Uh oh!

coroa left a comment

Uh oh!

coroa Jul 16, 2026

Uh oh!

coroa commented Jul 16, 2026

Uh oh!

koen-vg commented Jul 16, 2026

Uh oh!

coroa commented Jul 16, 2026

Uh oh!

coroa commented Jul 16, 2026

Uh oh!

coroa commented Jul 16, 2026 •

edited

Loading

Uh oh!

koen-vg commented Jul 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

koen-vg commented Jul 1, 2026

Changes proposed in this Pull Request

Checklist

Uh oh!

codspeed-hq Bot commented Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will not alter performance

Footnotes

Uh oh!

coroa left a comment

Choose a reason for hiding this comment

Uh oh!

coroa Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

coroa commented Jul 16, 2026

Uh oh!

koen-vg commented Jul 16, 2026

Uh oh!

coroa commented Jul 16, 2026

Uh oh!

coroa commented Jul 16, 2026

Uh oh!

coroa commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

koen-vg commented Jul 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codspeed-hq Bot commented Jul 1, 2026 •

edited

Loading

coroa commented Jul 16, 2026 •

edited

Loading