Add efficiency for handling PyROS separation problem sub-solver errors

pyomo/contrib/pyros/separation_problem_methods.py

@@ -423,8 +423,13 @@ def get_worst_discrete_separation_solution(
     # violation of specified second-stage inequality
     # constraint by separation
     # problem solutions for all scenarios
+    # scenarios with subsolver errors are replaced with nan
     violations_of_ss_ineq_con = [
-        solve_call_res.scaled_violations[ss_ineq_con]
+        (
+            solve_call_res.scaled_violations[ss_ineq_con]
+            if not solve_call_res.subsolver_error
+            else np.nan
+        )
         for solve_call_res in discrete_solve_results.solver_call_results.values()
     ]
 
@@ -433,9 +438,9 @@ def get_worst_discrete_separation_solution(
     # determine separation solution for which scaled violation of this
     # second-stage inequality constraint is the worst
     worst_case_res = discrete_solve_results.solver_call_results[
-        list_of_scenario_idxs[np.argmax(violations_of_ss_ineq_con)]
+        list_of_scenario_idxs[np.nanargmax(violations_of_ss_ineq_con)]
     ]
-    worst_case_violation = np.max(violations_of_ss_ineq_con)
+    worst_case_violation = np.nanmax(violations_of_ss_ineq_con)
     assert worst_case_violation in worst_case_res.scaled_violations.values()
 
     # evaluate violations for specified second-stage inequality constraints
@@ -463,6 +468,13 @@ def get_worst_discrete_separation_solution(
     else:
         results_list = []
 
+    # check if there were any failed scenarios for subsolver_error
+    # this only needs to be returned once for the subsolver error efficiency
+    if any(np.isnan(violations_of_ss_ineq_con)) and is_optimized_ss_ineq_con:
+        subsolver_error_flag = True
+    else:
+        subsolver_error_flag = False
+
     return SeparationSolveCallResults(
         solved_globally=worst_case_res.solved_globally,
         results_list=results_list,
@@ -471,7 +483,7 @@ def get_worst_discrete_separation_solution(
         variable_values=worst_case_res.variable_values,
         found_violation=(worst_case_violation > config.robust_feasibility_tolerance),
         time_out=False,
-        subsolver_error=False,
+        subsolver_error=subsolver_error_flag,
         discrete_set_scenario_index=worst_case_res.discrete_set_scenario_index,
     )
 
@@ -642,9 +654,7 @@ def perform_separation_loop(separation_data, master_data, solve_globally):
 
             priority_group_solve_call_results[ss_ineq_con] = solve_call_results
 
-            termination_not_ok = (
-                solve_call_results.time_out or solve_call_results.subsolver_error
-            )
+            termination_not_ok = solve_call_results.time_out
             if termination_not_ok:
                 all_solve_call_results.update(priority_group_solve_call_results)
                 return SeparationLoopResults(
@@ -653,6 +663,14 @@ def perform_separation_loop(separation_data, master_data, solve_globally):
                     worst_case_ss_ineq_con=None,
                 )
 
+            # provide message that PyROS will attempt to find a violation and move
+            # to the next iteration even after subsolver error
+            if solve_call_results.subsolver_error:
+                config.progress_logger.warning(
+                    "PyROS is attempting to recover and will continue to "
+                    "the next iteration if a constraint violation is found."
+                )
+
         all_solve_call_results.update(priority_group_solve_call_results)
 
         # there may be multiple separation problem solutions
@@ -1158,12 +1176,17 @@ def discrete_solve(
         solve_call_results_dict[scenario_idx] = solve_call_results
 
         # halt at first encounter of unacceptable termination
-        termination_not_ok = (
-            solve_call_results.subsolver_error or solve_call_results.time_out
-        )
+        termination_not_ok = solve_call_results.time_out
         if termination_not_ok:
             break
 
+        # report any subsolver errors, but continue
+        if solve_call_results.subsolver_error:
+            config.progress_logger.warning(
+                f"All solvers failed to solve discrete scenario {scenario_idx}: "
+                f"{config.uncertainty_set.scenarios[scenario_idx]}"
+            )
+
     return DiscreteSeparationSolveCallResults(
         solved_globally=solve_globally,
         solver_call_results=solve_call_results_dict,

pyomo/contrib/pyros/solve_data.py

@@ -277,11 +277,11 @@ def time_out(self):
     @property
     def subsolver_error(self):
         """
-        bool : True if there is a subsolver error status for at least
-        one of the the ``SeparationSolveCallResults`` objects listed
+        bool : True if there is a subsolver error status for all
+        of the the ``SeparationSolveCallResults`` objects listed
         in `self`, False otherwise.
         """
-        return any(res.subsolver_error for res in self.solver_call_results.values())
+        return all(res.subsolver_error for res in self.solver_call_results.values())
 
 
 class SeparationLoopResults:
@@ -430,11 +430,14 @@ def subsolver_error(self):
         """
         bool : Return True if subsolver error reported for
         at least one ``SeparationSolveCallResults`` stored in
-        `self`, False otherwise.
+        `self` and no violations are found, False otherwise.
         """
-        return any(
-            solver_call_res.subsolver_error
-            for solver_call_res in self.solver_call_results.values()
+        return (
+            any(
+                solver_call_res.subsolver_error
+                for solver_call_res in self.solver_call_results.values()
+            )
+            and not self.found_violation
         )
 
     @property

pyomo/contrib/pyros/tests/test_grcs.py

@@ -17,6 +17,7 @@
 import math
 import time
 
+from parameterized import parameterized
 import pyomo.common.unittest as unittest
 from pyomo.common.log import LoggingIntercept
 from pyomo.common.collections import Bunch
@@ -3144,5 +3145,217 @@ def test_pyros_invalid_bypass_separation(self):
             )
 
 
+# @SolverFactory.register("subsolver_error__solver")
+class SubsolverErrorSolver(object):
+    """
+    Solver that returns a bad termination condition
+    to purposefully create an SP subsolver error.
+
+    Parameters
+    ----------
+    sub_solver: SolverFactory
+        The wrapped solver object
+    all_fail: bool
+        Set to true to always return a subsolver error.
+        Otherwise, the solver checks `failed_flag` to see if it should behave normally or error.
+        The solver sets `failed_flag=True` after returning an error, and subsequent solves
+        should behave normally unless `failed_flag` is manually toggled off again.
+
+    Attributes
+    ----------
+    failed_flag
+    """
+
+    def __init__(self, sub_solver, all_fail):
+        self.sub_solver = sub_solver
+        self.all_fail = all_fail
+
+        self.failed_flag = False
+        self.options = Bunch()
+
+    def available(self, exception_flag=True):
+        return True
+
+    def license_is_valid(self):
+        return True
+
+    def __enter__(self):
+        return self
+
+    def __exit__(self, et, ev, tb):
+        pass
+
+    def solve(self, model, **kwargs):
+        """
+        'Solve' a model.
+
+        Parameters
+        ----------
+        model : ConcreteModel
+            Model of interest.
+
+        Returns
+        -------
+        results : SolverResults
+            Solver results.
+        """
+
+        # ensure only one active objective
+        active_objs = [
+            obj for obj in model.component_data_objects(Objective, active=True)
+        ]
+        assert len(active_objs) == 1
+
+        # check if a separation problem is being solved
+        # this is done by checking if there is a separation objective
+        sp_check = hasattr(model, 'separation_obj_0')
+        if sp_check:
+            # check if the problem needs to fail
+            if not self.failed_flag or self.all_fail:
+                # set up results.solver
+                results = SolverResults()
+
+                results.solver.termination_condition = TerminationCondition.error
+                results.solver.status = SolverStatus.error
+
+                # record that a failure has been produced
+                self.failed_flag = True
+
+                return results
+
+        # invoke subsolver
+        results = self.sub_solver.solve(model, **kwargs)
+
+        return results
+
+
+class TestPyROSSubsolverErrorEfficiency(unittest.TestCase):
+    """
+    Test PyROS subsolver error efficiency for continuous and discrete uncertainty sets.
+    """
+
+    @parameterized.expand(
+        [
+            ("failed_but_recovered_local", 7, False),
+            ("failed_and_terminated_local", 10, False),
+            ("failed_and_terminated_global", 7, True),
+        ]
+    )
+    def test_continuous_set_subsolver_error_recovery(
+        self, name, sec_con_UB, test_global_error
+    ):
+        m = build_leyffer_two_cons()
+        # the following constraint is unviolated/violated depending on the UB
+        # if the constraint is unviolated, no other violations are found, and
+        # PyROS should terminate with subsolver error.
+        # if the constraint is violated, PyROS can continue to the next iteration
+        # despite subsolver errors.
+        m.sec_con = Constraint(expr=m.u * m.x1 <= sec_con_UB)
+        m.sec_con.pprint()
+
+        # Define the uncertainty set
+        interval = BoxSet(bounds=[(0.25, 2)])
+
+        # Instantiate the PyROS solver
+        pyros_solver = SolverFactory("pyros")
+
+        # Define subsolvers utilized in the algorithm
+        # the error solver will cause the first separation problem to fail
+        local_subsolver = SubsolverErrorSolver(
+            sub_solver=SolverFactory('ipopt'), all_fail=False
+        )
+        if test_global_error:
+            global_subsolver = SubsolverErrorSolver(
+                sub_solver=SolverFactory('baron'), all_fail=False
+            )
+        else:
+            global_subsolver = SolverFactory("baron")
+
+        # Call the PyROS solver
+        results = pyros_solver.solve(
+            model=m,
+            first_stage_variables=[m.x1],
+            second_stage_variables=[m.x2],
+            uncertain_params=[m.u],
+            uncertainty_set=interval,
+            local_solver=local_subsolver,
+            global_solver=global_subsolver,
+            options={
+                "objective_focus": ObjectiveType.worst_case,
+                "solve_master_globally": True,
+            },
+        )
+
+        if 'recovered' in name:
+            # check successful termination
+            self.assertEqual(
+                results.pyros_termination_condition,
+                pyrosTerminationCondition.robust_optimal,
+                msg="Did not identify robust optimal solution to problem instance.",
+            )
+        else:
+            # check unsuccessful termination
+            self.assertEqual(
+                results.pyros_termination_condition,
+                pyrosTerminationCondition.subsolver_error,
+                msg="Did not report subsolver error to problem instance.",
+            )
+
+    @parameterized.expand(
+        [("failed_but_recovered_local", 7), ("failed_and_terminated_local", 10)]
+    )
+    def test_discrete_set_subsolver_error_recovery(self, name, sec_con_UB):
+        m = build_leyffer_two_cons()
+        # the following constraint is unviolated/violated depending on the UB
+        # if the constraint is unviolated, no other violations are found, and
+        # PyROS should terminate with subsolver error.
+        # if the constraint is violated, PyROS can continue to the next iteration
+        # despite subsolver errors.
+        m.sec_con = Constraint(expr=m.u * m.x1 <= sec_con_UB)
+
+        # Define the uncertainty set
+        discrete_set = DiscreteScenarioSet(scenarios=[[0.25], [1.125], [2]])
+
+        # Instantiate the PyROS solver
+        pyros_solver = SolverFactory("pyros")
+
+        # Define subsolvers utilized in the algorithm
+        # the error solver will cause the first separation problem to fail
+        local_subsolver = SubsolverErrorSolver(
+            sub_solver=SolverFactory('ipopt'), all_fail=False
+        )
+        global_subsolver = SolverFactory("baron")
+
+        # Call the PyROS solver
+        results = pyros_solver.solve(
+            model=m,
+            first_stage_variables=[m.x1],
+            second_stage_variables=[m.x2],
+            uncertain_params=[m.u],
+            uncertainty_set=discrete_set,
+            local_solver=local_subsolver,
+            global_solver=global_subsolver,
+            options={
+                "objective_focus": ObjectiveType.worst_case,
+                "solve_master_globally": True,
+            },
+        )
+
+        if 'recovered' in name:
+            # check successful termination
+            self.assertEqual(
+                results.pyros_termination_condition,
+                pyrosTerminationCondition.robust_optimal,
+                msg="Did not identify robust optimal solution to problem instance.",
+            )
+        else:
+            # check unsuccessful termination
+            self.assertEqual(
+                results.pyros_termination_condition,
+                pyrosTerminationCondition.subsolver_error,
+                msg="Did not report subsolver error to problem instance.",
+            )
+
+
 if __name__ == "__main__":
     unittest.main()