From 03db9e804ae6b66da1593ea31dde188579fd6603 Mon Sep 17 00:00:00 2001 From: John Siirola Date: Tue, 13 Aug 2024 16:57:51 -0600 Subject: [PATCH 1/8] Add context to SuffixFinder; support finding a block in its own suffixes --- pyomo/core/base/suffix.py | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/pyomo/core/base/suffix.py b/pyomo/core/base/suffix.py index be2f732650d..b9aa3b58ced 100644 --- a/pyomo/core/base/suffix.py +++ b/pyomo/core/base/suffix.py @@ -19,6 +19,7 @@ from pyomo.common.modeling import NOTSET from pyomo.common.pyomo_typing import overload from pyomo.common.timing import ConstructionTimer +from pyomo.core.base.block import BlockData from pyomo.core.base.component import ActiveComponent, ModelComponentFactory from pyomo.core.base.disable_methods import disable_methods from pyomo.core.base.initializer import Initializer @@ -409,7 +410,7 @@ class AbstractSuffix(Suffix): class SuffixFinder(object): - def __init__(self, name, default=None): + def __init__(self, name, default=None, context=None): """This provides an efficient utility for finding suffix values on a (hierarchical) Pyomo model. @@ -428,7 +429,14 @@ def __init__(self, name, default=None): self.name = name self.default = default self.all_suffixes = [] - self._suffixes_by_block = {None: []} + self._context = context + self._suffixes_by_block = ComponentMap() + self._suffixes_by_block[self._context] = [] + if context is not None: + s = context.component(name) + if s is not None and s.ctype is Suffix and s.active: + self._suffixes_by_block[context].append(s) + self.all_suffixes.append(s) def find(self, component_data): """Find suffix value for a given component data object in model tree @@ -458,7 +466,17 @@ def find(self, component_data): """ # Walk parent tree and search for suffixes - suffixes = self._get_suffix_list(component_data.parent_block()) + if isinstance(component_data, BlockData): + _block = component_data + else: + _block = component_data.parent_block() + try: + suffixes = self._get_suffix_list(_block) + except AttributeError: + raise ValueError( + f"Component '{component_data.name}' not found in the SuffixFinder " + f"context (Block hierarchy rooted at {self._context.name})" + ) from None # Pass 1: look for the component_data, working root to leaf for s in suffixes: if component_data in s: From 89994ae7c09e127a726971131b8d148f180d925e Mon Sep 17 00:00:00 2001 From: John Siirola Date: Tue, 13 Aug 2024 16:59:22 -0600 Subject: [PATCH 2/8] Test SuffixFinder context --- pyomo/core/tests/unit/test_suffix.py | 72 +++++++++++++++++++++++----- 1 file changed, 61 insertions(+), 11 deletions(-) diff --git a/pyomo/core/tests/unit/test_suffix.py b/pyomo/core/tests/unit/test_suffix.py index d2e861cceb5..1278601bb3b 100644 --- a/pyomo/core/tests/unit/test_suffix.py +++ b/pyomo/core/tests/unit/test_suffix.py @@ -1795,47 +1795,97 @@ def test_suffix_finder(self): m.b1.b2 = Block() m.b1.b2.v3 = Var([0]) - _suffix_finder = SuffixFinder('suffix') - # Add Suffixes m.suffix = Suffix(direction=Suffix.EXPORT) # No suffix on b1 - make sure we can handle missing suffixes m.b1.b2.suffix = Suffix(direction=Suffix.EXPORT) + _suffix_finder = SuffixFinder('suffix') + _suffix_b1_finder = SuffixFinder('suffix', context=m.b1) + _suffix_b2_finder = SuffixFinder('suffix', context=m.b1.b2) + # Check for no suffix value - assert _suffix_finder.find(m.b1.b2.v3[0]) == None + self.assertEqual(_suffix_finder.find(m.b1.b2.v3[0]), None) + self.assertEqual(_suffix_b1_finder.find(m.b1.b2.v3[0]), None) + self.assertEqual(_suffix_b2_finder.find(m.b1.b2.v3[0]), None) # Check finding default values # Add a default at the top level m.suffix[None] = 1 - assert _suffix_finder.find(m.b1.b2.v3[0]) == 1 + self.assertEqual(_suffix_finder.find(m.b1.b2.v3[0]), 1) + self.assertEqual(_suffix_b1_finder.find(m.b1.b2.v3[0]), None) + self.assertEqual(_suffix_b2_finder.find(m.b1.b2.v3[0]), None) # Add a default suffix at a lower level m.b1.b2.suffix[None] = 2 - assert _suffix_finder.find(m.b1.b2.v3[0]) == 2 + self.assertEqual(_suffix_finder.find(m.b1.b2.v3[0]), 2) + self.assertEqual(_suffix_b1_finder.find(m.b1.b2.v3[0]), 2) + self.assertEqual(_suffix_b2_finder.find(m.b1.b2.v3[0]), 2) # Check for container at lowest level m.b1.b2.suffix[m.b1.b2.v3] = 3 - assert _suffix_finder.find(m.b1.b2.v3[0]) == 3 + self.assertEqual(_suffix_finder.find(m.b1.b2.v3[0]), 3) + self.assertEqual(_suffix_b1_finder.find(m.b1.b2.v3[0]), 3) + self.assertEqual(_suffix_b2_finder.find(m.b1.b2.v3[0]), 3) # Check for container at top level m.suffix[m.b1.b2.v3] = 4 - assert _suffix_finder.find(m.b1.b2.v3[0]) == 4 + self.assertEqual(_suffix_finder.find(m.b1.b2.v3[0]), 4) + self.assertEqual(_suffix_b1_finder.find(m.b1.b2.v3[0]), 3) + self.assertEqual(_suffix_b2_finder.find(m.b1.b2.v3[0]), 3) # Check for specific values at lowest level m.b1.b2.suffix[m.b1.b2.v3[0]] = 5 - assert _suffix_finder.find(m.b1.b2.v3[0]) == 5 + self.assertEqual(_suffix_finder.find(m.b1.b2.v3[0]), 5) + self.assertEqual(_suffix_b1_finder.find(m.b1.b2.v3[0]), 5) + self.assertEqual(_suffix_b2_finder.find(m.b1.b2.v3[0]), 5) # Check for specific values at top level m.suffix[m.b1.b2.v3[0]] = 6 - assert _suffix_finder.find(m.b1.b2.v3[0]) == 6 + self.assertEqual(_suffix_finder.find(m.b1.b2.v3[0]), 6) + self.assertEqual(_suffix_b1_finder.find(m.b1.b2.v3[0]), 5) + self.assertEqual(_suffix_b2_finder.find(m.b1.b2.v3[0]), 5) # Make sure we don't find default suffixes at lower levels - assert _suffix_finder.find(m.b1.v2) == 1 + self.assertEqual(_suffix_finder.find(m.b1.v2), 1) + self.assertEqual(_suffix_b1_finder.find(m.b1.v2), None) + with self.assertRaisesRegex( + ValueError, + r"Component 'b1.v2' not found in the SuffixFinder context " + r"\(Block hierarchy rooted at b1.b2\)", + ): + _suffix_b2_finder.find(m.b1.v2) # Make sure we don't find specific suffixes at lower levels m.b1.b2.suffix[m.v1] = 5 - assert _suffix_finder.find(m.v1) == 1 + self.assertEqual(_suffix_finder.find(m.v1), 1) + with self.assertRaisesRegex( + ValueError, + r"Component 'v1' not found in the SuffixFinder context " + r"\(Block hierarchy rooted at b1\)", + ): + _suffix_b1_finder.find(m.v1) + with self.assertRaisesRegex( + ValueError, + r"Component 'v1' not found in the SuffixFinder context " + r"\(Block hierarchy rooted at b1.b2\)", + ): + _suffix_b2_finder.find(m.v1) + + # Make sure we can look up Blocks and that they will match + # suffixes that they hold + self.assertEqual(_suffix_finder.find(m.b1.b2), 2) + self.assertEqual(_suffix_b1_finder.find(m.b1.b2), 2) + self.assertEqual(_suffix_b2_finder.find(m.b1.b2), 2) + + self.assertEqual(_suffix_finder.find(m.b1), 1) + self.assertEqual(_suffix_b1_finder.find(m.b1), None) + with self.assertRaisesRegex( + ValueError, + r"Component 'b1' not found in the SuffixFinder context " + r"\(Block hierarchy rooted at b1.b2\)", + ): + _suffix_b2_finder.find(m.b1) if __name__ == "__main__": From bc185466c5137d9dff4729a93150989f5c138ab9 Mon Sep 17 00:00:00 2001 From: John Siirola Date: Tue, 13 Aug 2024 17:00:04 -0600 Subject: [PATCH 3/8] NLv2: only locate / process Suffixes in the context of the model being written --- pyomo/repn/plugins/nl_writer.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pyomo/repn/plugins/nl_writer.py b/pyomo/repn/plugins/nl_writer.py index 8fc82d21d30..09d3f9b9ef0 100644 --- a/pyomo/repn/plugins/nl_writer.py +++ b/pyomo/repn/plugins/nl_writer.py @@ -510,8 +510,8 @@ def compile(self, column_order, row_order, obj_order, model_id): class CachingNumericSuffixFinder(SuffixFinder): scale = True - def __init__(self, name, default=None): - super().__init__(name, default) + def __init__(self, name, default=None, context=None): + super().__init__(name, default, context) self.suffix_cache = {} def __call__(self, obj): @@ -646,7 +646,7 @@ def write(self, model): # Data structures to support variable/constraint scaling # if self.config.scale_model and 'scaling_factor' in suffix_data: - scaling_factor = CachingNumericSuffixFinder('scaling_factor', 1) + scaling_factor = CachingNumericSuffixFinder('scaling_factor', 1, model) scaling_cache = scaling_factor.suffix_cache del suffix_data['scaling_factor'] else: From 5fbeb0941a6878e4b949baac47a275be6bac9804 Mon Sep 17 00:00:00 2001 From: John Siirola Date: Tue, 13 Aug 2024 17:02:14 -0600 Subject: [PATCH 4/8] SacalingTransform: only find/process Suffixes in the context of the transformation scope --- pyomo/core/plugins/transform/scaling.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyomo/core/plugins/transform/scaling.py b/pyomo/core/plugins/transform/scaling.py index 11d4ac8c493..87303514857 100644 --- a/pyomo/core/plugins/transform/scaling.py +++ b/pyomo/core/plugins/transform/scaling.py @@ -90,7 +90,7 @@ def _get_float_scaling_factor(self, component): def _apply_to(self, model, rename=True): # create a map of component to scaling factor component_scaling_factor_map = ComponentMap() - self._suffix_finder = SuffixFinder('scaling_factor', 1.0) + self._suffix_finder = SuffixFinder('scaling_factor', 1.0, model) # if the scaling_method is 'user', get the scaling parameters from the suffixes if self._scaling_method == 'user': From 84ca6ae088610ff8c68834819739221a38b58e9a Mon Sep 17 00:00:00 2001 From: John Siirola Date: Tue, 13 Aug 2024 17:12:55 -0600 Subject: [PATCH 5/8] remove unused _get_float_scaling_factor method --- pyomo/core/plugins/transform/scaling.py | 5 -- pyomo/core/tests/transform/test_scaling.py | 57 ++++++++++++++-------- 2 files changed, 37 insertions(+), 25 deletions(-) diff --git a/pyomo/core/plugins/transform/scaling.py b/pyomo/core/plugins/transform/scaling.py index 87303514857..0835e5fd060 100644 --- a/pyomo/core/plugins/transform/scaling.py +++ b/pyomo/core/plugins/transform/scaling.py @@ -82,11 +82,6 @@ def _create_using(self, original_model, **kwds): self._apply_to(scaled_model, **kwds) return scaled_model - def _get_float_scaling_factor(self, component): - if self._suffix_finder is None: - self._suffix_finder = SuffixFinder('scaling_factor', 1.0) - return self._suffix_finder.find(component) - def _apply_to(self, model, rename=True): # create a map of component to scaling factor component_scaling_factor_map = ComponentMap() diff --git a/pyomo/core/tests/transform/test_scaling.py b/pyomo/core/tests/transform/test_scaling.py index d0fbfab61bd..2d66502271e 100644 --- a/pyomo/core/tests/transform/test_scaling.py +++ b/pyomo/core/tests/transform/test_scaling.py @@ -13,8 +13,7 @@ import pyomo.common.unittest as unittest import pyomo.environ as pyo from pyomo.opt.base.solvers import UnknownSolver -from pyomo.core.plugins.transform.scaling import ScaleModel - +from pyomo.core.plugins.transform.scaling import ScaleModel, SuffixFinder class TestScaleModelTransformation(unittest.TestCase): def test_linear_scaling(self): @@ -600,6 +599,13 @@ def con_rule(m, i): self.assertAlmostEqual(pyo.value(model.zcon), -8, 4) def test_get_float_scaling_factor_top_level(self): + # Note: the transformation used to have a private method for + # finding suffix values (which this method tested). The + # transformation now leverages the SuffixFinder. To ensure that + # the SuffixFinder behaves in the same way as the original local + # method, we preserve these tests, but directly test the + # SuffixFinder + m = pyo.ConcreteModel() m.scaling_factor = pyo.Suffix(direction=pyo.Suffix.EXPORT) @@ -616,17 +622,23 @@ def test_get_float_scaling_factor_top_level(self): m.scaling_factor[m.v1] = 0.1 m.scaling_factor[m.b1.v2] = 0.2 + _finder = SuffixFinder('scaling_factor', 1.0, m) + # SF should be 0.1 from top level - sf = ScaleModel()._get_float_scaling_factor(m.v1) - assert sf == float(0.1) + self.assertEqual(_finder.find(m.v1), 0.1) # SF should be 0.1 from top level, lower level ignored - sf = ScaleModel()._get_float_scaling_factor(m.b1.v2) - assert sf == float(0.2) + self.assertEqual(_finder.find(m.b1.v2), 0.2) # No SF, should return 1 - sf = ScaleModel()._get_float_scaling_factor(m.b1.b2.v3) - assert sf == 1.0 + self.assertEqual(_finder.find(m.b1.b2.v3), 1.0) def test_get_float_scaling_factor_local_level(self): + # Note: the transformation used to have a private method for + # finding suffix values (which this method tested). The + # transformation now leverages the SuffixFinder. To ensure that + # the SuffixFinder behaves in the same way as the original local + # method, we preserve these tests, but directly test the + # SuffixFinder + m = pyo.ConcreteModel() m.scaling_factor = pyo.Suffix(direction=pyo.Suffix.EXPORT) @@ -647,15 +659,21 @@ def test_get_float_scaling_factor_local_level(self): # Add an intermediate scaling factor - this should take priority m.b1.scaling_factor[m.b1.b2.v3] = 0.4 + _finder = SuffixFinder('scaling_factor', 1.0, m) + # Should get SF from local levels - sf = ScaleModel()._get_float_scaling_factor(m.v1) - assert sf == float(0.1) - sf = ScaleModel()._get_float_scaling_factor(m.b1.v2) - assert sf == float(0.2) - sf = ScaleModel()._get_float_scaling_factor(m.b1.b2.v3) - assert sf == float(0.4) + self.assertEqual(_finder.find(m.v1), 0.1) + self.assertEqual(_finder.find(m.b1.v2), 0.2) + self.assertEqual(_finder.find(m.b1.b2.v3), 0.4) def test_get_float_scaling_factor_intermediate_level(self): + # Note: the transformation used to have a private method for + # finding suffix values (which this method tested). The + # transformation now leverages the SuffixFinder. To ensure that + # the SuffixFinder behaves in the same way as the original local + # method, we preserve these tests, but directly test the + # SuffixFinder + m = pyo.ConcreteModel() m.scaling_factor = pyo.Suffix(direction=pyo.Suffix.EXPORT) @@ -680,15 +698,14 @@ def test_get_float_scaling_factor_intermediate_level(self): m.b1.b2.b3.scaling_factor[m.b1.b2.b3.v3] = 0.4 + _finder = SuffixFinder('scaling_factor', 1.0, m) + # v1 should be unscaled as SF set below variable level - sf = ScaleModel()._get_float_scaling_factor(m.v1) - assert sf == 1.0 + self.assertEqual(_finder.find(m.v1), 1.0) # v2 should get SF from b1 level - sf = ScaleModel()._get_float_scaling_factor(m.b1.b2.b3.v2) - assert sf == float(0.2) + self.assertEqual(_finder.find(m.b1.b2.b3.v2), 0.2) # v2 should get SF from highest level, ignoring b3 level - sf = ScaleModel()._get_float_scaling_factor(m.b1.b2.b3.v3) - assert sf == float(0.3) + self.assertEqual(_finder.find(m.b1.b2.b3.v3), 0.3) if __name__ == "__main__": From f1c6ff128292732523ba6a77ce3acab03c349509 Mon Sep 17 00:00:00 2001 From: John Siirola Date: Tue, 13 Aug 2024 20:59:09 -0600 Subject: [PATCH 6/8] NFC: apply black --- pyomo/core/tests/transform/test_scaling.py | 1 + 1 file changed, 1 insertion(+) diff --git a/pyomo/core/tests/transform/test_scaling.py b/pyomo/core/tests/transform/test_scaling.py index 2d66502271e..7168f6bb707 100644 --- a/pyomo/core/tests/transform/test_scaling.py +++ b/pyomo/core/tests/transform/test_scaling.py @@ -15,6 +15,7 @@ from pyomo.opt.base.solvers import UnknownSolver from pyomo.core.plugins.transform.scaling import ScaleModel, SuffixFinder + class TestScaleModelTransformation(unittest.TestCase): def test_linear_scaling(self): model = pyo.ConcreteModel() From c0eef1798660d5af74eadf1db98de83a0978fe4f Mon Sep 17 00:00:00 2001 From: John Siirola Date: Wed, 14 Aug 2024 07:54:51 -0600 Subject: [PATCH 7/8] SuffixFinder: return default for out-of-scope components (not an exception) --- pyomo/core/base/suffix.py | 8 ++++---- pyomo/core/tests/unit/test_suffix.py | 28 ++++------------------------ 2 files changed, 8 insertions(+), 28 deletions(-) diff --git a/pyomo/core/base/suffix.py b/pyomo/core/base/suffix.py index b9aa3b58ced..c07f7a0deb2 100644 --- a/pyomo/core/base/suffix.py +++ b/pyomo/core/base/suffix.py @@ -473,10 +473,10 @@ def find(self, component_data): try: suffixes = self._get_suffix_list(_block) except AttributeError: - raise ValueError( - f"Component '{component_data.name}' not found in the SuffixFinder " - f"context (Block hierarchy rooted at {self._context.name})" - ) from None + # Component was outside the context (eventually parent + # becomes None and parent.parent_block() raises an + # AttributeError): we will return the default value + return self.default # Pass 1: look for the component_data, working root to leaf for s in suffixes: if component_data in s: diff --git a/pyomo/core/tests/unit/test_suffix.py b/pyomo/core/tests/unit/test_suffix.py index 1278601bb3b..f56f84fc129 100644 --- a/pyomo/core/tests/unit/test_suffix.py +++ b/pyomo/core/tests/unit/test_suffix.py @@ -1849,28 +1849,13 @@ def test_suffix_finder(self): # Make sure we don't find default suffixes at lower levels self.assertEqual(_suffix_finder.find(m.b1.v2), 1) self.assertEqual(_suffix_b1_finder.find(m.b1.v2), None) - with self.assertRaisesRegex( - ValueError, - r"Component 'b1.v2' not found in the SuffixFinder context " - r"\(Block hierarchy rooted at b1.b2\)", - ): - _suffix_b2_finder.find(m.b1.v2) + self.assertEqual(_suffix_b2_finder.find(m.b1.v2), None) # Make sure we don't find specific suffixes at lower levels m.b1.b2.suffix[m.v1] = 5 self.assertEqual(_suffix_finder.find(m.v1), 1) - with self.assertRaisesRegex( - ValueError, - r"Component 'v1' not found in the SuffixFinder context " - r"\(Block hierarchy rooted at b1\)", - ): - _suffix_b1_finder.find(m.v1) - with self.assertRaisesRegex( - ValueError, - r"Component 'v1' not found in the SuffixFinder context " - r"\(Block hierarchy rooted at b1.b2\)", - ): - _suffix_b2_finder.find(m.v1) + self.assertEqual(_suffix_b1_finder.find(m.v1), None) + self.assertEqual(_suffix_b2_finder.find(m.v1), None) # Make sure we can look up Blocks and that they will match # suffixes that they hold @@ -1880,12 +1865,7 @@ def test_suffix_finder(self): self.assertEqual(_suffix_finder.find(m.b1), 1) self.assertEqual(_suffix_b1_finder.find(m.b1), None) - with self.assertRaisesRegex( - ValueError, - r"Component 'b1' not found in the SuffixFinder context " - r"\(Block hierarchy rooted at b1.b2\)", - ): - _suffix_b2_finder.find(m.b1) + self.assertEqual(_suffix_b2_finder.find(m.b1), None) if __name__ == "__main__": From 31a13f625c7def976471cbe6f1e2e57e9471f1b9 Mon Sep 17 00:00:00 2001 From: John Siirola Date: Wed, 14 Aug 2024 07:55:05 -0600 Subject: [PATCH 8/8] NFC: update docstring --- pyomo/core/base/suffix.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/pyomo/core/base/suffix.py b/pyomo/core/base/suffix.py index c07f7a0deb2..19dbe48f650 100644 --- a/pyomo/core/base/suffix.py +++ b/pyomo/core/base/suffix.py @@ -425,6 +425,14 @@ def __init__(self, name, default=None, context=None): Default value to return from `.find()` if no matching Suffix is found. + context: BlockData + + The root of the Block hierarchy to use when searching for + Suffix components. Suffixes outside this hierarchy will not + be interrogated and components that are queried (with + :py:meth:`find(component_data)` will return the default + value. + """ self.name = name self.default = default