Merge branch 'Pyomo:main' into fix-pyros-subsolver-time-limits

Author: shermanjasonaf

doc/OnlineDocs/developer_reference/solvers.rst

@@ -84,6 +84,37 @@ be used with other Pyomo tools / capabilities.
    ...
    3 Declarations: x y obj
 
+In keeping with our commitment to backwards compatibility, both the legacy and
+future methods of specifying solver options are supported:
+
+.. testcode::
+   :skipif: not ipopt_available
+
+   import pyomo.environ as pyo
+
+   model = pyo.ConcreteModel()
+   model.x = pyo.Var(initialize=1.5)
+   model.y = pyo.Var(initialize=1.5)
+
+   def rosenbrock(model):
+       return (1.0 - model.x) ** 2 + 100.0 * (model.y - model.x**2) ** 2
+
+   model.obj = pyo.Objective(rule=rosenbrock, sense=pyo.minimize)
+
+   # Backwards compatible
+   status = pyo.SolverFactory('ipopt_v2').solve(model, options={'max_iter' : 6})
+   # Forwards compatible
+   status = pyo.SolverFactory('ipopt_v2').solve(model, solver_options={'max_iter' : 6})
+   model.pprint()
+
+.. testoutput::
+   :skipif: not ipopt_available
+   :hide:
+
+   2 Var Declarations
+   ...
+   3 Declarations: x y obj
+
 Using the new interfaces directly
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 

pyomo/contrib/gdpopt/tests/test_LBB.py

@@ -59,6 +59,7 @@ def test_infeasible_GDP(self):
         self.assertIsNone(m.d.disjuncts[0].indicator_var.value)
         self.assertIsNone(m.d.disjuncts[1].indicator_var.value)
 
+    @unittest.skipUnless(z3_available, "Z3 SAT solver is not available")
     def test_infeasible_GDP_check_sat(self):
         """Test for infeasible GDP with check_sat option True."""
         m = ConcreteModel()

pyomo/contrib/gdpopt/tests/test_gdpopt.py

@@ -22,7 +22,6 @@
 from pyomo.common.collections import Bunch
 from pyomo.common.config import ConfigDict, ConfigValue
 from pyomo.common.fileutils import import_file, PYOMO_ROOT_DIR
-from pyomo.contrib.appsi.solvers.gurobi import Gurobi
 from pyomo.contrib.gdpopt.create_oa_subproblems import (
     add_util_block,
     add_disjunct_list,
@@ -767,6 +766,9 @@ def test_time_limit(self):
             results.solver.termination_condition, TerminationCondition.maxTimeLimit
         )
 
+    @unittest.skipUnless(
+        license_available, "No BARON license--8PP logical problem exceeds demo size"
+    )
     def test_LOA_8PP_logical_default_init(self):
         """Test logic-based outer approximation with 8PP."""
         exfile = import_file(join(exdir, 'eight_process', 'eight_proc_logical.py'))
@@ -870,6 +872,9 @@ def test_LOA_8PP_maxBinary(self):
         )
         ct.check_8PP_solution(self, eight_process, results)
 
+    @unittest.skipUnless(
+        license_available, "No BARON license--8PP logical problem exceeds demo size"
+    )
     def test_LOA_8PP_logical_maxBinary(self):
         """Test logic-based OA with max_binary initialization."""
         exfile = import_file(join(exdir, 'eight_process', 'eight_proc_logical.py'))
@@ -1050,7 +1055,11 @@ def assert_correct_disjuncts_active(
 
         self.assertTrue(fabs(value(eight_process.profit.expr) - 68) <= 1e-2)
 
-    @unittest.skipUnless(Gurobi().available(), "APPSI Gurobi solver is not available")
+    @unittest.skipUnless(
+        SolverFactory('appsi_gurobi').available(exception_flag=False)
+        and SolverFactory('appsi_gurobi').license_is_valid(),
+        "Legacy APPSI Gurobi solver is not available",
+    )
     def test_auto_persistent_solver(self):
         exfile = import_file(join(exdir, 'eight_process', 'eight_proc_model.py'))
         m = exfile.build_eight_process_flowsheet()
@@ -1126,6 +1135,9 @@ def test_RIC_8PP_default_init(self):
         )
         ct.check_8PP_solution(self, eight_process, results)
 
+    @unittest.skipUnless(
+        license_available, "No BARON license--8PP logical problem exceeds demo size"
+    )
     def test_RIC_8PP_logical_default_init(self):
         """Test logic-based outer approximation with 8PP."""
         exfile = import_file(join(exdir, 'eight_process', 'eight_proc_logical.py'))

pyomo/contrib/gdpopt/util.py

@@ -553,6 +553,13 @@ def _add_bigm_constraint_to_transformed_model(m, constraint, block):
     # making a Reference to the ComponentData so that it will look like an
     # indexed component for now. If I redesign bigm at some point, then this
     # could be prettier.
