Skip to content

Conversation

coroa
Copy link
Member

@coroa coroa commented Sep 11, 2025

Changes proposed in this Pull Request

For eventually deprecating sanitize_infinities, this PR proposes to only remove them during lp file generation, rather than removing them inplace.

This also aligns the behaviour of infinities across

  • m.solve() where they are removed (due to sanitize_infinities = True), and
  • m.to_file("...lp") where they are written down

@fneum @FabianHofmann If we agree this is sensible behaviour, i would:

  1. add tests to make sure this works as expected,
  2. implement them also in the matrix generation used for the direct api's, and
  3. add deprecation tags to the sanitize functions (or at least remove them from m.solve)

What do you think?

Checklist

  • 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.

@lkstrp
Copy link
Member

lkstrp commented Sep 12, 2025

I would rather lean towards inplace modifications, for the reason of not having another state of the same model. It could be confusing for users who investigate the LP file, since it's not mirroring their linopy representation.

Otherwise, what about moving it to a sanitize flag in to_file, which is off by default but turned on when running .solve?

@coroa
Copy link
Member Author

coroa commented Sep 12, 2025

I would rather sanitize on every export, ie default to sanitize = True, i think the consistency between m.solve() and m.to_file(...) is important. And I also don't think inifinitys or nans should appear in the constraints in an lp file.

I think we would have to rework how sanitize works at the moment, i find it honestly quite confusing:

import linopy
m = linopy.Model()
x = m.add_variables(lower=0, upper=inf, name='x')
m.add_constraints(2 * x <= inf, name="inf")
m.add_constraints(x >= 2, name="lower bound")

Then you have:

>>> m.constraints["inf"]
Constraint `inf constraint`
---------------------------
+2 x ≤ inf
>>> m.to_file("before-sanitize.lp")
>>> !cat before-sanitize
[...]
c0:
+2.0 x0
<= inf
[...]

includes it literally.

If i apply m.constraints.sanitize_infinities() now, the repr stays absolutely the same:

>>> m.constraints["inf"]
Constraint `inf constraint`
---------------------------
+2 x ≤ inf

No indication that something happened here (i don't even understand yet, why).

What happened was that m.constraints["inf"].labels got masked out to -1, but this is not shown in the repr (because it only shows vars, coeffs and rhs and anything that is shadowed by mask, labels is not taken into account there). Should we maybe use mask instead of labels for sanitization, then? or update the __repr__ function?

m.to_file("...") omits it then completely. m.constraints["inf"].to_polars() already returns an empty table.

@FabianHofmann FabianHofmann self-requested a review September 21, 2025 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants