BUG FIXES: Solver Refactor for IDAES Integration

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/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,20 @@ 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.
+        if 'options' in kwargs and 'solver_options' in kwargs:
+            raise ApplicationError(
+                "Both 'options' and 'solver_options' were requested. "
+                "Please use one or the other, not both."
+            )
+        elif 'options' in kwargs:
+            self.options = kwargs.pop('options')
+        elif 'solver_options' in kwargs:
+            self.solver_options = kwargs.pop('solver_options')
         super().__init__(**kwargs)
 
     #
@@ -376,6 +387,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 +406,29 @@ 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 hasattr(self, 'options'):
+            self.config.solver_options.set_value(self.options)
+        if hasattr(self, 'solver_options'):
+            self.config.solver_options.set_value(self.solver_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 ApplicationError(
+                "Both 'options' and 'solver_options' were declared "
+                "in the 'solve' call. 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 +469,13 @@ 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 = len(
+            list(model.component_map(ctype=Constraint))
+        )
+        legacy_results.problem.number_of_variables = len(
+            list(model.component_map(ctype=Var))
+        )
+        number_of_objectives = len(list(model.component_map(ctype=Objective)))
         legacy_results.problem.number_of_objectives = number_of_objectives
         if number_of_objectives == 1:
             obj = get_objective(model)
@@ -464,7 +502,15 @@ def _solution_handler(
         """Method to handle the preferred action for the solution"""
         symbol_map = SymbolMap()
         symbol_map.default_labeler = NumericLabeler('x')
-        model.solutions.add_symbol_map(symbol_map)
+        try:
+            model.solutions.add_symbol_map(symbol_map)
+        except AttributeError:
+            # Something wacky happens in IDAES due to the usage of ScalarBlock
+            # instead of PyomoModel. This is an attempt to fix that.
+            from pyomo.core.base.PyomoModel import ModelSolutions
+
+            setattr(model, 'solutions', ModelSolutions(model))
+            model.solutions.add_symbol_map(symbol_map)
         legacy_results._smap_id = id(symbol_map)
         delete_legacy_soln = True
         if load_solutions:
@@ -508,7 +554,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 +583,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 +610,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

@@ -14,6 +14,7 @@
 from pyomo.common import unittest
 from pyomo.common.config import ConfigDict
 from pyomo.contrib.solver import base
+from pyomo.common.errors import ApplicationError
 
 
 class TestSolverBase(unittest.TestCase):
@@ -272,6 +273,104 @@ 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.solver_options, {'max_iter': 6})
+
+        # Test case 2: Set later
+        solver = base.LegacySolverWrapper()
+        solver.solver_options = {'max_iter': 4, 'foo': 'bar'}
+        self.assertEqual(solver.solver_options, {'max_iter': 4, 'foo': 'bar'})
+
+        # Test case 3: 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 4: 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.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(ApplicationError):
+            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(ApplicationError):
+            solver._map_config(solver_options={'max_iter': 4}, options={'max_iter': 6})
+
     def test_map_results(self):
         # Unclear how to test this
         pass