diff --git a/.github/workflows/test_pr_and_main.yml b/.github/workflows/test_pr_and_main.yml index e99be30a1..1106d91c5 100644 --- a/.github/workflows/test_pr_and_main.yml +++ b/.github/workflows/test_pr_and_main.yml @@ -116,6 +116,10 @@ jobs: run: | coverage run $COV_ARGS -m pytest mpisppy/tests/test_nonant_validation.py -v + - name: Test prox/solver quadratic compatibility checks + run: | + coverage run $COV_ARGS -m pytest mpisppy/tests/test_prox_solver_compat.py -v + - name: Test generic_cylinders usage run: | coverage run $COV_ARGS -m pytest mpisppy/tests/test_generic_cylinders.py -v diff --git a/mpisppy/phbase.py b/mpisppy/phbase.py index 7269c9cd4..648b7ed4f 100644 --- a/mpisppy/phbase.py +++ b/mpisppy/phbase.py @@ -29,6 +29,14 @@ def profile(filename=None, comm=MPI.COMM_WORLD): logger = logging.getLogger('PHBase') logger.setLevel(logging.WARN) +# Shared remediation hint for the quadratic-prox / solver compatibility checks +# (issue #762): a solver that cannot handle the quadratic proximal term should +# linearize it instead. +_LINEARIZE_PROX_HINT = ( + "Re-run with --linearize-proximal-terms (add " + "--linearize-binary-proximal-terms for binary variables)." +) + #====================== def _Compute_Xbar(opt, verbose=False): @@ -723,6 +731,103 @@ def prox_disabled(self): return not bool(self.local_scenarios[self.local_scenario_names[0]]._mpisppy_model.prox_on.value) + def _prox_is_quadratic(self): + """True when subproblem objectives carry a (non-linearized) quadratic + proximal term, i.e. the prox term is attached and active and is not + being approximated by linear cuts. This is exactly the condition under + which a solver that cannot handle a quadratic objective will fail. + """ + return (not self._prox_approx) and (not self.prox_disabled) + + + def _check_prox_solver_capability(self): + """Proactive half of the quadratic-prox/solver compatibility check. + + If the solver reports (via the legacy ``has_capability`` API) that it + cannot handle a quadratic objective, fail immediately with an + actionable message rather than letting the first proximal solve fail + cryptically. Solvers that do not expose capability information (e.g. + HiGHS via the APPSI or ``pyomo.contrib.solver`` interfaces) are left to + the reactive check after the first solve. + + The decision is deterministic and identical on every rank (same + ``solver_name``), so raising here cannot desynchronize MPI. + """ + if not self._prox_is_quadratic(): + return + # All subproblems share solver_name, so one probe is representative. + s = next(iter(self.local_scenarios.values())) + if sputils.solver_quadratic_objective_capability(s._solver_plugin) is False: + raise RuntimeError( + f"Solver '{self.options.get('solver_name')}' reports that it " + "cannot handle a quadratic objective, which the Progressive " + "Hedging proximal term requires. " + _LINEARIZE_PROX_HINT + ) + + + def _check_prox_solve_succeeded(self): + """Reactive half of the quadratic-prox/solver compatibility check. + + After the first proximal (quadratic) solve, if no subproblem anywhere + produced a solution, the most likely cause is a solver that cannot + handle a quadratic objective but does not report it through + ``has_capability`` (e.g. HiGHS, which cannot solve an MIQP). Emit an + actionable message instead of leaving the user with the cryptic + ``TerminationCondition=unknown`` from the failed solve. + + Restricted to iteration 1: if the first quadratic solve succeeds, the + solver supports quadratic objectives, so any later failure is a genuine + optimization issue rather than a capability problem. The "no solution + anywhere" test is reduced across ranks with ``allreduce_or`` so the + raise decision is identical on every rank (no MPI desynchronization); + a partial failure (some subproblems still solve) falls through to the + existing behavior. + """ + if self._PHIter != 1 or not self._prox_is_quadratic(): + return + local_any_solution = any( + s._mpisppy_data.solution_available + for s in self.local_scenarios.values() + ) + if self.allreduce_or(local_any_solution): + return + raise RuntimeError( + f"No subproblem produced a solution at PH iteration " + f"{self._PHIter} while a quadratic proximal term was active. " + f"Solver '{self.options.get('solver_name')}' may not support " + "quadratic objectives (e.g. HiGHS cannot solve an MIQP). " + + _LINEARIZE_PROX_HINT + ) + + + def _reraise_as_prox_capability_error(self, exc): + """Wrap a raised first-solve error as an actionable capability message. + + Completes the quadratic-prox/solver compatibility checks for solvers + that signal "cannot handle a quadratic objective" by *raising* during + the solve rather than returning without a solution. cbc and glpk are the + motivating case: their LP writer raises before the reactive + ``_check_prox_solve_succeeded`` can run, and the proactive + ``has_capability`` probe does not always catch them (the capability is + reported inconsistently across Pyomo versions / solver interfaces). + + Only the first quadratic solve is treated this way; a raise at a later + iteration is a genuine solve error and is left to propagate unchanged. + When applicable this raises a new ``RuntimeError`` chained from ``exc`` + (so the original traceback is preserved); otherwise it returns and the + caller re-raises ``exc`` as-is. The guard matches the reactive check, so + MPI synchronization is unchanged beyond the raise that already occurred. + """ + if self._PHIter != 1 or not self._prox_is_quadratic(): + return + raise RuntimeError( + f"Solver '{self.options.get('solver_name')}' raised an error on the " + "first solve with a quadratic proximal term active, which it may " + "not support (e.g. cbc/glpk cannot write a quadratic objective to " + "LP format). " + _LINEARIZE_PROX_HINT + ) from exc + + def attach_PH_to_objective(self, add_duals, add_prox, add_smooth=0): """ Attach dual weight and prox terms to the objective function of the models in `local_scenarios`. @@ -1182,15 +1287,35 @@ def iterk_loop(self): and self.cylinder_rank == 0 ) - self.solve_loop( - solver_options=self._effective_solver_options(self._PHIter), - dtiming=dtiming, - gripe=True, - disable_pyomo_signal_handling=False, - tee=teeme, - verbose=verbose, - warmstart=True, - ) + # Before the first proximal (quadratic) solve, fail fast with + # guidance if the solver reports it cannot handle a quadratic + # objective; see issue #762. + if self._PHIter == 1: + self._check_prox_solver_capability() + + try: + self.solve_loop( + solver_options=self._effective_solver_options(self._PHIter), + dtiming=dtiming, + gripe=True, + disable_pyomo_signal_handling=False, + tee=teeme, + verbose=verbose, + warmstart=True, + ) + except Exception as e: + # Some solvers reject a quadratic objective by *raising* during + # the solve rather than returning no solution -- e.g. cbc/glpk, + # whose LP writer raises before the reactive check below can + # run. If this is the first quadratic solve, re-raise with an + # actionable message; see issue #762. + self._reraise_as_prox_capability_error(e) + raise + + # If the first proximal solve produced nothing, the solver may not + # support quadratic objectives; give an actionable message + # (see issue #762). + self._check_prox_solve_succeeded() if have_extensions: self.extobject.enditer() diff --git a/mpisppy/tests/test_prox_solver_compat.py b/mpisppy/tests/test_prox_solver_compat.py new file mode 100644 index 000000000..e30ddfadc --- /dev/null +++ b/mpisppy/tests/test_prox_solver_compat.py @@ -0,0 +1,198 @@ +############################################################################### +# mpi-sppy: MPI-based Stochastic Programming in PYthon +# +# Copyright (c) 2024, Lawrence Livermore National Security, LLC, Alliance for +# Sustainable Energy, LLC, The Regents of the University of California, et al. +# All rights reserved. Please see the files COPYRIGHT.md and LICENSE.md for +# full copyright and license information. +############################################################################### +"""Tests for the quadratic-proximal-term / solver-capability checks. + +Progressive Hedging's proximal term makes the subproblem objective quadratic +(unless it is linearized). A solver that cannot handle a quadratic objective +then fails -- cryptically, in the case of HiGHS on an MIQP (see issue #762). +mpi-sppy guards this in two complementary ways: + +* a proactive check that uses the legacy ``has_capability`` API where it is + available (catches e.g. cbc/glpk before any solve), and +* a reactive check after the first proximal solve, for solvers that do not + expose capability information (e.g. HiGHS via the APPSI or + ``pyomo.contrib.solver`` interfaces). + +These tests exercise the decision logic directly and need neither MPI nor a +solver. +""" + +import types +import unittest + +import mpisppy.utils.sputils as sputils +from mpisppy.phbase import PHBase + + +class _FakePlugin: + """Stand-in solver plugin exposing has_capability (legacy-style).""" + + def __init__(self, cap): + self._cap = cap + + def has_capability(self, name): + if isinstance(self._cap, Exception): + raise self._cap + return self._cap + + +class _NoCapPlugin: + """Stand-in for APPSI / contrib.solver wrappers: no has_capability.""" + + +def _scenario(plugin=None, solution_available=True): + return types.SimpleNamespace( + _solver_plugin=plugin, + _mpisppy_data=types.SimpleNamespace(solution_available=solution_available), + ) + + +def _opt(prox_quadratic, scenarios, phiter=1, solver_name="testsolver"): + """A minimal duck-typed stand-in for a PHBase instance. + + Only the attributes the checks under test actually touch are provided; + ``allreduce_or`` defaults to the single-rank identity and can be + overridden to simulate other ranks. + """ + opt = types.SimpleNamespace() + opt._prox_is_quadratic = lambda: prox_quadratic + opt.local_scenarios = scenarios + opt.options = {"solver_name": solver_name} + opt._PHIter = phiter + opt.allreduce_or = lambda local: local + return opt + + +class TestSolverQuadraticCapabilityProbe(unittest.TestCase): + def test_reports_supported(self): + self.assertIs( + sputils.solver_quadratic_objective_capability(_FakePlugin(True)), True + ) + + def test_reports_unsupported(self): + self.assertIs( + sputils.solver_quadratic_objective_capability(_FakePlugin(False)), False + ) + + def test_unknown_when_no_capability_method(self): + self.assertIsNone( + sputils.solver_quadratic_objective_capability(_NoCapPlugin()) + ) + + def test_unknown_when_capability_raises(self): + self.assertIsNone( + sputils.solver_quadratic_objective_capability( + _FakePlugin(RuntimeError("boom")) + ) + ) + + +class TestProxIsQuadratic(unittest.TestCase): + @staticmethod + def _stub(prox_approx, prox_disabled): + return types.SimpleNamespace( + _prox_approx=prox_approx, prox_disabled=prox_disabled + ) + + def test_quadratic_when_attached_and_not_linearized(self): + self.assertTrue(PHBase._prox_is_quadratic(self._stub(False, False))) + + def test_not_quadratic_when_linearized(self): + self.assertFalse(PHBase._prox_is_quadratic(self._stub(True, False))) + + def test_not_quadratic_when_prox_disabled(self): + self.assertFalse(PHBase._prox_is_quadratic(self._stub(False, True))) + + +class TestCheckProxSolverCapability(unittest.TestCase): + def test_raises_when_solver_reports_no_quadratic(self): + opt = _opt(True, {"Scenario1": _scenario(_FakePlugin(False))}) + with self.assertRaisesRegex(RuntimeError, "linearize-proximal-terms"): + PHBase._check_prox_solver_capability(opt) + + def test_no_raise_when_capability_unknown(self): + # HiGHS-like: no has_capability -> deferred to the reactive check + opt = _opt(True, {"Scenario1": _scenario(_NoCapPlugin())}) + PHBase._check_prox_solver_capability(opt) + + def test_no_raise_when_solver_supports_quadratic(self): + opt = _opt(True, {"Scenario1": _scenario(_FakePlugin(True))}) + PHBase._check_prox_solver_capability(opt) + + def test_no_raise_when_prox_linearized(self): + # quadratic check is skipped entirely when the prox term is linearized + opt = _opt(False, {"Scenario1": _scenario(_FakePlugin(False))}) + PHBase._check_prox_solver_capability(opt) + + +class TestCheckProxSolveSucceeded(unittest.TestCase): + def test_raises_when_no_solution_anywhere_at_iter1(self): + opt = _opt(True, {"s1": _scenario(solution_available=False)}, phiter=1) + with self.assertRaisesRegex(RuntimeError, "linearize-proximal-terms"): + PHBase._check_prox_solve_succeeded(opt) + + def test_no_raise_when_a_subproblem_solved(self): + opt = _opt( + True, + { + "s1": _scenario(solution_available=False), + "s2": _scenario(solution_available=True), + }, + phiter=1, + ) + PHBase._check_prox_solve_succeeded(opt) + + def test_no_raise_after_first_iteration(self): + opt = _opt(True, {"s1": _scenario(solution_available=False)}, phiter=2) + PHBase._check_prox_solve_succeeded(opt) + + def test_no_raise_when_prox_linearized(self): + opt = _opt(False, {"s1": _scenario(solution_available=False)}, phiter=1) + PHBase._check_prox_solve_succeeded(opt) + + def test_no_raise_when_another_rank_has_solution(self): + # locally everything failed, but allreduce_or reports a solution on + # some other rank -> the raise decision must be False on every rank + opt = _opt(True, {"s1": _scenario(solution_available=False)}, phiter=1) + opt.allreduce_or = lambda local: True + PHBase._check_prox_solve_succeeded(opt) + + +class TestReraiseAsProxCapabilityError(unittest.TestCase): + """The reactive check is unreachable when a solver signals 'no quadratic + objective' by *raising* during the solve (e.g. cbc/glpk, whose LP writer + raises before any solution is returned). This converts that raise into the + same actionable message, preserving the original via exception chaining. + """ + + def test_reraises_actionable_message_at_iter1(self): + opt = _opt(True, {"s1": _scenario()}, phiter=1) + original = ValueError("contains nonlinear terms") + with self.assertRaisesRegex(RuntimeError, "linearize-proximal-terms") as cm: + PHBase._reraise_as_prox_capability_error(opt, original) + # the original solver error is preserved for debugging + self.assertIs(cm.exception.__cause__, original) + + def test_no_reraise_after_first_iteration(self): + # a later raise is a genuine solve error -> caller re-raises it as-is + opt = _opt(True, {"s1": _scenario()}, phiter=2) + self.assertIsNone( + PHBase._reraise_as_prox_capability_error(opt, ValueError("boom")) + ) + + def test_no_reraise_when_prox_linearized(self): + # linearized prox is not quadratic, so a raise is not a capability issue + opt = _opt(False, {"s1": _scenario()}, phiter=1) + self.assertIsNone( + PHBase._reraise_as_prox_capability_error(opt, ValueError("boom")) + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/mpisppy/utils/sputils.py b/mpisppy/utils/sputils.py index 39be62df3..5c9cad942 100644 --- a/mpisppy/utils/sputils.py +++ b/mpisppy/utils/sputils.py @@ -511,7 +511,35 @@ def _models_have_same_sense(models): def is_persistent(solver): return isinstance(solver, pyo.pyomo.solvers.plugins.solvers.persistent_solver.PersistentSolver) - + + +def solver_quadratic_objective_capability(solver_plugin): + """Tri-state probe of whether a solver can handle a quadratic objective. + + Returns ``True`` or ``False`` as reported by the legacy ``has_capability`` + API, or ``None`` when the capability cannot be determined. The APPSI and + newer ``pyomo.contrib.solver`` interfaces (e.g. ``appsi_highs``, ``highs``) + do not expose ``has_capability``, so they return ``None`` and the caller + must fall back to detecting a failed solve. ``None`` therefore means + "unknown", never "unsupported". + + Args: + solver_plugin: an instantiated Pyomo solver object (e.g. the object + attached as ``scenario._solver_plugin``). + + Returns: + bool or None + """ + has_capability = getattr(solver_plugin, "has_capability", None) + if has_capability is None: + return None + try: + return bool(has_capability("quadratic_objective")) + except Exception: + # Be conservative: any trouble querying capability means "unknown", + # so we never wrongly block a solve based on a capability probe. + return None + def ef_scenarios(ef): """ An iterator to give the scenario sub-models in an ef Args: diff --git a/run_coverage.bash b/run_coverage.bash index f6b841ad8..b5d9b2ec4 100755 --- a/run_coverage.bash +++ b/run_coverage.bash @@ -83,6 +83,9 @@ run_phase "test_options_reach_solver (serial; gurobi-only)" \ run_phase "test_nonant_validation (serial)" \ coverage run --rcfile=.coveragerc -m pytest mpisppy/tests/test_nonant_validation.py -v +run_phase "test_prox_solver_compat (serial)" \ + coverage run --rcfile=.coveragerc -m pytest mpisppy/tests/test_prox_solver_compat.py -v + run_phase "test_ph_main (serial)" \ coverage run --rcfile=.coveragerc mpisppy/tests/test_ph_main.py