refactor(solver): validation, sanitize kwargs, and result wiring on Solver path by FBumann · Pull Request #691 · PyPSA/linopy

FBumann · 2026-05-18T14:58:11Z

Stacked on #690. Closes the gap between Model.solve() and Solver.from_name(...).solve().

Core principle

Solver.from_model() does not mutate the model. Model-state transformations are Model-level primitives, called explicitly:

model.constraints.sanitize_zeros()        # drop zero-coefficient terms
model.constraints.sanitize_infinities()   # drop trivial <= +inf / >= -inf rows
model.apply_sos_reformulation()           # rewrite SOS as binary + linear (#690)
model.undo_sos_reformulation()            # restore the original SOS form (#690)

Model.solve() calls these internally as orchestration. The low-level Solver path expects the caller to apply them — no silent mutation on build.

Canonical low-level pattern:

model.constraints.sanitize_zeros()   # optional preprocessing
solver = Solver.from_name("highs", model, io_api="direct")
result = solver.solve()
model.assign_result(result, solver=solver)

What lives where after this PR

Port? = should it move to Solver. Status = ✅ done · ⬜ todo · — n/a.

Item	Port?	Status	Where (on `Solver` or why not)
Quadratic / semi-continuous feature gates	yes	✅	`Solver._validate_model()` (build-time, called from `_build()`)
SOS feature gate	yes	✅ (#690)	`Solver._validate_model()` (build-time)
Empty-objective check	yes	✅	`Solver.solve()` (run-time); scoped dup in `Model.solve()`'s remote branch, removed by #683
Result wiring (`model.solver` + primal/dual/status)	yes	✅	`Model.assign_result(result, solver=...)` (post-solve)
Build-time kwargs: `io_api`, `explicit_coordinate_names`, `set_names`, `problem_fn`, `slice_size`, `progress`, `**solver_options`	yes	✅	`Solver.from_model(...)`
Run-time kwargs: `basis_fn`, `warmstart_fn`	yes	✅	`Solver.solve(...)`
`env`, `log_fn`	yes	⬜	accepted on both `from_model()` and `solve()` today; PR3 consolidates to `from_model()` only (13 subclasses)
`sanitize_zeros` / `sanitize_infinities`	no	—	Model-level: `model.constraints.sanitize_*()`. Mutates — caller-owned.
SOS apply/undo lifecycle	no	—	Model-level: `model.apply_sos_reformulation()` / `undo_sos_reformulation()` (#690). Mutates — caller-owned.
`reformulate_sos` (`True`/`False`/`"auto"`)	no	—	Model.solve()-side convenience wrapping `model.apply_sos_reformulation()`. Solver-path users call apply/undo directly.
OETC remote (`remote=OetcHandler(...)`)	yes	⬜ (#683)	refactor into a `Solver` subclass
Generic `remote=RemoteHandler(...)`	?	—	could follow #683
`mock_solve=True`	?	—	could become a `Mock` `Solver` subclass
"Any solver installed" raise	?	—	`Solver.__post_init__` covers "this-installed"; "any-installed" is module-level
`solver_name` auto-pick, default `solution_fn`, `keep_files` cleanup, info logs, `reset_solution()`	no	—	orchestration: belongs above the Solver layer

Docs follow-up. Keeping sanitize off the Solver kwargs makes it less discoverable for users coming from Model.solve(). The user guide / example notebooks should surface the model-level primitives — separate doc PR #685, not in scope here.

Make Solver.from_name(...).solve() a real first-class entry point that doesn't lose Model.solve()'s safety nets: - Lift solver-feature gates into Solver._build() via a new _validate_model() hook: quadratic models against LP-only solvers and semi-continuous variables against solvers that don't support them. Removed the duplicate checks from Model.solve(). - Add sanitize_zeros / sanitize_infinities kwargs to Solver.from_model() (default True). The kwargs are processed in _build() before dispatch, so both file and direct io_apis honor them. Model.solve() forwards the kwargs through instead of pre-mutating the constraints itself. - Extend Model.assign_result(result, solver=None) so the Solver-path canonical pattern works: solver = Solver.from_name(...); result = solver.solve(); model.assign_result(result, solver=solver). When the solver kwarg is provided, model.solver gets wired the same way Model.solve() wires it, so compute_infeasibilities() and friends keep working through the low-level API. The empty-objective check stays on Model.solve() — to_gurobipy() / to_highspy() and similar build-only converters legitimately work against objectiveless models (gurobi/highs default to a zero objective), so the check belongs at the actual submit point. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

The empty-objective UX guardrail was previously only on Model.solve(), leaving the lower-level Solver.from_name(...).solve() path with a silent gap. Move it to Solver.solve() — the actual submit primitive that both entry points go through — so the same check fires regardless of which API the user reaches for. Build-time translate-only paths (to_gurobipy(), to_highspy(), to_file()) are unaffected since they don't call solve(). The cost of catching the error after build instead of before is bounded and only hits a programming-error case. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Consolidate the Model.solve() and Solver.from_name(...).solve() tests into one parametrized case — same check, two callers, one assertion. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Same property tested twice — no need for separate test IDs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

The remote-solve branch in Model.solve() short-circuits to a RemoteHandler before reaching Solver.solve(), so the check now in Solver.solve() doesn't cover it. Restore the early raise in Model.solve() so behavior is unchanged for all Model.solve() callers (mock, remote, local) while Solver.solve() still covers direct-Solver callers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

The early-position check was a workaround: the remote branch short-circuits before Solver.solve() (where the canonical check now lives), so empty-objective with remote=... wouldn't raise. Moving it into the remote branch itself makes the intent local to where it's needed, with a comment pointing at #683 where this duplication disappears once OETC becomes a Solver subclass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Remove the sanitize_zeros / sanitize_infinities kwargs from Solver.from_model(). The Solver builder now never mutates the model. Sanitization is exposed where it has always lived — model.constraints.sanitize_zeros() / .sanitize_infinities() — and Model.solve() calls them inline as part of its orchestration. Rationale: model-state transformations should be Model-level primitives (matches the SOS reformulation pattern from #690). The Solver's job is to translate the model and run; it should not silently change the caller's model on the way in. Users who go through the lower-level Solver path apply sanitize explicitly when they want it. Replaces TestSanitizeKwargs with TestSolverDoesNotMutateModel, pinning the mutation-free invariant: building a Solver against a model with a near-zero coefficient leaves model.constraints["c"].coeffs unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…r-from-model-options # Conflicts: # linopy/solvers.py

FabianHofmann

this is extremely sensible! I did not think about model mutations in the solver refactor pr. thanks for addressing this. let me know when this is ready to review

FabianHofmann · 2026-05-18T15:58:16Z

+        Passing ``solver=`` to :meth:`Model.assign_result` wires
+        ``model.solver`` so post-solve helpers like
+        :meth:`Model.compute_infeasibilities` keep working.


very good catch!

FabianHofmann · 2026-05-18T15:58:22Z

+    @pytest.mark.skipif(
+        "highs" not in solvers.available_solvers, reason="HiGHS not installed"
+    )


we definitely need a helper function that makes these skips snappy

FBumann · 2026-05-18T16:12:21Z

this is extremely sensible! I did not think about model mutations in the solver refactor pr. thanks for addressing this. let me know when this is ready to review

It is ready, but i think its less about correctness, but more about the conceptual split between Model and Solver, and the ergonomics for users using the new Solver class

* refactor(sos): add Model.apply/undo_sos_reformulation methods Introduce a stateful pair of methods on Model that own the SOS reformulation lifecycle: - apply_sos_reformulation() stashes the reformulation token on the model (new _sos_reformulation_state attribute). Raises if already applied. - undo_sos_reformulation() reads the stashed token and restores the original SOS form. No-op if nothing is applied. Model.solve(reformulate_sos=...) now delegates to these methods rather than threading the token through local state. The Solver path (which was previously raising via Model.solve's pre-flight check) now gets a clean ValueError directly from Solver._build() when an SOS-bearing model is handed to a solver without native SOS support — making the low-level API safe to use independently of Model.solve. Persistence: - copy() (and copy.copy / copy.deepcopy) carry the reformulation token with a deepcopy, so the copy is independently undoable. - to_netcdf() raises if a reformulation is active; users must undo first to serialize a stable model state. Context: motivated by the same investigation as #688 — while reviewing the new Solver.from_model() API surface introduced by #682, the SOS reformulation lifecycle stood out as load-bearing orchestration that the Solver path couldn't reproduce. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(solver): validation, sanitize kwargs, and result wiring on Solver path (#691) * refactor(solver): lift feature checks + sanitize/wiring to Solver path Make Solver.from_name(...).solve() a real first-class entry point that doesn't lose Model.solve()'s safety nets: - Lift solver-feature gates into Solver._build() via a new _validate_model() hook: quadratic models against LP-only solvers and semi-continuous variables against solvers that don't support them. Removed the duplicate checks from Model.solve(). - Add sanitize_zeros / sanitize_infinities kwargs to Solver.from_model() (default True). The kwargs are processed in _build() before dispatch, so both file and direct io_apis honor them. Model.solve() forwards the kwargs through instead of pre-mutating the constraints itself. - Extend Model.assign_result(result, solver=None) so the Solver-path canonical pattern works: solver = Solver.from_name(...); result = solver.solve(); model.assign_result(result, solver=solver). When the solver kwarg is provided, model.solver gets wired the same way Model.solve() wires it, so compute_infeasibilities() and friends keep working through the low-level API. The empty-objective check stays on Model.solve() — to_gurobipy() / to_highspy() and similar build-only converters legitimately work against objectiveless models (gurobi/highs default to a zero objective), so the check belongs at the actual submit point. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * move empty-objective check to Solver.solve() for entry-point parity The empty-objective UX guardrail was previously only on Model.solve(), leaving the lower-level Solver.from_name(...).solve() path with a silent gap. Move it to Solver.solve() — the actual submit primitive that both entry points go through — so the same check fires regardless of which API the user reaches for. Build-time translate-only paths (to_gurobipy(), to_highspy(), to_file()) are unaffected since they don't call solve(). The cost of catching the error after build instead of before is bounded and only hits a programming-error case. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test: parametrize empty-objective check across both entry points Consolidate the Model.solve() and Solver.from_name(...).solve() tests into one parametrized case — same check, two callers, one assertion. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test: collapse parametrize to a single test with two raises blocks Same property tested twice — no need for separate test IDs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * preserve empty-objective check for remote-solve path in Model.solve() The remote-solve branch in Model.solve() short-circuits to a RemoteHandler before reaching Solver.solve(), so the check now in Solver.solve() doesn't cover it. Restore the early raise in Model.solve() so behavior is unchanged for all Model.solve() callers (mock, remote, local) while Solver.solve() still covers direct-Solver callers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * move remote-path empty-objective check inside the remote branch The early-position check was a workaround: the remote branch short-circuits before Solver.solve() (where the canonical check now lives), so empty-objective with remote=... wouldn't raise. Moving it into the remote branch itself makes the intent local to where it's needed, with a comment pointing at #683 where this duplication disappears once OETC becomes a Solver subclass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * keep sanitize on Model; Solver.from_model() stays mutation-free Remove the sanitize_zeros / sanitize_infinities kwargs from Solver.from_model(). The Solver builder now never mutates the model. Sanitization is exposed where it has always lived — model.constraints.sanitize_zeros() / .sanitize_infinities() — and Model.solve() calls them inline as part of its orchestration. Rationale: model-state transformations should be Model-level primitives (matches the SOS reformulation pattern from #690). The Solver's job is to translate the model and run; it should not silently change the caller's model on the way in. Users who go through the lower-level Solver path apply sanitize explicitly when they want it. Replaces TestSanitizeKwargs with TestSolverDoesNotMutateModel, pinning the mutation-free invariant: building a Solver against a model with a near-zero coefficient leaves model.constraints["c"].coeffs unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * address review: SOS hint, lp_only_solver fixture, assign_result doc --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Fabian <fab.hof@gmx.de> * refactor(sos): tighten undo semantics and error hints - undo_sos_reformulation() now raises if no state is applied (fail-fast) - to_netcdf error no longer suggests poking the private state slot - Solver._build runs _validate_model before _check_sos_unmasked so SOS on an LP-only solver surfaces the reformulate-first hint - reformulate_sos_constraints docstring points at the stateful API * fix(sos): auto-undo SOS reformulation when build/solve raises `Model.solve(reformulate_sos=...)` left `_sos_reformulation_state` set if `Solver.from_name`, `solver.solve`, or the file-cleanup `finally` raised, since the undo lived in a second `try` around `assign_result` that those failures never reached. The next solve then hit `RuntimeError: SOS reformulation has already been applied`. Wrap sanitize, build/solve, file cleanup, and assign_result in a single outer try/finally so the undo always runs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(sos): support reformulation through remote/oetc netcdf path `to_netcdf` previously raised when the model had an active SOS reformulation, blocking `Model.solve(remote=...)` for users who passed `reformulate_sos`. Beyond the raise, the remote branch was silently ignoring `reformulate_sos` entirely. Changes: - `to_netcdf` warns (UserWarning) instead of raising; the reformulated MILP form is what gets serialized. The `_sos_reformulation_state` token is not persisted — it lives only on the in-memory caller's Model, where the apply/undo bracket keeps its lifecycle intact. - `Model.solve(remote=...)` now brackets the remote dispatch with `apply_sos_reformulation` / `undo_sos_reformulation`, exactly like the local path. The `to_netcdf` warning emitted inside the remote helper is suppressed via `warnings.catch_warnings`. - New `Model._resolve_sos_reformulation(solver_name, reformulate_sos)` helper deduplicates the should-reformulate decision between the local and remote branches and uses `solver_supports(...)` instead of the ad-hoc `getattr(solvers, SolverName(...).name)` pattern. - `solver_name=None` with `reformulate_sos="auto"` now raises a sharp error pointing users at either passing `solver_name=...` or using `True`/`False` to skip the lookup. The local path is unaffected because its existing default (`solver_name = available_solvers[0]`) runs before the helper sees None. Addresses the open thread on #690 from FabianHofmann. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(sos): fix mypy errors on remote-bracket and resolve tests - Drop now-unused type: ignore on _resolve_sos_reformulation call where mypy correctly narrows (True, False, "auto") to bool | Literal["auto"]. - Type _fake_handler as RemoteHandler via cast so the three Model.solve(remote=handler, ...) calls satisfy the RemoteHandler | OetcHandler | None signature. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(sos): move reformulation lifecycle into remote handlers * fix(types): tighten reformulate_sos to bool | Literal["auto"] * test(ssh): cover SOS bracket in RemoteHandler.solve_on_remote --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Fabian <fab.hof@gmx.de>

FBumann and others added 6 commits May 18, 2026 16:57

test: parametrize empty-objective check across both entry points

05a549e

Consolidate the Model.solve() and Solver.from_name(...).solve() tests into one parametrized case — same check, two callers, one assertion. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

test: collapse parametrize to a single test with two raises blocks

d34e453

Same property tested twice — no need for separate test IDs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

FBumann added this to the v0.8.0 milestone May 18, 2026

FBumann marked this pull request as ready for review May 18, 2026 15:53

FBumann requested a review from FabianHofmann May 18, 2026 15:53

This was referenced May 18, 2026

docs: add working-with-solvers notebook for the stateful Solver API #685

Open

refactor(sos): add Model.apply/undo_sos_reformulation methods #690

Merged

Merge branch 'refactor/sos-reformulation-methods' into refactor/solve…

fb85872

…r-from-model-options # Conflicts: # linopy/solvers.py

FabianHofmann reviewed May 18, 2026

View reviewed changes

address review: SOS hint, lp_only_solver fixture, assign_result doc

3f765ca

FabianHofmann merged commit 68d75cd into refactor/sos-reformulation-methods May 18, 2026
2 checks passed

FabianHofmann deleted the refactor/solver-from-model-options branch May 18, 2026 18:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(solver): validation, sanitize kwargs, and result wiring on Solver path#691

refactor(solver): validation, sanitize kwargs, and result wiring on Solver path#691
FabianHofmann merged 9 commits into
refactor/sos-reformulation-methodsfrom
refactor/solver-from-model-options

FBumann commented May 18, 2026 •

edited

Loading

Uh oh!

FabianHofmann left a comment

Uh oh!

FabianHofmann May 18, 2026

Uh oh!

FabianHofmann May 18, 2026

Uh oh!

Uh oh!

FBumann commented May 18, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

FBumann commented May 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Core principle

What lives where after this PR

Uh oh!

FabianHofmann left a comment

Choose a reason for hiding this comment

Uh oh!

FabianHofmann May 18, 2026

Choose a reason for hiding this comment

Uh oh!

FabianHofmann May 18, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

FBumann commented May 18, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

FBumann commented May 18, 2026 •

edited

Loading