-    bigm._transform_constraint(Reference(constraint), parent_disjunct, None, [], [])
+    bigm._transform_constraint(
+        Reference(constraint),
+        parent_disjunct,
+        None,
+        [],
+        [],
+        1 - parent_disjunct.binary_indicator_var,
+    )
     # Now get rid of it because this is a class attribute!
     del bigm._config

pyomo/contrib/solver/base.py

@@ -14,11 +14,11 @@
 from typing import Sequence, Dict, Optional, Mapping, NoReturn, List, Tuple
 import os
 
-from pyomo.core.base.constraint import _GeneralConstraintData
-from pyomo.core.base.var import _GeneralVarData
+from pyomo.core.base.constraint import Constraint, _GeneralConstraintData
+from pyomo.core.base.var import Var, _GeneralVarData
 from pyomo.core.base.param import _ParamData
 from pyomo.core.base.block import _BlockData
-from pyomo.core.base.objective import _GeneralObjectiveData
+from pyomo.core.base.objective import Objective, _GeneralObjectiveData
 from pyomo.common.config import document_kwargs_from_configdict, ConfigValue
 from pyomo.common.errors import ApplicationError
 from pyomo.common.deprecation import deprecation_warning
@@ -348,9 +348,19 @@ class LegacySolverWrapper:
     interface. Necessary for backwards compatibility.
     """
 
-    def __init__(self, solver_io=None, **kwargs):
-        if solver_io is not None:
+    def __init__(self, **kwargs):
+        if 'solver_io' in kwargs:
             raise NotImplementedError('Still working on this')
+        # There is no reason for a user to be trying to mix both old
+        # and new options. That is silly. So we will yell at them.
+        self.options = kwargs.pop('options', None)
+        if 'solver_options' in kwargs:
+            if self.options is not None:
+                raise ValueError(
+                    "Both 'options' and 'solver_options' were requested. "
+                    "Please use one or the other, not both."
+                )
+            self.options = kwargs.pop('solver_options')
         super().__init__(**kwargs)
 
     #
@@ -376,6 +386,8 @@ def _map_config(
         keepfiles=NOTSET,
         solnfile=NOTSET,
         options=NOTSET,
+        solver_options=NOTSET,
+        writer_config=NOTSET,
     ):
         """Map between legacy and new interface configuration options"""
         self.config = self.config()
@@ -393,8 +405,26 @@ def _map_config(
             self.config.time_limit = timelimit
         if report_timing is not NOTSET:
             self.config.report_timing = report_timing
-        if options is not NOTSET:
+        if self.options is not None:
+            self.config.solver_options.set_value(self.options)
+        if (options is not NOTSET) and (solver_options is not NOTSET):
+            # There is no reason for a user to be trying to mix both old
+            # and new options. That is silly. So we will yell at them.
+            # Example that would raise an error:
+            # solver.solve(model, options={'foo' : 'bar'}, solver_options={'foo' : 'not_bar'})
+            raise ValueError(
+                "Both 'options' and 'solver_options' were requested. "
+                "Please use one or the other, not both."
+            )
+        elif options is not NOTSET:
+            # This block is trying to mimic the existing logic in the legacy
+            # interface that allows users to pass initialized options to
+            # the solver object and override them in the solve call.
             self.config.solver_options.set_value(options)
+        elif solver_options is not NOTSET:
+            self.config.solver_options.set_value(solver_options)
+        if writer_config is not NOTSET:
+            self.config.writer_config.set_value(writer_config)
         # This is a new flag in the interface. To preserve backwards compatibility,
         # its default is set to "False"
         if raise_exception_on_nonoptimal_result is not NOTSET:
@@ -435,9 +465,14 @@ def _map_results(self, model, results):
         ]
         legacy_soln.status = legacy_solution_status_map[results.solution_status]
         legacy_results.solver.termination_message = str(results.termination_condition)
-        legacy_results.problem.number_of_constraints = model.nconstraints()
-        legacy_results.problem.number_of_variables = model.nvariables()
-        number_of_objectives = model.nobjectives()
+        legacy_results.problem.number_of_constraints = float('nan')
+        legacy_results.problem.number_of_variables = float('nan')
+        number_of_objectives = sum(
+            1
+            for _ in model.component_data_objects(
+                Objective, active=True, descend_into=True
+            )
+        )
         legacy_results.problem.number_of_objectives = number_of_objectives
         if number_of_objectives == 1:
             obj = get_objective(model)
@@ -508,7 +543,10 @@ def solve(
         options: Optional[Dict] = None,
         keepfiles: bool = False,
         symbolic_solver_labels: bool = False,
+        # These are for forward-compatibility
         raise_exception_on_nonoptimal_result: bool = False,
+        solver_options: Optional[Dict] = None,
+        writer_config: Optional[Dict] = None,
     ):
         """
         Solve method: maps new solve method style to backwards compatible version.
@@ -534,6 +572,8 @@ def solve(
             'keepfiles',
             'solnfile',
             'options',
+            'solver_options',
+            'writer_config',
         )
         loc = locals()
         filtered_args = {k: loc[k] for k in map_args if loc.get(k, None) is not None}
