From 08970d817c5aeae6cca25026ac1509394ffc8da5 Mon Sep 17 00:00:00 2001 From: Jason Yao Date: Tue, 3 Dec 2024 16:32:08 -0500 Subject: [PATCH 1/7] added efficiency for handling SP subsolver errors --- .../pyros/separation_problem_methods.py | 39 ++++++++++++++----- pyomo/contrib/pyros/solve_data.py | 11 ++++-- 2 files changed, 36 insertions(+), 14 deletions(-) diff --git a/pyomo/contrib/pyros/separation_problem_methods.py b/pyomo/contrib/pyros/separation_problem_methods.py index 4bb9eb7b32b..ca1f294ba5a 100644 --- a/pyomo/contrib/pyros/separation_problem_methods.py +++ b/pyomo/contrib/pyros/separation_problem_methods.py @@ -424,7 +424,11 @@ def get_worst_discrete_separation_solution( # constraint by separation # problem solutions for all scenarios 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 +437,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 +467,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 to trigger 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 +482,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 +653,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( @@ -652,6 +661,11 @@ def perform_separation_loop(separation_data, master_data, solve_globally): solved_globally=solve_globally, worst_case_ss_ineq_con=None, ) + 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) @@ -1158,12 +1172,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, diff --git a/pyomo/contrib/pyros/solve_data.py b/pyomo/contrib/pyros/solve_data.py index db403212eb4..3445eb99266 100644 --- a/pyomo/contrib/pyros/solve_data.py +++ b/pyomo/contrib/pyros/solve_data.py @@ -281,7 +281,7 @@ def subsolver_error(self): one 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: @@ -432,9 +432,12 @@ def subsolver_error(self): at least one ``SeparationSolveCallResults`` stored in `self`, 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 From 77e88f2495d9009e02aa43591a83f5c00d6c7cb6 Mon Sep 17 00:00:00 2001 From: Jason Yao Date: Tue, 3 Dec 2024 16:33:10 -0500 Subject: [PATCH 2/7] added unit tests for subsolver error efficiency --- pyomo/contrib/pyros/tests/test_grcs.py | 211 +++++++++++++++++++++++++ 1 file changed, 211 insertions(+) diff --git a/pyomo/contrib/pyros/tests/test_grcs.py b/pyomo/contrib/pyros/tests/test_grcs.py index ebb2c8b7a37..ecab6a092c8 100644 --- a/pyomo/contrib/pyros/tests/test_grcs.py +++ b/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,215 @@ 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 + 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 + 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() From 0038ffc772de85467e4f3244d50ec1903d5b0a75 Mon Sep 17 00:00:00 2001 From: Jason Yao Date: Tue, 3 Dec 2024 17:00:05 -0500 Subject: [PATCH 3/7] added and updated comments --- pyomo/contrib/pyros/separation_problem_methods.py | 6 +++++- pyomo/contrib/pyros/solve_data.py | 6 +++--- pyomo/contrib/pyros/tests/test_grcs.py | 2 ++ 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/pyomo/contrib/pyros/separation_problem_methods.py b/pyomo/contrib/pyros/separation_problem_methods.py index ca1f294ba5a..e39d0e4edf1 100644 --- a/pyomo/contrib/pyros/separation_problem_methods.py +++ b/pyomo/contrib/pyros/separation_problem_methods.py @@ -423,6 +423,7 @@ 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] @@ -468,7 +469,7 @@ def get_worst_discrete_separation_solution( results_list = [] # check if there were any failed scenarios for subsolver_error - # this only needs to be returned once to trigger subsolver error efficiency + # 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: @@ -661,6 +662,9 @@ def perform_separation_loop(separation_data, master_data, solve_globally): solved_globally=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 " diff --git a/pyomo/contrib/pyros/solve_data.py b/pyomo/contrib/pyros/solve_data.py index 3445eb99266..bc0dd41d0c3 100644 --- a/pyomo/contrib/pyros/solve_data.py +++ b/pyomo/contrib/pyros/solve_data.py @@ -277,8 +277,8 @@ 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 all(res.subsolver_error for res in self.solver_call_results.values()) @@ -430,7 +430,7 @@ 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( diff --git a/pyomo/contrib/pyros/tests/test_grcs.py b/pyomo/contrib/pyros/tests/test_grcs.py index ecab6a092c8..a99a8c801a7 100644 --- a/pyomo/contrib/pyros/tests/test_grcs.py +++ b/pyomo/contrib/pyros/tests/test_grcs.py @@ -3260,6 +3260,7 @@ def test_continuous_set_subsolver_error_recovery( 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 ) @@ -3319,6 +3320,7 @@ def test_discrete_set_subsolver_error_recovery(self, name, sec_con_UB): 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 ) From 94e97322db2e3f91f59cb938ad518d06b0a5e683 Mon Sep 17 00:00:00 2001 From: Jason Yao Date: Mon, 16 Dec 2024 09:52:02 -0500 Subject: [PATCH 4/7] add additional logging information --- pyomo/contrib/pyros/pyros_algorithm_methods.py | 7 +++++++ pyomo/contrib/pyros/separation_problem_methods.py | 10 ++++++++-- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/pyomo/contrib/pyros/pyros_algorithm_methods.py b/pyomo/contrib/pyros/pyros_algorithm_methods.py index 86e5d52935b..ba2ba2362c6 100644 --- a/pyomo/contrib/pyros/pyros_algorithm_methods.py +++ b/pyomo/contrib/pyros/pyros_algorithm_methods.py @@ -273,6 +273,13 @@ def ROSolver_iterative_solve(model_data): # terminate on time limit if separation_results.time_out or separation_results.subsolver_error: + # report PyROS failure to find violated constraint for subsolver error + if separation_results.subsolver_error: + config.progress_logger.warning( + "PyROS failed to find a constraint violation and " + "will terminate with sub-solver error." + ) + pyros_term_cond = ( pyrosTerminationCondition.time_out if separation_results.time_out diff --git a/pyomo/contrib/pyros/separation_problem_methods.py b/pyomo/contrib/pyros/separation_problem_methods.py index e39d0e4edf1..241763a86b8 100644 --- a/pyomo/contrib/pyros/separation_problem_methods.py +++ b/pyomo/contrib/pyros/separation_problem_methods.py @@ -469,8 +469,8 @@ def get_worst_discrete_separation_solution( 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: + # if there are failed scenarios, subsolver error triggers for all ineq + if any(np.isnan(violations_of_ss_ineq_con)): subsolver_error_flag = True else: subsolver_error_flag = False @@ -1164,6 +1164,12 @@ def discrete_solve( for param, coord_val in zip(uncertain_param_vars, scenario): param.fix(coord_val) + # debug statement for solving square problem for each scenario + # TODO add information on total number of scenarios not added to MP + config.progress_logger.debug( + f"Attempting to solve square problem for discrete scenario {scenario_idx}" + ) + # obtain separation problem solution solve_call_results = solver_call_separation( separation_data=separation_data, From 2c05c094b1f41af9d05c308f95023aa75f63bcc8 Mon Sep 17 00:00:00 2001 From: Jason Yao Date: Fri, 20 Dec 2024 14:30:54 -0500 Subject: [PATCH 5/7] update discrete scenario debug logging --- pyomo/contrib/pyros/separation_problem_methods.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pyomo/contrib/pyros/separation_problem_methods.py b/pyomo/contrib/pyros/separation_problem_methods.py index 241763a86b8..322fa6a542b 100644 --- a/pyomo/contrib/pyros/separation_problem_methods.py +++ b/pyomo/contrib/pyros/separation_problem_methods.py @@ -1157,7 +1157,7 @@ def discrete_solve( ] solve_call_results_dict = {} - for scenario_idx in scenario_idxs_to_separate: + for idx, scenario_idx in enumerate(scenario_idxs_to_separate): # fix uncertain parameters to scenario value # hence, no need to activate uncertainty set constraints scenario = config.uncertainty_set.scenarios[scenario_idx] @@ -1165,9 +1165,9 @@ def discrete_solve( param.fix(coord_val) # debug statement for solving square problem for each scenario - # TODO add information on total number of scenarios not added to MP config.progress_logger.debug( - f"Attempting to solve square problem for discrete scenario {scenario_idx}" + f"Attempting to solve square problem for discrete scenario {scenario}" + f", {idx + 1} of {len(scenario_idxs_to_separate)} total" ) # obtain separation problem solution @@ -1190,7 +1190,7 @@ def discrete_solve( 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]}" + f"{scenario}" ) return DiscreteSeparationSolveCallResults( From 1f599033c9e243e6f6dd7b9e277dff633011eaaf Mon Sep 17 00:00:00 2001 From: Jason Yao Date: Fri, 20 Dec 2024 14:49:32 -0500 Subject: [PATCH 6/7] added skipUnless solver available for unit tests --- pyomo/contrib/pyros/tests/test_grcs.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pyomo/contrib/pyros/tests/test_grcs.py b/pyomo/contrib/pyros/tests/test_grcs.py index a99a8c801a7..fab6347fef2 100644 --- a/pyomo/contrib/pyros/tests/test_grcs.py +++ b/pyomo/contrib/pyros/tests/test_grcs.py @@ -3229,6 +3229,11 @@ def solve(self, model, **kwargs): return results +@unittest.skipUnless(ipopt_available, "IPOPT is not available.") +@unittest.skipUnless( + baron_available and baron_license_is_valid, + "Global NLP solver is not available and licensed.", +) class TestPyROSSubsolverErrorEfficiency(unittest.TestCase): """ Test PyROS subsolver error efficiency for continuous and discrete uncertainty sets. From 45b5a460fe9c98ee7cad4bb22da680ddabed78bf Mon Sep 17 00:00:00 2001 From: Jason Yao Date: Thu, 6 Feb 2025 09:42:22 -0500 Subject: [PATCH 7/7] Update version number and changelog --- doc/OnlineDocs/explanation/solvers/pyros.rst | 2 +- pyomo/contrib/pyros/CHANGELOG.txt | 10 ++++++++++ pyomo/contrib/pyros/pyros.py | 2 +- 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/doc/OnlineDocs/explanation/solvers/pyros.rst b/doc/OnlineDocs/explanation/solvers/pyros.rst index f96b9285240..cfaef1b9a93 100644 --- a/doc/OnlineDocs/explanation/solvers/pyros.rst +++ b/doc/OnlineDocs/explanation/solvers/pyros.rst @@ -958,7 +958,7 @@ Observe that the log contains the following information: :linenos: ============================================================================== - PyROS: The Pyomo Robust Optimization Solver, v1.3.2. + PyROS: The Pyomo Robust Optimization Solver, v1.3.3. Pyomo version: 6.9.0 Commit hash: unknown Invoked at UTC 2024-11-01T00:00:00.000000 diff --git a/pyomo/contrib/pyros/CHANGELOG.txt b/pyomo/contrib/pyros/CHANGELOG.txt index a17fbf23641..d3934b1c1fe 100644 --- a/pyomo/contrib/pyros/CHANGELOG.txt +++ b/pyomo/contrib/pyros/CHANGELOG.txt @@ -2,6 +2,16 @@ PyROS CHANGELOG =============== +------------------------------------------------------------------------------- +PyROS 1.3.3 03 Dec 2024 +------------------------------------------------------------------------------- +- Add efficiency for handling PyROS separation problem sub-solver errors +- Add logger warnings to report sub-solver errors and inform that PyROS + will continue to solve if a violation is found +- Add unit tests for new sub-solver error handling for continuous + and discrete uncertainty sets + + ------------------------------------------------------------------------------- PyROS 1.3.2 29 Nov 2024 ------------------------------------------------------------------------------- diff --git a/pyomo/contrib/pyros/pyros.py b/pyomo/contrib/pyros/pyros.py index 3044658cb39..02ecbbcf6e8 100644 --- a/pyomo/contrib/pyros/pyros.py +++ b/pyomo/contrib/pyros/pyros.py @@ -33,7 +33,7 @@ ) -__version__ = "1.3.2" +__version__ = "1.3.3" default_pyros_solver_logger = setup_pyros_logger()