linopy
linopy copied to clipboard
fix: filter out unbounded rhs in lp-polars
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 tosanitize_infinities = True), andm.to_file("...lp")where they are written down
@fneum @FabianHofmann If we agree this is sensible behaviour, i would:
- add tests to make sure this works as expected,
- implement them also in the matrix generation used for the direct api's, and
- 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.rstof the upcoming release is included. - [ ] I consent to the release of this PR's code under the MIT license.
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?
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.