@@ -559,7 +599,10 @@ def available(self, exception_flag=True):
         """
         ans = super().available()
         if exception_flag and not ans:
-            raise ApplicationError(f'Solver {self.__class__} is not available ({ans}).')
+            raise ApplicationError(
+                f'Solver "{self.name}" is not available. '
+                f'The returned status is: {ans}.'
+            )
         return bool(ans)
 
     def license_is_valid(self) -> bool:

pyomo/contrib/solver/tests/unit/test_base.py

@@ -272,6 +272,99 @@ def test_map_config(self):
         with self.assertRaises(AttributeError):
             print(instance.config.keepfiles)
 
+    def test_solver_options_behavior(self):
+        # options can work in multiple ways (set from instantiation, set
+        # after instantiation, set during solve).
+        # Test case 1: Set at instantiation
+        solver = base.LegacySolverWrapper(options={'max_iter': 6})
+        self.assertEqual(solver.options, {'max_iter': 6})
+
+        # Test case 2: Set later
+        solver = base.LegacySolverWrapper()
+        solver.options = {'max_iter': 4, 'foo': 'bar'}
+        self.assertEqual(solver.options, {'max_iter': 4, 'foo': 'bar'})
+
+        # Test case 3: pass some options to the mapping (aka, 'solve' command)
+        solver = base.LegacySolverWrapper()
+        config = ConfigDict(implicit=True)
+        config.declare(
+            'solver_options',
+            ConfigDict(implicit=True, description="Options to pass to the solver."),
+        )
+        solver.config = config
+        solver._map_config(options={'max_iter': 4})
+        self.assertEqual(solver.config.solver_options, {'max_iter': 4})
+
+        # Test case 4: Set at instantiation and override during 'solve' call
+        solver = base.LegacySolverWrapper(options={'max_iter': 6})
+        config = ConfigDict(implicit=True)
+        config.declare(
+            'solver_options',
+            ConfigDict(implicit=True, description="Options to pass to the solver."),
+        )
+        solver.config = config
+        solver._map_config(options={'max_iter': 4})
+        self.assertEqual(solver.config.solver_options, {'max_iter': 4})
+        self.assertEqual(solver.options, {'max_iter': 6})
+
+        # solver_options are also supported
+        # Test case 1: set at instantiation
+        solver = base.LegacySolverWrapper(solver_options={'max_iter': 6})
+        self.assertEqual(solver.options, {'max_iter': 6})
+
+        # Test case 2: pass some solver_options to the mapping (aka, 'solve' command)
+        solver = base.LegacySolverWrapper()
+        config = ConfigDict(implicit=True)
+        config.declare(
+            'solver_options',
+            ConfigDict(implicit=True, description="Options to pass to the solver."),
+        )
+        solver.config = config
+        solver._map_config(solver_options={'max_iter': 4})
+        self.assertEqual(solver.config.solver_options, {'max_iter': 4})
+
+        # Test case 3: Set at instantiation and override during 'solve' call
+        solver = base.LegacySolverWrapper(solver_options={'max_iter': 6})
+        config = ConfigDict(implicit=True)
+        config.declare(
+            'solver_options',
+            ConfigDict(implicit=True, description="Options to pass to the solver."),
+        )
+        solver.config = config
+        solver._map_config(solver_options={'max_iter': 4})
+        self.assertEqual(solver.config.solver_options, {'max_iter': 4})
+        self.assertEqual(solver.options, {'max_iter': 6})
+
+        # users can mix... sort of
+        # Test case 1: Initialize with options, solve with solver_options
+        solver = base.LegacySolverWrapper(options={'max_iter': 6})
+        config = ConfigDict(implicit=True)
+        config.declare(
+            'solver_options',
+            ConfigDict(implicit=True, description="Options to pass to the solver."),
+        )
+        solver.config = config
+        solver._map_config(solver_options={'max_iter': 4})
+        self.assertEqual(solver.config.solver_options, {'max_iter': 4})
+
+        # users CANNOT initialize both values at the same time, because how
+        # do we know what to do with it then?
+        # Test case 1: Class instance
+        with self.assertRaises(ValueError):
+            solver = base.LegacySolverWrapper(
+                options={'max_iter': 6}, solver_options={'max_iter': 4}
+            )
+        # Test case 2: Passing to `solve`
+        solver = base.LegacySolverWrapper()
+        config = ConfigDict(implicit=True)
+        config.declare(
+            'solver_options',
+            ConfigDict(implicit=True, description="Options to pass to the solver."),
+        )
+        solver.config = config
+        with self.assertRaises(ValueError):
+            solver._map_config(solver_options={'max_iter': 4}, options={'max_iter': 6})
+
     def test_map_results(self):
         # Unclear how to test this
         pass

pyomo/core/expr/base.py

@@ -360,7 +360,7 @@ def size(self):
         """
         return visitor.sizeof_expression(self)
 
-    def _apply_operation(self, result):  # pragma: no cover
+    def _apply_operation(self, result):
         """
         Compute the values of this node given the values of its children.