From 017258dd593817f96ee9da4207e379807e5c8e52 Mon Sep 17 00:00:00 2001 From: John Siirola Date: Tue, 6 Aug 2024 12:33:02 -0600 Subject: [PATCH 1/5] Restore 'options' as an alias of the new config.solver_options --- pyomo/contrib/solver/base.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/pyomo/contrib/solver/base.py b/pyomo/contrib/solver/base.py index 98bf3836004..ac07bda4e67 100644 --- a/pyomo/contrib/solver/base.py +++ b/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 From 61bb290ee5a2b1c9600d9abc7b381202b52dc238 Mon Sep 17 00:00:00 2001 From: John Siirola Date: Tue, 6 Aug 2024 13:40:20 -0600 Subject: [PATCH 2/5] Update tests to not directly instantiate the LegacySolverWrapper mixin --- pyomo/contrib/solver/tests/unit/test_base.py | 35 ++++++++++++-------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/pyomo/contrib/solver/tests/unit/test_base.py b/pyomo/contrib/solver/tests/unit/test_base.py index b52f96ba903..b7937d16af3 100644 --- a/pyomo/contrib/solver/tests/unit/test_base.py +++ b/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,20 +278,21 @@ 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}) # 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'}) # Test case 3: pass some options to the mapping (aka, 'solve' command) - solver = base.LegacySolverWrapper() + solver = _LegacyWrappedSolverBase() config = ConfigDict(implicit=True) config.declare( 'solver_options', @@ -296,7 +303,7 @@ def test_solver_options_behavior(self): 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}) + solver = _LegacyWrappedSolverBase(options={'max_iter': 6}) config = ConfigDict(implicit=True) config.declare( 'solver_options', @@ -309,11 +316,11 @@ def test_solver_options_behavior(self): # 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}) # Test case 2: pass some solver_options to the mapping (aka, 'solve' command) - solver = base.LegacySolverWrapper() + solver = _LegacyWrappedSolverBase() config = ConfigDict(implicit=True) config.declare( 'solver_options', @@ -324,7 +331,7 @@ def test_solver_options_behavior(self): 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}) + solver = _LegacyWrappedSolverBase(solver_options={'max_iter': 6}) config = ConfigDict(implicit=True) config.declare( 'solver_options', @@ -337,7 +344,7 @@ def test_solver_options_behavior(self): # users can mix... sort of # Test case 1: Initialize with options, solve with solver_options - solver = base.LegacySolverWrapper(options={'max_iter': 6}) + solver = _LegacyWrappedSolverBase(options={'max_iter': 6}) config = ConfigDict(implicit=True) config.declare( 'solver_options', @@ -351,11 +358,11 @@ def test_solver_options_behavior(self): # 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() config = ConfigDict(implicit=True) config.declare( 'solver_options', From 896eae16203d9b3ab8865ce68ae5f13db6057b23 Mon Sep 17 00:00:00 2001 From: John Siirola Date: Tue, 6 Aug 2024 14:07:31 -0600 Subject: [PATCH 3/5] LegacySolverWrapper: 'options' and 'config' should be singleton attributes --- pyomo/contrib/solver/base.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/pyomo/contrib/solver/base.py b/pyomo/contrib/solver/base.py index ac07bda4e67..4fe8bee4e53 100644 --- a/pyomo/contrib/solver/base.py +++ b/pyomo/contrib/solver/base.py @@ -377,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, @@ -395,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) @@ -410,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. From 6975cb6b068beda72f748396dd51644f03b895ce Mon Sep 17 00:00:00 2001 From: John Siirola Date: Tue, 6 Aug 2024 14:08:02 -0600 Subject: [PATCH 4/5] Update 'options' tests to reflect new bas class --- pyomo/contrib/solver/tests/unit/test_base.py | 50 +++++++------------- 1 file changed, 16 insertions(+), 34 deletions(-) diff --git a/pyomo/contrib/solver/tests/unit/test_base.py b/pyomo/contrib/solver/tests/unit/test_base.py index b7937d16af3..ba62f97542a 100644 --- a/pyomo/contrib/solver/tests/unit/test_base.py +++ b/pyomo/contrib/solver/tests/unit/test_base.py @@ -285,73 +285,49 @@ def test_solver_options_behavior(self): # Test case 1: Set at instantiation 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 = _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 = _LegacyWrappedSolverBase() - 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.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 = _LegacyWrappedSolverBase(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.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 = _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 = _LegacyWrappedSolverBase() - 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.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 = _LegacyWrappedSolverBase(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.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 = _LegacyWrappedSolverBase(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.options, {'max_iter': 4}) self.assertEqual(solver.config.solver_options, {'max_iter': 4}) # users CANNOT initialize both values at the same time, because how @@ -363,14 +339,20 @@ def test_solver_options_behavior(self): ) # Test case 2: Passing to `solve` 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_vlaue: + 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 From a61df0c68d98a60090bc884521769ced4caa5b46 Mon Sep 17 00:00:00 2001 From: John Siirola Date: Tue, 6 Aug 2024 14:12:52 -0600 Subject: [PATCH 5/5] Fix typo --- pyomo/contrib/solver/tests/unit/test_base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyomo/contrib/solver/tests/unit/test_base.py b/pyomo/contrib/solver/tests/unit/test_base.py index ba62f97542a..e9ea717593f 100644 --- a/pyomo/contrib/solver/tests/unit/test_base.py +++ b/pyomo/contrib/solver/tests/unit/test_base.py @@ -342,7 +342,7 @@ def test_solver_options_behavior(self): with self.assertRaises(ValueError): solver._map_config(solver_options={'max_iter': 4}, options={'max_iter': 6}) - # Test that assignment to maps to set_vlaue: + # Test that assignment to maps to set_value: solver = _LegacyWrappedSolverBase() config = ConfigDict(implicit=True) config.declare(