Merge pull request #3334 from jsiirola/legacy-solver-options

LegacySolverWrapper: restore 'options' attribute

pyomo/contrib/solver/base.py

@@ -353,15 +353,20 @@ def __init__(self, **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)
+        _options = kwargs.pop('options', None)
         if 'solver_options' in kwargs:
-            if self.options is not None:
+            if _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')
+            _options = kwargs.pop('solver_options')
+        if _options is not None:
+            kwargs['solver_options'] = _options
         super().__init__(**kwargs)
+        # Make the legacy 'options' attribute an alias of the new
+        # config.solver_options
+        self.options = self.config.solver_options
 
     #
     # Support "with" statements
@@ -372,6 +377,14 @@ def __enter__(self):
     def __exit__(self, t, v, traceback):
         """Exit statement - enables `with` statements."""
 
+    def __setattr__(self, attr, value):
+        # 'options' and 'config' are really singleton attributes.  Map
+        # any assignment to set_value()
+        if attr in ('options', 'config') and attr in self.__dict__:
+            getattr(self, attr).set_value(value)
+        else:
+            super().__setattr__(attr, value)
+
     def _map_config(
         self,
         tee=NOTSET,
@@ -390,7 +403,6 @@ def _map_config(
         writer_config=NOTSET,
     ):
         """Map between legacy and new interface configuration options"""
-        self.config = self.config()
         if 'report_timing' not in self.config:
             self.config.declare(
                 'report_timing', ConfigValue(domain=bool, default=False)
@@ -405,8 +417,6 @@ def _map_config(
             self.config.time_limit = timelimit
         if report_timing is not NOTSET:
             self.config.report_timing = report_timing
-        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.

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

@@ -16,6 +16,10 @@
 from pyomo.contrib.solver import base
 
 
+class _LegacyWrappedSolverBase(base.LegacySolverWrapper, base.SolverBase):
+    pass
+
+
 class TestSolverBase(unittest.TestCase):
     def test_abstract_member_list(self):
         expected_list = ['solve', 'available', 'version']
@@ -192,11 +196,13 @@ def test_class_method_list(self):
         ]
         self.assertEqual(sorted(expected_list), sorted(method_list))
 
+    @unittest.mock.patch.multiple(_LegacyWrappedSolverBase, __abstractmethods__=set())
     def test_context_manager(self):
-        with base.LegacySolverWrapper() as instance:
-            with self.assertRaises(AttributeError):
-                instance.available()
+        with _LegacyWrappedSolverBase() as instance:
+            self.assertIsInstance(instance, _LegacyWrappedSolverBase)
+            self.assertFalse(instance.available(False))
 
+    @unittest.mock.patch.multiple(_LegacyWrappedSolverBase, __abstractmethods__=set())
     def test_map_config(self):
         # Create a fake/empty config structure that can be added to an empty
         # instance of LegacySolverWrapper
@@ -205,7 +211,7 @@ def test_map_config(self):
             'solver_options',
             ConfigDict(implicit=True, description="Options to pass to the solver."),
         )
-        instance = base.LegacySolverWrapper()
+        instance = _LegacyWrappedSolverBase()
         instance.config = self.config
         instance._map_config(
             True, False, False, 20, True, False, None, None, None, False, None, None
@@ -272,98 +278,81 @@ def test_map_config(self):
         with self.assertRaises(AttributeError):
             print(instance.config.keepfiles)
 
+    @unittest.mock.patch.multiple(_LegacyWrappedSolverBase, __abstractmethods__=set())
     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})
+        solver = _LegacyWrappedSolverBase(options={'max_iter': 6})
         self.assertEqual(solver.options, {'max_iter': 6})
+        self.assertEqual(solver.config.solver_options, {'max_iter': 6})
 
         # Test case 2: Set later
-        solver = base.LegacySolverWrapper()
+        solver = _LegacyWrappedSolverBase()
         solver.options = {'max_iter': 4, 'foo': 'bar'}
         self.assertEqual(solver.options, {'max_iter': 4, 'foo': 'bar'})
+        self.assertEqual(solver.config.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 = _LegacyWrappedSolverBase()
         solver._map_config(options={'max_iter': 4})
+        self.assertEqual(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(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 = _LegacyWrappedSolverBase(options={'max_iter': 6})
         solver._map_config(options={'max_iter': 4})
+        self.assertEqual(solver.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})
+        solver = _LegacyWrappedSolverBase(solver_options={'max_iter': 6})
         self.assertEqual(solver.options, {'max_iter': 6})
+        self.assertEqual(solver.config.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 = _LegacyWrappedSolverBase()
         solver._map_config(solver_options={'max_iter': 4})
+        self.assertEqual(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 = _LegacyWrappedSolverBase(solver_options={'max_iter': 6})
         solver._map_config(solver_options={'max_iter': 4})
+        self.assertEqual(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 = _LegacyWrappedSolverBase(options={'max_iter': 6})
         solver._map_config(solver_options={'max_iter': 4})
+        self.assertEqual(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(
+            solver = _LegacyWrappedSolverBase(
                 options={'max_iter': 6}, solver_options={'max_iter': 4}
             )
         # Test case 2: Passing to `solve`
-        solver = base.LegacySolverWrapper()
+        solver = _LegacyWrappedSolverBase()
+        with self.assertRaises(ValueError):
+            solver._map_config(solver_options={'max_iter': 4}, options={'max_iter': 6})
+
+        # Test that assignment to maps to set_value:
+        solver = _LegacyWrappedSolverBase()
         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})
+        solver.config.solver_options.max_iter = 6
+        self.assertEqual(solver.options, {'max_iter': 6})
+        self.assertEqual(solver.config.solver_options, {'max_iter': 6})
 
     def test_map_results(self):
         # Unclear how to test this