From f1858dea37ec3a9c07c31b8d67f1d1fe5fd0a426 Mon Sep 17 00:00:00 2001 From: FBumann <117816358+FBumann@users.noreply.github.com> Date: Mon, 18 May 2026 16:57:38 +0200 Subject: [PATCH 1/8] refactor(solver): lift feature checks + sanitize/wiring to Solver path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- linopy/model.py | 53 ++++++++++++--------- linopy/solvers.py | 54 ++++++++++++++++++---- test/test_solvers.py | 107 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 185 insertions(+), 29 deletions(-) diff --git a/linopy/model.py b/linopy/model.py index be572279..ba5cad62 100644 --- a/linopy/model.py +++ b/linopy/model.py @@ -1728,19 +1728,6 @@ def solve( else: solution_fn = self.get_solution_file() - if sanitize_zeros: - self.constraints.sanitize_zeros() - - if sanitize_infinities: - self.constraints.sanitize_infinities() - - if self.is_quadratic and not solver_class.supports( - SolverFeature.QUADRATIC_OBJECTIVE - ): - raise ValueError( - f"Solver {solver_name} does not support quadratic problems." - ) - if reformulate_sos not in (True, False, "auto"): raise ValueError( f"Invalid value for reformulate_sos: {reformulate_sos!r}. " @@ -1762,13 +1749,6 @@ def solve( # If SOS is present and the solver doesn't support it (and the user # didn't ask for reformulation), Solver._build() will raise. - if self.variables.semi_continuous: - if not solver_class.supports(SolverFeature.SEMI_CONTINUOUS_VARIABLES): - raise ValueError( - f"Solver {solver_name} does not support semi-continuous variables. " - "Use a solver that supports them (gurobi, cplex, highs)." - ) - try: self.solver = None # closes any previous solver if io_api == "direct": @@ -1778,6 +1758,8 @@ def solve( "explicit_coordinate_names": explicit_coordinate_names, "set_names": set_names, "log_fn": to_path(log_fn), + "sanitize_zeros": sanitize_zeros, + "sanitize_infinities": sanitize_infinities, } if env is not None: build_kwargs["env"] = env @@ -1787,6 +1769,8 @@ def solve( "slice_size": slice_size, "progress": progress, "problem_fn": to_path(problem_fn), + "sanitize_zeros": sanitize_zeros, + "sanitize_infinities": sanitize_infinities, } self.solver = solver = solvers.Solver.from_name( solver_name, @@ -1815,7 +1799,34 @@ def solve( if applied_sos_reformulation_here: self.undo_sos_reformulation() - def assign_result(self, result: Result) -> tuple[str, str]: + def assign_result( + self, + result: Result, + solver: solvers.Solver | None = None, + ) -> tuple[str, str]: + """ + Write a solver Result back onto the model. + + Copies primal / dual values onto variables / constraints, sets + :attr:`status`, :attr:`termination_condition`, and + :attr:`objective.value`. When ``solver`` is provided, also stores it on + ``self.solver`` so post-solve introspection (``model.solver_model``, + ``compute_infeasibilities()``) works. + + Parameters + ---------- + result : Result + The :class:`linopy.constants.Result` returned by + :meth:`linopy.solvers.Solver.solve`. + solver : Solver, optional + The solver instance that produced the result. Pass this when going + through the low-level ``Solver.from_name(...).solve()`` path so the + model's solver reference is wired up the same way ``Model.solve()`` + wires it. + """ + if solver is not None: + self.solver = solver + result.info() if result.solution is not None: diff --git a/linopy/solvers.py b/linopy/solvers.py index 0ca5d956..6cef7594 100644 --- a/linopy/solvers.py +++ b/linopy/solvers.py @@ -507,18 +507,44 @@ def _build(self, **build_kwargs: Any) -> None: """Dispatch to direct or file build based on ``io_api``.""" if self.model is None: raise RuntimeError("Solver has no model attached; cannot build.") - if self.model.variables.sos and not type(self).supports( - SolverFeature.SOS_CONSTRAINTS + sanitize_zeros = build_kwargs.pop("sanitize_zeros", True) + sanitize_infinities = build_kwargs.pop("sanitize_infinities", True) + self._validate_model() + if sanitize_zeros: + self.model.constraints.sanitize_zeros() + if sanitize_infinities: + self.model.constraints.sanitize_infinities() + if self.io_api == "direct": + self._build_direct(**build_kwargs) + else: + self._build_file(**build_kwargs) + + def _validate_model(self) -> None: + """Pre-build checks on whether this solver can handle ``self.model``.""" + model = self.model + assert model is not None + solver_name = self.solver_name.value + cls = type(self) + + if model.is_quadratic and not cls.supports(SolverFeature.QUADRATIC_OBJECTIVE): + raise ValueError( + f"Solver {solver_name} does not support quadratic problems." + ) + + if model.variables.semi_continuous and not cls.supports( + SolverFeature.SEMI_CONTINUOUS_VARIABLES ): raise ValueError( - f"Solver {self.solver_name.value} does not support SOS constraints. " + f"Solver {solver_name} does not support semi-continuous variables. " + "Use a solver that supports them (gurobi, cplex, highs)." + ) + + if model.variables.sos and not cls.supports(SolverFeature.SOS_CONSTRAINTS): + raise ValueError( + f"Solver {solver_name} does not support SOS constraints. " "Call `model.apply_sos_reformulation()` first, or use a solver that " "supports SOS." ) - if self.io_api == "direct": - self._build_direct(**build_kwargs) - else: - self._build_file(**build_kwargs) def _build_direct(self, **build_kwargs: Any) -> None: """Build the native solver model from ``self.model``. Override per-solver.""" @@ -560,7 +586,19 @@ def _build_file(self, **build_kwargs: Any) -> None: self._cache_model_sizes(model) def solve(self, **run_kwargs: Any) -> Result: - """Run the prepared solver and return a :class:`Result`.""" + """ + Run the prepared solver and return a :class:`Result`. + + The canonical low-level pattern is:: + + solver = Solver.from_name("gurobi", model, io_api="direct") + result = solver.solve() + model.assign_result(result, solver=solver) + + Passing ``solver=`` to :meth:`Model.assign_result` wires + ``model.solver`` so post-solve helpers like + :meth:`Model.compute_infeasibilities` keep working. + """ if self.io_api == "direct" or self.solver_model is not None: return self._run_direct(**run_kwargs) if self._problem_fn is not None: diff --git a/test/test_solvers.py b/test/test_solvers.py index db894137..b5cb7dad 100644 --- a/test/test_solvers.py +++ b/test/test_solvers.py @@ -464,3 +464,110 @@ def test_xpress_gpu_feature_reflects_installed_version() -> None: assert solvers.Xpress.supports( SolverFeature.GPU_ACCELERATION ) == _installed_version_in("xpress", ">=9.8.0") + + +class TestValidateModelOnBuild: + """Solver._build() runs solver-feature checks regardless of entry point.""" + + @pytest.mark.skipif( + "highs" not in solvers.available_solvers, reason="HiGHS not installed" + ) + def test_quadratic_without_qp_support_raises(self) -> None: + # GLPK is LP-only; if not installed, fall back to a different LP-only path. + # CBC and GLPK both lack QUADRATIC_OBJECTIVE. + lp_only = next( + (s for s in ("glpk", "cbc") if s in solvers.available_solvers), None + ) + if lp_only is None: + pytest.skip("Need an LP-only solver (glpk or cbc) to run this test") + + m = Model() + x = m.add_variables(name="x", lower=0, upper=10) + m.add_objective(x * x, sense="min") + + with pytest.raises(ValueError, match="does not support quadratic"): + solvers.Solver.from_name(lp_only, m, io_api="lp") + + def test_semi_continuous_without_support_raises(self) -> None: + lp_only = next( + (s for s in ("glpk", "cbc") if s in solvers.available_solvers), None + ) + if lp_only is None: + pytest.skip("Need an LP-only solver (glpk or cbc) to run this test") + + m = Model() + x = m.add_variables(name="x", lower=1, upper=10, semi_continuous=True) + m.add_objective(x) + + with pytest.raises(ValueError, match="does not support semi-continuous"): + solvers.Solver.from_name(lp_only, m, io_api="lp") + + +class TestSanitizeKwargs: + """sanitize_zeros / sanitize_infinities on Solver.from_model() control the build.""" + + @pytest.mark.skipif( + "highs" not in solvers.available_solvers, reason="HiGHS not installed" + ) + def test_sanitize_zeros_default_on_mutates_constraints(self) -> None: + m = Model() + x = m.add_variables(name="x", lower=0, upper=10) + # Constraint with a near-zero coefficient + m.add_constraints(1e-12 * x + x >= 0, name="c") + m.add_objective(x) + + # Default: sanitize_zeros=True -> the near-zero term gets masked + solvers.Solver.from_name("highs", m, io_api="lp") + coeffs = m.constraints["c"].coeffs.values.ravel() + assert np.isnan(coeffs[0]) or coeffs[0] == 0 or coeffs[1] != 0 + + @pytest.mark.skipif( + "highs" not in solvers.available_solvers, reason="HiGHS not installed" + ) + def test_sanitize_zeros_off_leaves_constraints_alone(self) -> None: + m = Model() + x = m.add_variables(name="x", lower=0, upper=10) + m.add_constraints(1e-12 * x + x >= 0, name="c") + m.add_objective(x) + + before = m.constraints["c"].coeffs.values.copy() + solvers.Solver.from_name( + "highs", m, io_api="lp", sanitize_zeros=False, sanitize_infinities=False + ) + after = m.constraints["c"].coeffs.values + # Without sanitization, the near-zero coefficient is preserved verbatim. + assert np.allclose(before, after, equal_nan=True) + + +class TestAssignResultWiring: + """assign_result(result, solver=...) populates model.solver.""" + + @pytest.mark.skipif( + "highs" not in solvers.available_solvers, reason="HiGHS not installed" + ) + def test_assign_result_with_solver_wires_model_solver(self) -> None: + m = Model() + x = m.add_variables(name="x", lower=0, upper=10) + m.add_objective(x, sense="min") + + assert m.solver is None + solver = solvers.Solver.from_name("highs", m, io_api="lp") + result = solver.solve() + m.assign_result(result, solver=solver) + + assert m.solver is solver + assert m.solver_model is solver.solver_model + + @pytest.mark.skipif( + "highs" not in solvers.available_solvers, reason="HiGHS not installed" + ) + def test_assign_result_without_solver_kwarg_leaves_solver_unset(self) -> None: + m = Model() + x = m.add_variables(name="x", lower=0, upper=10) + m.add_objective(x, sense="min") + + solver = solvers.Solver.from_name("highs", m, io_api="lp") + result = solver.solve() + m.assign_result(result) # no solver kwarg + + assert m.solver is None From 69942523526ff7ce56c6e07b41fbfa9119bec0f9 Mon Sep 17 00:00:00 2001 From: FBumann <117816358+FBumann@users.noreply.github.com> Date: Mon, 18 May 2026 17:09:53 +0200 Subject: [PATCH 2/8] move empty-objective check to Solver.solve() for entry-point parity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- linopy/model.py | 6 ------ linopy/solvers.py | 11 +++++++++++ test/test_solvers.py | 24 ++++++++++++++++++++++++ 3 files changed, 35 insertions(+), 6 deletions(-) diff --git a/linopy/model.py b/linopy/model.py index ba5cad62..58fe10b7 100644 --- a/linopy/model.py +++ b/linopy/model.py @@ -1650,12 +1650,6 @@ def solve( sanitize_zeros=sanitize_zeros, sanitize_infinities=sanitize_infinities ) - if self.objective.expression.empty: - raise ValueError( - "No objective has been set on the model. Use `m.add_objective(...)` " - "first (e.g. `m.add_objective(0 * x)` for a pure feasibility problem)." - ) - # check io_api if io_api is not None and io_api not in IO_APIS: raise ValueError( diff --git a/linopy/solvers.py b/linopy/solvers.py index 6cef7594..14f1cf43 100644 --- a/linopy/solvers.py +++ b/linopy/solvers.py @@ -598,7 +598,18 @@ def solve(self, **run_kwargs: Any) -> Result: Passing ``solver=`` to :meth:`Model.assign_result` wires ``model.solver`` so post-solve helpers like :meth:`Model.compute_infeasibilities` keep working. + + Raises + ------ + ValueError + If the attached model has no objective set. Submit-time check + shared by both ``Model.solve()`` and direct-Solver callers. """ + if self.model is not None and self.model.objective.expression.empty: + raise ValueError( + "No objective has been set on the model. Use `m.add_objective(...)` " + "first (e.g. `m.add_objective(0 * x)` for a pure feasibility problem)." + ) if self.io_api == "direct" or self.solver_model is not None: return self._run_direct(**run_kwargs) if self._problem_fn is not None: diff --git a/test/test_solvers.py b/test/test_solvers.py index b5cb7dad..f20eee5d 100644 --- a/test/test_solvers.py +++ b/test/test_solvers.py @@ -502,6 +502,30 @@ def test_semi_continuous_without_support_raises(self) -> None: with pytest.raises(ValueError, match="does not support semi-continuous"): solvers.Solver.from_name(lp_only, m, io_api="lp") + @pytest.mark.skipif( + "highs" not in solvers.available_solvers, reason="HiGHS not installed" + ) + def test_solve_without_objective_raises_on_solver_path(self) -> None: + m = Model() + m.add_variables(name="x", lower=0, upper=10) + # No objective added. + + # Build is permitted (translate-only paths like to_gurobipy() need this). + solver = solvers.Solver.from_name("highs", m, io_api="lp") + # Solve is not — the check fires at the submit primitive. + with pytest.raises(ValueError, match="No objective has been set"): + solver.solve() + + @pytest.mark.skipif( + "highs" not in solvers.available_solvers, reason="HiGHS not installed" + ) + def test_solve_without_objective_raises_on_model_path(self) -> None: + m = Model() + m.add_variables(name="x", lower=0, upper=10) + + with pytest.raises(ValueError, match="No objective has been set"): + m.solve("highs") + class TestSanitizeKwargs: """sanitize_zeros / sanitize_infinities on Solver.from_model() control the build.""" From 05a549ea4a4c8cbffabbb7af9188efc389206002 Mon Sep 17 00:00:00 2001 From: FBumann <117816358+FBumann@users.noreply.github.com> Date: Mon, 18 May 2026 17:11:26 +0200 Subject: [PATCH 3/8] test: parametrize empty-objective check across both entry points MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- test/test_solvers.py | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/test/test_solvers.py b/test/test_solvers.py index f20eee5d..39fc8939 100644 --- a/test/test_solvers.py +++ b/test/test_solvers.py @@ -5,6 +5,7 @@ @author: sid """ +from collections.abc import Callable from pathlib import Path import numpy as np @@ -505,26 +506,24 @@ def test_semi_continuous_without_support_raises(self) -> None: @pytest.mark.skipif( "highs" not in solvers.available_solvers, reason="HiGHS not installed" ) - def test_solve_without_objective_raises_on_solver_path(self) -> None: - m = Model() - m.add_variables(name="x", lower=0, upper=10) - # No objective added. - - # Build is permitted (translate-only paths like to_gurobipy() need this). - solver = solvers.Solver.from_name("highs", m, io_api="lp") - # Solve is not — the check fires at the submit primitive. - with pytest.raises(ValueError, match="No objective has been set"): - solver.solve() - - @pytest.mark.skipif( - "highs" not in solvers.available_solvers, reason="HiGHS not installed" + @pytest.mark.parametrize( + "submit", + [ + pytest.param( + lambda m: solvers.Solver.from_name("highs", m, io_api="lp").solve(), + id="Solver.from_name(...).solve()", + ), + pytest.param(lambda m: m.solve("highs"), id="Model.solve()"), + ], ) - def test_solve_without_objective_raises_on_model_path(self) -> None: + def test_solve_without_objective_raises( + self, submit: "Callable[[Model], object]" + ) -> None: m = Model() m.add_variables(name="x", lower=0, upper=10) - + # No objective added — both entry points should raise the same error. with pytest.raises(ValueError, match="No objective has been set"): - m.solve("highs") + submit(m) class TestSanitizeKwargs: From d34e453b890e6588bf43f39a45641619518ba2ce Mon Sep 17 00:00:00 2001 From: FBumann <117816358+FBumann@users.noreply.github.com> Date: Mon, 18 May 2026 17:14:07 +0200 Subject: [PATCH 4/8] test: collapse parametrize to a single test with two raises blocks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Same property tested twice — no need for separate test IDs. Co-Authored-By: Claude Opus 4.7 (1M context) --- test/test_solvers.py | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/test/test_solvers.py b/test/test_solvers.py index 39fc8939..72cc013e 100644 --- a/test/test_solvers.py +++ b/test/test_solvers.py @@ -5,7 +5,6 @@ @author: sid """ -from collections.abc import Callable from pathlib import Path import numpy as np @@ -506,24 +505,14 @@ def test_semi_continuous_without_support_raises(self) -> None: @pytest.mark.skipif( "highs" not in solvers.available_solvers, reason="HiGHS not installed" ) - @pytest.mark.parametrize( - "submit", - [ - pytest.param( - lambda m: solvers.Solver.from_name("highs", m, io_api="lp").solve(), - id="Solver.from_name(...).solve()", - ), - pytest.param(lambda m: m.solve("highs"), id="Model.solve()"), - ], - ) - def test_solve_without_objective_raises( - self, submit: "Callable[[Model], object]" - ) -> None: + def test_solve_without_objective_raises(self) -> None: m = Model() m.add_variables(name="x", lower=0, upper=10) # No objective added — both entry points should raise the same error. with pytest.raises(ValueError, match="No objective has been set"): - submit(m) + solvers.Solver.from_name("highs", m, io_api="lp").solve() + with pytest.raises(ValueError, match="No objective has been set"): + m.solve("highs") class TestSanitizeKwargs: From eacca3d13bbfefd9d15a75846cb0bc22d3d79cb6 Mon Sep 17 00:00:00 2001 From: FBumann <117816358+FBumann@users.noreply.github.com> Date: Mon, 18 May 2026 17:19:33 +0200 Subject: [PATCH 5/8] 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) --- linopy/model.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/linopy/model.py b/linopy/model.py index 58fe10b7..ba5cad62 100644 --- a/linopy/model.py +++ b/linopy/model.py @@ -1650,6 +1650,12 @@ def solve( sanitize_zeros=sanitize_zeros, sanitize_infinities=sanitize_infinities ) + if self.objective.expression.empty: + raise ValueError( + "No objective has been set on the model. Use `m.add_objective(...)` " + "first (e.g. `m.add_objective(0 * x)` for a pure feasibility problem)." + ) + # check io_api if io_api is not None and io_api not in IO_APIS: raise ValueError( From 20e636bbe3c57dc3f4402cf683655360cb2bee2c Mon Sep 17 00:00:00 2001 From: FBumann <117816358+FBumann@users.noreply.github.com> Date: Mon, 18 May 2026 17:22:46 +0200 Subject: [PATCH 6/8] 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) --- linopy/model.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/linopy/model.py b/linopy/model.py index ba5cad62..5942454b 100644 --- a/linopy/model.py +++ b/linopy/model.py @@ -1650,12 +1650,6 @@ def solve( sanitize_zeros=sanitize_zeros, sanitize_infinities=sanitize_infinities ) - if self.objective.expression.empty: - raise ValueError( - "No objective has been set on the model. Use `m.add_objective(...)` " - "first (e.g. `m.add_objective(0 * x)` for a pure feasibility problem)." - ) - # check io_api if io_api is not None and io_api not in IO_APIS: raise ValueError( @@ -1663,6 +1657,16 @@ def solve( ) if remote is not None: + # The remote branch short-circuits before reaching Solver.solve(), + # which is where the empty-objective check normally fires. Replicate + # it here. This duplication becomes obsolete once OETC is folded + # into the Solver pipeline (see PyPSA/linopy#683). + if self.objective.expression.empty: + raise ValueError( + "No objective has been set on the model. Use " + "`m.add_objective(...)` first (e.g. `m.add_objective(0 * x)` " + "for a pure feasibility problem)." + ) if isinstance(remote, OetcHandler): solved = remote.solve_on_oetc( self, solver_name=solver_name, **solver_options From 691bfacd3a3d6ee0bd13171a48d30956925e57e5 Mon Sep 17 00:00:00 2001 From: FBumann <117816358+FBumann@users.noreply.github.com> Date: Mon, 18 May 2026 17:42:06 +0200 Subject: [PATCH 7/8] keep sanitize on Model; Solver.from_model() stays mutation-free MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- linopy/model.py | 9 +++++---- linopy/solvers.py | 17 ++++++++++------- test/test_solvers.py | 34 ++++++++++++---------------------- 3 files changed, 27 insertions(+), 33 deletions(-) diff --git a/linopy/model.py b/linopy/model.py index 5942454b..e4555e38 100644 --- a/linopy/model.py +++ b/linopy/model.py @@ -1753,6 +1753,11 @@ def solve( # If SOS is present and the solver doesn't support it (and the user # didn't ask for reformulation), Solver._build() will raise. + if sanitize_zeros: + self.constraints.sanitize_zeros() + if sanitize_infinities: + self.constraints.sanitize_infinities() + try: self.solver = None # closes any previous solver if io_api == "direct": @@ -1762,8 +1767,6 @@ def solve( "explicit_coordinate_names": explicit_coordinate_names, "set_names": set_names, "log_fn": to_path(log_fn), - "sanitize_zeros": sanitize_zeros, - "sanitize_infinities": sanitize_infinities, } if env is not None: build_kwargs["env"] = env @@ -1773,8 +1776,6 @@ def solve( "slice_size": slice_size, "progress": progress, "problem_fn": to_path(problem_fn), - "sanitize_zeros": sanitize_zeros, - "sanitize_infinities": sanitize_infinities, } self.solver = solver = solvers.Solver.from_name( solver_name, diff --git a/linopy/solvers.py b/linopy/solvers.py index 14f1cf43..43ce05f0 100644 --- a/linopy/solvers.py +++ b/linopy/solvers.py @@ -504,16 +504,19 @@ def from_model( return instance def _build(self, **build_kwargs: Any) -> None: - """Dispatch to direct or file build based on ``io_api``.""" + """ + Dispatch to direct or file build based on ``io_api``. + + The Solver never mutates ``self.model``. Constraint sanitization + (``model.constraints.sanitize_zeros()`` / + ``.sanitize_infinities()``) and SOS reformulation + (``model.apply_sos_reformulation()``) are Model-level operations + the caller applies first; this builder consumes whatever shape it + is handed. + """ if self.model is None: raise RuntimeError("Solver has no model attached; cannot build.") - sanitize_zeros = build_kwargs.pop("sanitize_zeros", True) - sanitize_infinities = build_kwargs.pop("sanitize_infinities", True) self._validate_model() - if sanitize_zeros: - self.model.constraints.sanitize_zeros() - if sanitize_infinities: - self.model.constraints.sanitize_infinities() if self.io_api == "direct": self._build_direct(**build_kwargs) else: diff --git a/test/test_solvers.py b/test/test_solvers.py index 72cc013e..27368120 100644 --- a/test/test_solvers.py +++ b/test/test_solvers.py @@ -515,40 +515,30 @@ def test_solve_without_objective_raises(self) -> None: m.solve("highs") -class TestSanitizeKwargs: - """sanitize_zeros / sanitize_infinities on Solver.from_model() control the build.""" +class TestSolverDoesNotMutateModel: + """Solver.from_model() must not mutate model state (sanitize stays Model-level).""" @pytest.mark.skipif( "highs" not in solvers.available_solvers, reason="HiGHS not installed" ) - def test_sanitize_zeros_default_on_mutates_constraints(self) -> None: + def test_from_model_leaves_constraints_untouched(self) -> None: m = Model() x = m.add_variables(name="x", lower=0, upper=10) - # Constraint with a near-zero coefficient + # Constraint with a near-zero coefficient — would be sanitized away if + # the Solver path were sanitizing on build. m.add_constraints(1e-12 * x + x >= 0, name="c") m.add_objective(x) - # Default: sanitize_zeros=True -> the near-zero term gets masked + before = m.constraints["c"].coeffs.values.copy() solvers.Solver.from_name("highs", m, io_api="lp") - coeffs = m.constraints["c"].coeffs.values.ravel() - assert np.isnan(coeffs[0]) or coeffs[0] == 0 or coeffs[1] != 0 - - @pytest.mark.skipif( - "highs" not in solvers.available_solvers, reason="HiGHS not installed" - ) - def test_sanitize_zeros_off_leaves_constraints_alone(self) -> None: - m = Model() - x = m.add_variables(name="x", lower=0, upper=10) - m.add_constraints(1e-12 * x + x >= 0, name="c") - m.add_objective(x) + after = m.constraints["c"].coeffs.values - before = m.constraints["c"].coeffs.values.copy() - solvers.Solver.from_name( - "highs", m, io_api="lp", sanitize_zeros=False, sanitize_infinities=False + assert np.allclose(before, after, equal_nan=True), ( + "Solver.from_model() must not mutate model constraints. " + "Sanitization is a Model-level primitive; call " + "model.constraints.sanitize_zeros() / .sanitize_infinities() " + "explicitly before building." ) - after = m.constraints["c"].coeffs.values - # Without sanitization, the near-zero coefficient is preserved verbatim. - assert np.allclose(before, after, equal_nan=True) class TestAssignResultWiring: From 3f765cad764a4db146b3ad02e774e76fb36ff42a Mon Sep 17 00:00:00 2001 From: Fabian Date: Mon, 18 May 2026 20:48:00 +0200 Subject: [PATCH 8/8] address review: SOS hint, lp_only_solver fixture, assign_result doc --- linopy/model.py | 8 ++++---- linopy/solvers.py | 4 ++-- test/test_solvers.py | 33 ++++++++++++--------------------- 3 files changed, 18 insertions(+), 27 deletions(-) diff --git a/linopy/model.py b/linopy/model.py index 843573a1..12a46206 100644 --- a/linopy/model.py +++ b/linopy/model.py @@ -1851,10 +1851,10 @@ def assign_result( The :class:`linopy.constants.Result` returned by :meth:`linopy.solvers.Solver.solve`. solver : Solver, optional - The solver instance that produced the result. Pass this when going - through the low-level ``Solver.from_name(...).solve()`` path so the - model's solver reference is wired up the same way ``Model.solve()`` - wires it. + The solver instance that produced the result. Pass it on the + low-level ``Solver.from_name(...).solve()`` path to attach it as + ``self.solver`` for post-solve introspection. ``Model.solve()`` + attaches the solver itself and does not pass this argument. """ if solver is not None: self.solver = solver diff --git a/linopy/solvers.py b/linopy/solvers.py index 6872ff94..0fbd2c12 100644 --- a/linopy/solvers.py +++ b/linopy/solvers.py @@ -546,8 +546,8 @@ def _validate_model(self) -> None: if model.variables.sos and not cls.supports(SolverFeature.SOS_CONSTRAINTS): raise ValueError( f"Solver {solver_name} does not support SOS constraints. " - "Call `model.apply_sos_reformulation()` first, or use a solver that " - "supports SOS." + "Reformulate first via `Model.solve(reformulate_sos=True)` or " + "`model.apply_sos_reformulation()`, or use a solver that supports SOS." ) def _build_direct(self, **build_kwargs: Any) -> None: diff --git a/test/test_solvers.py b/test/test_solvers.py index 27368120..1b6bd9a9 100644 --- a/test/test_solvers.py +++ b/test/test_solvers.py @@ -23,6 +23,14 @@ from linopy.solvers import _installed_version_in +@pytest.fixture +def lp_only_solver() -> str: + for name in ("glpk", "cbc"): + if name in solvers.available_solvers: + return name + pytest.skip("Need an LP-only solver (glpk or cbc) installed") + + @pytest.fixture def simple_model() -> Model: m = Model(chunk=None) @@ -469,38 +477,21 @@ def test_xpress_gpu_feature_reflects_installed_version() -> None: class TestValidateModelOnBuild: """Solver._build() runs solver-feature checks regardless of entry point.""" - @pytest.mark.skipif( - "highs" not in solvers.available_solvers, reason="HiGHS not installed" - ) - def test_quadratic_without_qp_support_raises(self) -> None: - # GLPK is LP-only; if not installed, fall back to a different LP-only path. - # CBC and GLPK both lack QUADRATIC_OBJECTIVE. - lp_only = next( - (s for s in ("glpk", "cbc") if s in solvers.available_solvers), None - ) - if lp_only is None: - pytest.skip("Need an LP-only solver (glpk or cbc) to run this test") - + def test_quadratic_without_qp_support_raises(self, lp_only_solver: str) -> None: m = Model() x = m.add_variables(name="x", lower=0, upper=10) m.add_objective(x * x, sense="min") with pytest.raises(ValueError, match="does not support quadratic"): - solvers.Solver.from_name(lp_only, m, io_api="lp") - - def test_semi_continuous_without_support_raises(self) -> None: - lp_only = next( - (s for s in ("glpk", "cbc") if s in solvers.available_solvers), None - ) - if lp_only is None: - pytest.skip("Need an LP-only solver (glpk or cbc) to run this test") + solvers.Solver.from_name(lp_only_solver, m, io_api="lp") + def test_semi_continuous_without_support_raises(self, lp_only_solver: str) -> None: m = Model() x = m.add_variables(name="x", lower=1, upper=10, semi_continuous=True) m.add_objective(x) with pytest.raises(ValueError, match="does not support semi-continuous"): - solvers.Solver.from_name(lp_only, m, io_api="lp") + solvers.Solver.from_name(lp_only_solver, m, io_api="lp") @pytest.mark.skipif( "highs" not in solvers.available_solvers, reason="HiGHS not installed"