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

linopy/model.py

@@ -1650,19 +1650,23 @@ 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(
                 f"Keyword argument `io_api` has to be one of {IO_APIS} or None"
             )
 
         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
@@ -1728,19 +1732,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,12 +1753,10 @@ 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)."
-                )
+        if sanitize_zeros:
+            self.constraints.sanitize_zeros()
+        if sanitize_infinities:
+            self.constraints.sanitize_infinities()
 
         try:
             self.solver = None  # closes any previous solver
@@ -1815,7 +1804,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:

linopy/solvers.py

@@ -504,21 +504,50 @@ 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.")
-        if self.model.variables.sos and not type(self).supports(
-            SolverFeature.SOS_CONSTRAINTS
+        self._validate_model()
+        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 +589,30 @@ 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.
+
+        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:

test/test_solvers.py

@@ -464,3 +464,112 @@ 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")
+
+    @pytest.mark.skipif(
+        "highs" not in solvers.available_solvers, reason="HiGHS not installed"
+    )
+    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"):
+            solvers.Solver.from_name("highs", m, io_api="lp").solve()
+        with pytest.raises(ValueError, match="No objective has been set"):
+            m.solve("highs")
+
+
+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_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 — 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)
+
+        before = m.constraints["c"].coeffs.values.copy()
+        solvers.Solver.from_name("highs", m, io_api="lp")
+        after = m.constraints["c"].coeffs.values
+
+        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."
+        )
+
+
+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