Merge pull request #3214 from mrmundt/sol-refac-nconstraint

jsiirola · web-flow · commit c68ce6614fe5 · 2024-04-03T15:46:54.000-06:00
BUG FIXES: Solver Refactor for IDAES Integration
diff --git a/doc/OnlineDocs/developer_reference/solvers.rst b/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
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
diff --git a/pyomo/contrib/solver/base.py b/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:
diff --git a/pyomo/contrib/solver/tests/unit/test_base.py b/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
