From 3fcb1802e78245e60ab9122cc481968a6e4f7124 Mon Sep 17 00:00:00 2001 From: John Siirola Date: Wed, 23 Aug 2023 22:54:43 -0600 Subject: [PATCH 01/23] Remove unnecessary f-string --- pyomo/common/config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyomo/common/config.py b/pyomo/common/config.py index 1b44d555b91..61e4f682a2a 100644 --- a/pyomo/common/config.py +++ b/pyomo/common/config.py @@ -2019,7 +2019,7 @@ def generate_documentation( ) if item_body is not None: deprecation_warning( - f"Overriding 'item_body' by passing strings to " + "Overriding 'item_body' by passing strings to " "generate_documentation is deprecated. Create an instance of a " "StringConfigFormatter and pass it as the 'format' argument.", version='6.6.0', From 19735453e66a0c6ce367e52ec9ba81ab8ba33760 Mon Sep 17 00:00:00 2001 From: John Siirola Date: Wed, 23 Aug 2023 22:54:52 -0600 Subject: [PATCH 02/23] Fix comment typo --- pyomo/contrib/pynumero/interfaces/pyomo_grey_box_nlp.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyomo/contrib/pynumero/interfaces/pyomo_grey_box_nlp.py b/pyomo/contrib/pynumero/interfaces/pyomo_grey_box_nlp.py index 9a5dc50ef7b..ea51e38983d 100644 --- a/pyomo/contrib/pynumero/interfaces/pyomo_grey_box_nlp.py +++ b/pyomo/contrib/pynumero/interfaces/pyomo_grey_box_nlp.py @@ -33,7 +33,7 @@ from pyomo.contrib.pynumero.interfaces.nlp_projections import ProjectedNLP -# Todo: make some of the numpy arise not writable from __init__ +# Todo: make some of the numpy arrays not writable from __init__ class PyomoNLPWithGreyBoxBlocks(NLP): def __init__(self, pyomo_model): super(PyomoNLPWithGreyBoxBlocks, self).__init__() From 8d015f100335ed86bbe11c9a5ffc3abb5b90ae84 Mon Sep 17 00:00:00 2001 From: John Siirola Date: Wed, 30 Aug 2023 09:42:52 -0600 Subject: [PATCH 03/23] NFC: reformatting comments/exceptions --- pyomo/core/plugins/transform/scaling.py | 42 +++++++++++++++---------- 1 file changed, 25 insertions(+), 17 deletions(-) diff --git a/pyomo/core/plugins/transform/scaling.py b/pyomo/core/plugins/transform/scaling.py index 0596590e979..49da559de8f 100644 --- a/pyomo/core/plugins/transform/scaling.py +++ b/pyomo/core/plugins/transform/scaling.py @@ -225,15 +225,16 @@ def _apply_to(self, model, rename=True): ] ) - # scale the variable bounds and values and build the variable substitution map - # for scaling vars in constraints + # scale the variable bounds and values and build the variable + # substitution map for scaling vars in constraints variable_substitution_map = ComponentMap() already_scaled = set() for variable in [ var for var in model.component_objects(ctype=Var, descend_into=True) ]: if variable.is_reference(): - # Skip any references - these should get picked up when handling the actual variable + # Skip any references - these should get picked up when + # handling the actual variable continue # set the bounds/value for the scaled variable @@ -277,7 +278,8 @@ def _apply_to(self, model, rename=True): ctype=(Constraint, Objective), descend_into=True ): if component.is_reference(): - # Skip any references - these should get picked up when handling the actual component + # Skip any references - these should get picked up when + # handling the actual component continue for k in component: @@ -325,7 +327,8 @@ def _apply_to(self, model, rename=True): ) else: raise NotImplementedError( - 'Unknown object type found when applying scaling factors in ScaleModel transformation - Internal Error' + 'Unknown object type found when applying scaling factors ' + 'in ScaleModel transformation - Internal Error' ) model.component_scaling_factor_map = component_scaling_factor_map @@ -336,12 +339,14 @@ def _apply_to(self, model, rename=True): return model def propagate_solution(self, scaled_model, original_model): - """ - This method takes the solution in scaled_model and maps it back to the original model. + """This method takes the solution in scaled_model and maps it back to + the original model. - It will also transform duals and reduced costs if the suffixes 'dual' and/or 'rc' are present. - The :code:`scaled_model` argument must be a model that was already scaled using this transformation - as it expects data from the transformation to perform the back mapping. + It will also transform duals and reduced costs if the suffixes + 'dual' and/or 'rc' are present. The :code:`scaled_model` + argument must be a model that was already scaled using this + transformation as it expects data from the transformation to + perform the back mapping. Parameters ---------- @@ -353,15 +358,17 @@ def propagate_solution(self, scaled_model, original_model): """ if not hasattr(scaled_model, 'component_scaling_factor_map'): raise AttributeError( - 'ScaleModel:propagate_solution called with scaled_model that does not ' - 'have a component_scaling_factor_map. It is possible this method was called ' - 'using a model that was not scaled with the ScaleModel transformation' + 'ScaleModel:propagate_solution called with scaled_model that does ' + 'not have a component_scaling_factor_map. It is possible this ' + 'method was called using a model that was not scaled with the ' + 'ScaleModel transformation' ) if not hasattr(scaled_model, 'scaled_component_to_original_name_map'): raise AttributeError( - 'ScaleModel:propagate_solution called with scaled_model that does not ' - 'have a scaled_component_to_original_name_map. It is possible this method was called ' - 'using a model that was not scaled with the ScaleModel transformation' + 'ScaleModel:propagate_solution called with scaled_model that does ' + 'not have a scaled_component_to_original_name_map. It is possible ' + 'this method was called using a model that was not scaled with ' + 'the ScaleModel transformation' ) component_scaling_factor_map = scaled_model.component_scaling_factor_map @@ -385,7 +392,8 @@ def propagate_solution(self, scaled_model, original_model): ) if len(scaled_objectives) != 1: raise NotImplementedError( - 'ScaleModel.propagate_solution requires a single active objective function, but %d objectives found.' + 'ScaleModel.propagate_solution requires a single active ' + 'objective function, but %d objectives found.' % (len(scaled_objectives)) ) else: From 23ee769486dc24c0644739de3d2d7a58f0312db9 Mon Sep 17 00:00:00 2001 From: John Siirola Date: Wed, 30 Aug 2023 09:49:27 -0600 Subject: [PATCH 04/23] Promote _suffix_finder to a re-usable SuffixFinder class --- pyomo/core/base/suffix.py | 79 +++++++++++++++ pyomo/core/plugins/transform/scaling.py | 108 ++------------------- pyomo/core/tests/transform/test_scaling.py | 92 ++---------------- pyomo/core/tests/unit/test_suffix.py | 56 +++++++++++ 4 files changed, 152 insertions(+), 183 deletions(-) diff --git a/pyomo/core/base/suffix.py b/pyomo/core/base/suffix.py index b0c3c8fdb22..8806bc7abcd 100644 --- a/pyomo/core/base/suffix.py +++ b/pyomo/core/base/suffix.py @@ -521,3 +521,82 @@ def __eq__(self, other): def __ne__(self, other): """Not implemented.""" raise NotImplementedError("Suffix components are not comparable") + + +class SuffixFinder(object): + def __init__(self, name, default=None): + """This provides an efficient utility for finding suffix values on a + (hierarchical) Pyomo model. + + Parameters + ---------- + name: str + + Name of Suffix to search for. + + default: + + Default value to return from `.find()` if no matching Suffix + is found. + + """ + self.name = name + self.default = default + self._suffixes_by_block = ComponentMap({None: []}) + self.all_suffixes = [] + + def find(self, component_data): + """Find suffix value for a given component data object in model tree + + Suffixes are searched by traversing the model hierarchy in three passes: + + 1. Search for a Suffix matching the specific component_data, + starting at the `root` and descending down the tree to + the component_data. Return the first match found. + 2. Search for a Suffix matching the component_data's container, + starting at the `root` and descending down the tree to + the component_data. Return the first match found. + 3. Search for a Suffix with key `None`, starting from the + component_data and working up the tree to the `root`. + Return the first match found. + 4. Return the default value + + Parameters + ---------- + component_data: ComponentDataBase + + Component or component data object to find suffix value for. + + Returns + ------- + The value for Suffix associated with component data if found, else None. + + """ + # Walk parent tree and search for suffixes + suffixes = self._get_suffix_list(component_data.parent_block()) + # Pass 1: look for the component_data, working root to leaf + for s in suffixes: + if component_data in s: + return s[component_data] + # Pass 2: look for the component container, working root to leaf + parent_comp = component_data.parent_component() + if parent_comp is not component_data: + for s in suffixes: + if parent_comp in s: + return s[parent_comp] + # Pass 3: look for None, working leaf to root + for s in reversed(suffixes): + if None in s: + return s[None] + return self.default + + def _get_suffix_list(self, parent): + if parent in self._suffixes_by_block: + return self._suffixes_by_block[parent] + suffixes = list(self._get_suffix_list(parent.parent_block())) + self._suffixes_by_block[parent] = suffixes + s = parent.component(self.name) + if s is not None and s.ctype is Suffix and s.active: + suffixes.append(s) + self.all_suffixes.append(s) + return suffixes diff --git a/pyomo/core/plugins/transform/scaling.py b/pyomo/core/plugins/transform/scaling.py index 49da559de8f..133c84032d0 100644 --- a/pyomo/core/plugins/transform/scaling.py +++ b/pyomo/core/plugins/transform/scaling.py @@ -22,6 +22,7 @@ ) from pyomo.core.plugins.transform.hierarchy import Transformation from pyomo.core.base import TransformationFactory +from pyomo.core.base.suffix import SuffixFinder from pyomo.core.expr import replace_expressions from pyomo.util.components import rename_components @@ -82,6 +83,7 @@ class ScaleModel(Transformation): def __init__(self, **kwds): kwds['name'] = "scale_model" self._scaling_method = kwds.pop('scaling_method', 'user') + self._suffix_finder = None super(ScaleModel, self).__init__(**kwds) def _create_using(self, original_model, **kwds): @@ -89,107 +91,15 @@ def _create_using(self, original_model, **kwds): self._apply_to(scaled_model, **kwds) return scaled_model - def _suffix_finder(self, component_data, suffix_name, root=None): - """Find suffix value for a given component data object in model tree - - Suffixes are searched by traversing the model hierarchy in three passes: - - 1. Search for a Suffix matching the specific component_data, - starting at the `root` and descending down the tree to - the component_data. Return the first match found. - 2. Search for a Suffix matching the component_data's container, - starting at the `root` and descending down the tree to - the component_data. Return the first match found. - 3. Search for a Suffix with key `None`, starting from the - component_data and working up the tree to the `root`. - Return the first match found. - 4. Return None - - Parameters - ---------- - component_data: ComponentDataBase - - Component or component data object to find suffix value for. - - suffix_name: str - - Name of Suffix to search for. - - root: BlockData - - When searching up the block hierarchy, stop at this - BlockData instead of traversing all the way to the - `component_data.model()` Block. If the `component_data` is - not in the subtree defined by `root`, then the search will - proceed up to `component_data.model()`. - - Returns - ------- - The value for Suffix associated with component data if found, else None. - - """ - # Prototype for Suffix finder - - # We want to *include* the root (if it is not None), so if - # it is not None, we want to stop as soon as we get to its - # parent. - if root is not None: - if root.ctype is not Block and not issubclass(root.ctype, Block): - raise ValueError( - "_find_suffix: root must be a BlockData " - f"(found {root.ctype.__name__}: {root})" - ) - if root.is_indexed(): - raise ValueError( - "_find_suffix: root must be a BlockData " - f"(found {type(root).__name__}: {root})" - ) - root = root.parent_block() - # Walk parent tree and search for suffixes - parent = component_data.parent_block() - suffixes = [] - while parent is not root: - s = parent.component(suffix_name) - if s is not None and s.ctype is Suffix: - suffixes.append(s) - parent = parent.parent_block() - # Pass 1: look for the component_data, working root to leaf - for s in reversed(suffixes): - if component_data in s: - return s[component_data] - # Pass 2: look for the component container, working root to leaf - parent_comp = component_data.parent_component() - if parent_comp is not component_data: - for s in reversed(suffixes): - if parent_comp in s: - return s[parent_comp] - # Pass 3: look for None, working leaf to root - for s in suffixes: - if None in s: - return s[None] - return None - - def _get_float_scaling_factor(self, instance, component_data): - scaling_factor = self._suffix_finder(component_data, "scaling_factor") - - # If still no scaling factor, return 1.0 - if scaling_factor is None: - return 1.0 - - # Make sure scaling factor is a float - try: - scaling_factor = float(scaling_factor) - except ValueError: - raise ValueError( - "Suffix 'scaling_factor' has a value %s for component %s that cannot be converted to a float. " - "Floating point values are required for this suffix in the ScaleModel transformation." - % (scaling_factor, component_data) - ) - return scaling_factor + 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() + self._suffix_finder = SuffixFinder('scaling_factor', 1.0) # if the scaling_method is 'user', get the scaling parameters from the suffixes if self._scaling_method == 'user': @@ -197,9 +107,7 @@ def _apply_to(self, model, rename=True): for c in model.component_data_objects( ctype=(Var, Constraint, Objective), descend_into=True ): - component_scaling_factor_map[c] = self._get_float_scaling_factor( - model, c - ) + component_scaling_factor_map[c] = self._suffix_finder.find(c) else: raise ValueError( "ScaleModel transformation: unknown scaling_method found" diff --git a/pyomo/core/tests/transform/test_scaling.py b/pyomo/core/tests/transform/test_scaling.py index b7f34a4f4aa..1cb4e886956 100644 --- a/pyomo/core/tests/transform/test_scaling.py +++ b/pyomo/core/tests/transform/test_scaling.py @@ -617,13 +617,13 @@ def test_get_float_scaling_factor_top_level(self): m.scaling_factor[m.b1.v2] = 0.2 # SF should be 0.1 from top level - sf = ScaleModel()._get_float_scaling_factor(m, m.v1) + sf = ScaleModel()._get_float_scaling_factor(m.v1) assert sf == float(0.1) # SF should be 0.1 from top level, lower level ignored - sf = ScaleModel()._get_float_scaling_factor(m, m.b1.v2) + sf = ScaleModel()._get_float_scaling_factor(m.b1.v2) assert sf == float(0.2) # No SF, should return 1 - sf = ScaleModel()._get_float_scaling_factor(m, m.b1.b2.v3) + sf = ScaleModel()._get_float_scaling_factor(m.b1.b2.v3) assert sf == 1.0 def test_get_float_scaling_factor_local_level(self): @@ -648,11 +648,11 @@ def test_get_float_scaling_factor_local_level(self): m.b1.scaling_factor[m.b1.b2.v3] = 0.4 # Should get SF from local levels - sf = ScaleModel()._get_float_scaling_factor(m, m.v1) + sf = ScaleModel()._get_float_scaling_factor(m.v1) assert sf == float(0.1) - sf = ScaleModel()._get_float_scaling_factor(m, m.b1.v2) + sf = ScaleModel()._get_float_scaling_factor(m.b1.v2) assert sf == float(0.2) - sf = ScaleModel()._get_float_scaling_factor(m, m.b1.b2.v3) + sf = ScaleModel()._get_float_scaling_factor(m.b1.b2.v3) assert sf == float(0.4) def test_get_float_scaling_factor_intermediate_level(self): @@ -681,89 +681,15 @@ def test_get_float_scaling_factor_intermediate_level(self): m.b1.b2.b3.scaling_factor[m.b1.b2.b3.v3] = 0.4 # v1 should be unscaled as SF set below variable level - sf = ScaleModel()._get_float_scaling_factor(m, m.v1) + sf = ScaleModel()._get_float_scaling_factor(m.v1) assert sf == 1.0 # v2 should get SF from b1 level - sf = ScaleModel()._get_float_scaling_factor(m, m.b1.b2.b3.v2) + sf = ScaleModel()._get_float_scaling_factor(m.b1.b2.b3.v2) assert sf == float(0.2) # v2 should get SF from highest level, ignoring b3 level - sf = ScaleModel()._get_float_scaling_factor(m, m.b1.b2.b3.v3) + sf = ScaleModel()._get_float_scaling_factor(m.b1.b2.b3.v3) assert sf == float(0.3) - def test_suffix_finder(self): - # Build a dummy model - m = pyo.ConcreteModel() - m.v1 = pyo.Var() - - m.b1 = pyo.Block() - m.b1.v2 = pyo.Var() - - m.b1.b2 = pyo.Block() - m.b1.b2.v3 = pyo.Var([0]) - - xfrm = ScaleModel() - _suffix_finder = xfrm._suffix_finder - - # Add Suffixes - m.suffix = pyo.Suffix(direction=pyo.Suffix.EXPORT) - # No suffix on b1 - make sure we can handle missing suffixes - m.b1.b2.suffix = pyo.Suffix(direction=pyo.Suffix.EXPORT) - - # Check for no suffix value - assert _suffix_finder(m.b1.b2.v3[0], "suffix") == None - assert _suffix_finder(m.b1.b2.v3[0], "suffix", root=m.b1) == None - - # Check finding default values - # Add a default at the top level - m.suffix[None] = 1 - assert _suffix_finder(m.b1.b2.v3[0], "suffix") == 1 - assert _suffix_finder(m.b1.b2.v3[0], "suffix", root=m.b1) == None - - # Add a default suffix at a lower level - m.b1.b2.suffix[None] = 2 - assert _suffix_finder(m.b1.b2.v3[0], "suffix") == 2 - assert _suffix_finder(m.b1.b2.v3[0], "suffix", root=m.b1) == 2 - - # Check for container at lowest level - m.b1.b2.suffix[m.b1.b2.v3] = 3 - assert _suffix_finder(m.b1.b2.v3[0], "suffix") == 3 - assert _suffix_finder(m.b1.b2.v3[0], "suffix", root=m.b1) == 3 - - # Check for container at top level - m.suffix[m.b1.b2.v3] = 4 - assert _suffix_finder(m.b1.b2.v3[0], "suffix") == 4 - assert _suffix_finder(m.b1.b2.v3[0], "suffix", root=m.b1) == 3 - - # Check for specific values at lowest level - m.b1.b2.suffix[m.b1.b2.v3[0]] = 5 - assert _suffix_finder(m.b1.b2.v3[0], "suffix") == 5 - assert _suffix_finder(m.b1.b2.v3[0], "suffix", root=m.b1) == 5 - - # Check for specific values at top level - m.suffix[m.b1.b2.v3[0]] = 6 - assert _suffix_finder(m.b1.b2.v3[0], "suffix") == 6 - assert _suffix_finder(m.b1.b2.v3[0], "suffix", root=m.b1) == 5 - - # Make sure we don't find default suffixes at lower levels - assert _suffix_finder(m.b1.v2, "suffix") == 1 - assert _suffix_finder(m.b1.v2, "suffix", root=m.b1) == None - - # Make sure we don't find specific suffixes at lower levels - m.b1.b2.suffix[m.v1] = 5 - assert _suffix_finder(m.v1, "suffix") == 1 - - with self.assertRaisesRegex( - ValueError, r"_find_suffix: root must be a BlockData \(found Var: v1\)" - ): - _suffix_finder(m.b1.v2, "suffix", root=m.v1) - - m.bn = pyo.Block([1, 2]) - with self.assertRaisesRegex( - ValueError, - r"_find_suffix: root must be a BlockData " r"\(found IndexedBlock: bn\)", - ): - _suffix_finder(m.b1.v2, "suffix", root=m.bn) - if __name__ == "__main__": unittest.main() diff --git a/pyomo/core/tests/unit/test_suffix.py b/pyomo/core/tests/unit/test_suffix.py index 55e3e9dbc3f..131e2054284 100644 --- a/pyomo/core/tests/unit/test_suffix.py +++ b/pyomo/core/tests/unit/test_suffix.py @@ -29,6 +29,7 @@ local_suffix_generator, active_suffix_generator, suffix_generator, + SuffixFinder, ) from pyomo.environ import ( ConcreteModel, @@ -1567,5 +1568,60 @@ def test_pickle_model(self): self.assertEqual(inst.junk.get(inst), 1.0) +class TestSuffixFinder(unittest.TestCase): + def test_suffix_finder(self): + # Build a dummy model + m = ConcreteModel() + m.v1 = Var() + + m.b1 = Block() + m.b1.v2 = Var() + + 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) + + # Check for no suffix value + assert _suffix_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 + + # Add a default suffix at a lower level + m.b1.b2.suffix[None] = 2 + assert _suffix_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 + + # Check for container at top level + m.suffix[m.b1.b2.v3] = 4 + assert _suffix_finder.find(m.b1.b2.v3[0]) == 4 + + # 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 + + # 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 + + # Make sure we don't find default suffixes at lower levels + assert _suffix_finder.find(m.b1.v2) == 1 + + # 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 + + if __name__ == "__main__": unittest.main() From 3326a508b5b6f556f58aedd2ff2c34fb7ae7c2b4 Mon Sep 17 00:00:00 2001 From: John Siirola Date: Wed, 30 Aug 2023 09:49:50 -0600 Subject: [PATCH 05/23] Minor implementation simplification --- 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 133c84032d0..02e8f852870 100644 --- a/pyomo/core/plugins/transform/scaling.py +++ b/pyomo/core/plugins/transform/scaling.py @@ -178,7 +178,7 @@ def _apply_to(self, model, rename=True): # to variable_substitution_dict (key: id() of component) # ToDo: We should change replace_expressions to accept a ComponentMap as well variable_substitution_dict = { - id(k): variable_substitution_map[k] for k in variable_substitution_map + id(k): v for k, v in variable_substitution_map.items() } already_scaled = set() From a64339f4d513fd1ce18fce395bbef0f9498186e5 Mon Sep 17 00:00:00 2001 From: John Siirola Date: Wed, 30 Aug 2023 10:03:34 -0600 Subject: [PATCH 06/23] Deactivate scaling_factor suffix after use --- pyomo/core/plugins/transform/scaling.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pyomo/core/plugins/transform/scaling.py b/pyomo/core/plugins/transform/scaling.py index 02e8f852870..0883455f9de 100644 --- a/pyomo/core/plugins/transform/scaling.py +++ b/pyomo/core/plugins/transform/scaling.py @@ -244,6 +244,12 @@ def _apply_to(self, model, rename=True): scaled_component_to_original_name_map ) + # Now that we have scaled the model, deactivate the relevant + # scaling suffixes so that we don't accidentally (later) + # double-scale. + for s in self._suffix_finder.all_suffixes: + s.deactivate() + return model def propagate_solution(self, scaled_model, original_model): From ffaed835ad5dc8917b1a58b5cb23094b08f65704 Mon Sep 17 00:00:00 2001 From: John Siirola Date: Wed, 30 Aug 2023 12:07:00 -0600 Subject: [PATCH 07/23] Remove UK spelling of argument --- examples/doc/samples/case_studies/network_flow/README.txt | 2 +- examples/doc/samples/case_studies/network_flow/networkFlow1.tex | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/examples/doc/samples/case_studies/network_flow/README.txt b/examples/doc/samples/case_studies/network_flow/README.txt index 67043f311f3..137138a6fdc 100644 --- a/examples/doc/samples/case_studies/network_flow/README.txt +++ b/examples/doc/samples/case_studies/network_flow/README.txt @@ -109,7 +109,7 @@ def supplyDemandRule(nn, model): model.supplyDemand = Constraint(model.places, rule=supplyDemandRule) }}} -What we did was just construct a few additional variables; this wasn't required but makes reading and understanding the code much, much simpler. The variable "amountIn" looks at each route to find ones that end in this location, then adds the amount on each of those routes together to determine how much flows to each location. The "amountOut" variable functions similarly for the flow out. Then we just create an "input" and "output" and ensure they're equal. Like in some of the previous constraints, we feed the set of places into the constraint as an arguement so it will index over that set, and thus this rule functions for all the places in our network. +What we did was just construct a few additional variables; this wasn't required but makes reading and understanding the code much, much simpler. The variable "amountIn" looks at each route to find ones that end in this location, then adds the amount on each of those routes together to determine how much flows to each location. The "amountOut" variable functions similarly for the flow out. Then we just create an "input" and "output" and ensure they're equal. Like in some of the previous constraints, we feed the set of places into the constraint as an argument so it will index over that set, and thus this rule functions for all the places in our network. We have now finished creating the model. Save this as a .py file before continuing. diff --git a/examples/doc/samples/case_studies/network_flow/networkFlow1.tex b/examples/doc/samples/case_studies/network_flow/networkFlow1.tex index 274320f917a..24e2c729374 100644 --- a/examples/doc/samples/case_studies/network_flow/networkFlow1.tex +++ b/examples/doc/samples/case_studies/network_flow/networkFlow1.tex @@ -172,7 +172,7 @@ \subsection*{Build the model} model.supplyDemand = Constraint(model.places, rule=supplyDemandRule) \end{verbatim} -What we did was just construct a few additional variables; this wasn't required but makes reading and understanding the code much, much simpler. The variable ``amountIn'' looks at each route to find ones that end in this location, then adds the amount on each of those routes together to determine how much flows to each location. The ``amountOut'' variable functions similarly for the flow out. Then we just create an ``input'' and ``output'' and ensure they're equal. As in some of the previous constraints, we feed the set of places into the constraint as an arguement so it will index over that set, and thus this rule functions for all the places in our network. +What we did was just construct a few additional variables; this wasn't required but makes reading and understanding the code much, much simpler. The variable ``amountIn'' looks at each route to find ones that end in this location, then adds the amount on each of those routes together to determine how much flows to each location. The ``amountOut'' variable functions similarly for the flow out. Then we just create an ``input'' and ``output'' and ensure they're equal. As in some of the previous constraints, we feed the set of places into the constraint as an argument so it will index over that set, and thus this rule functions for all the places in our network. We have now finished creating the model. Save this as a .py file before continuing. From c6d368690d34b44ec1bef002bee14c0af466c843 Mon Sep 17 00:00:00 2001 From: John Siirola Date: Wed, 30 Aug 2023 17:16:05 -0600 Subject: [PATCH 08/23] Fix handling of 0*term in sums --- pyomo/repn/linear.py | 4 ++++ pyomo/repn/quadratic.py | 7 ++++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/pyomo/repn/linear.py b/pyomo/repn/linear.py index bae480f22d0..3ea9c599b77 100644 --- a/pyomo/repn/linear.py +++ b/pyomo/repn/linear.py @@ -158,6 +158,10 @@ def append(self, other): return mult = other.multiplier + if not mult: + # 0 * other, so there is nothing to add/change about + # self. We can just exit now. + return if other.constant: self.constant += mult * other.constant if other.linear: diff --git a/pyomo/repn/quadratic.py b/pyomo/repn/quadratic.py index 5e102c89833..4195d95e2e7 100644 --- a/pyomo/repn/quadratic.py +++ b/pyomo/repn/quadratic.py @@ -141,7 +141,12 @@ def append(self, other): return mult = other.multiplier - self.constant += mult * other.constant + if not mult: + # 0 * other, so there is nothing to add/change about + # self. We can just exit now. + return + if other.constant: + self.constant += mult * other.constant if other.linear: _merge_dict(self.linear, mult, other.linear) if other.quadratic: From 88a2fcd353580bf8a0222402f27416af5ebf5119 Mon Sep 17 00:00:00 2001 From: John Siirola Date: Wed, 30 Aug 2023 17:17:29 -0600 Subject: [PATCH 09/23] Fix inconsistency updating var_map between var and monomial expressions --- pyomo/repn/linear.py | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/pyomo/repn/linear.py b/pyomo/repn/linear.py index 3ea9c599b77..f4c8d5eb381 100644 --- a/pyomo/repn/linear.py +++ b/pyomo/repn/linear.py @@ -626,6 +626,17 @@ def _before_monomial(visitor, child): except (ValueError, ArithmeticError): return True, None + # We want to check / update the var_map before processing "0" + # coefficients to that we are consistent with what gets added to the + # var_map (e.g., 0*x*y: y is processed by _before_var and will + # always be added, but x is processed here) + _id = id(arg2) + if _id not in visitor.var_map: + if arg2.fixed: + return False, (_CONSTANT, arg1 * visitor._eval_fixed(arg2)) + visitor.var_map[_id] = arg2 + visitor.var_order[_id] = len(visitor.var_order) + # Trap multiplication by 0 and nan. if not arg1: if arg2.fixed: @@ -640,12 +651,6 @@ def _before_monomial(visitor, child): ) return False, (_CONSTANT, arg1) - _id = id(arg2) - if _id not in visitor.var_map: - if arg2.fixed: - return False, (_CONSTANT, arg1 * visitor._eval_fixed(arg2)) - visitor.var_map[_id] = arg2 - visitor.var_order[_id] = len(visitor.var_order) ans = visitor.Result() ans.linear[_id] = arg1 return False, (_LINEAR, ans) From 4f4da72443b876cecc0197f2933089f9da969402 Mon Sep 17 00:00:00 2001 From: John Siirola Date: Wed, 30 Aug 2023 17:17:49 -0600 Subject: [PATCH 10/23] NFC: clarify doc comment --- pyomo/repn/linear.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/pyomo/repn/linear.py b/pyomo/repn/linear.py index f4c8d5eb381..fc7b565c7d7 100644 --- a/pyomo/repn/linear.py +++ b/pyomo/repn/linear.py @@ -143,9 +143,11 @@ def append(self, other): Notes ----- This method assumes that the operator was "+". It is implemented - so that we can directly use a LinearRepn() as a data object in - the expression walker (thereby avoiding the function call for a - custom callback) + so that we can directly use a LinearRepn() as a `data` object in + the expression walker (thereby allowing us to use the default + implementation of acceptChildResult [which calls + `data.append()`] and avoid the function call for a custom + callback). """ # Note that self.multiplier will always be 1 (we only call append() From f572c2d47ef1fc7cd403d64cf5454b8ea8c02941 Mon Sep 17 00:00:00 2001 From: John Siirola Date: Wed, 30 Aug 2023 17:19:27 -0600 Subject: [PATCH 11/23] Add tests for 0* terms and None*terms --- pyomo/repn/tests/test_linear.py | 55 ++++++++++++++++++++++++++--- pyomo/repn/tests/test_quadratic.py | 56 +++++++++++++++++++++++++++++- 2 files changed, 106 insertions(+), 5 deletions(-) diff --git a/pyomo/repn/tests/test_linear.py b/pyomo/repn/tests/test_linear.py index cee887e9bab..1501ecfcc9d 100644 --- a/pyomo/repn/tests/test_linear.py +++ b/pyomo/repn/tests/test_linear.py @@ -544,15 +544,13 @@ def test_monomial(self): cfg = VisitorConfig() with LoggingIntercept() as LOG: repn = LinearRepnVisitor(*cfg).walk_expression(param_expr) - self.assertIn( - "DEPRECATED: Encountered 0*nan in expression tree.", LOG.getvalue() - ) + self.assertEqual(LOG.getvalue(), "") self.assertEqual(cfg.subexpr, {}) self.assertEqual(cfg.var_map, {}) self.assertEqual(cfg.var_order, {}) self.assertEqual(repn.multiplier, 1) - self.assertEqual(repn.constant, 0) + self.assertEqual(str(repn.constant), 'InvalidNumber(nan)') self.assertEqual(repn.linear, {}) self.assertEqual(repn.nonlinear, None) @@ -1608,3 +1606,52 @@ def test_nonnumeric(self): self.assertEqual(str(repn.constant), 'InvalidNumber(array([3, 4]))') self.assertEqual(repn.linear, {}) self.assertEqual(repn.nonlinear, None) + + def test_zero_elimination(self): + m = ConcreteModel() + m.x = Var(range(4)) + + e = 0 * m.x[0] + 0 * m.x[1] * m.x[2] + 0 * log(m.x[3]) + + cfg = VisitorConfig() + repn = LinearRepnVisitor(*cfg).walk_expression(e) + self.assertEqual(cfg.subexpr, {}) + self.assertEqual( + cfg.var_map, + { + id(m.x[0]): m.x[0], + id(m.x[1]): m.x[1], + id(m.x[2]): m.x[2], + id(m.x[3]): m.x[3], + }, + ) + self.assertEqual( + cfg.var_order, {id(m.x[0]): 0, id(m.x[1]): 1, id(m.x[2]): 2, id(m.x[3]): 3} + ) + self.assertEqual(repn.multiplier, 1) + self.assertEqual(repn.constant, 0) + self.assertEqual(repn.linear, {}) + self.assertEqual(repn.nonlinear, None) + + m.p = Param(mutable=True, within=Any, initialize=None) + e = m.p * m.x[0] + m.p * m.x[1] * m.x[2] + m.p * log(m.x[3]) + + cfg = VisitorConfig() + repn = LinearRepnVisitor(*cfg).walk_expression(e) + self.assertEqual(cfg.subexpr, {}) + self.assertEqual( + cfg.var_map, + { + id(m.x[0]): m.x[0], + id(m.x[1]): m.x[1], + id(m.x[2]): m.x[2], + id(m.x[3]): m.x[3], + }, + ) + self.assertEqual( + cfg.var_order, {id(m.x[0]): 0, id(m.x[1]): 1, id(m.x[2]): 2, id(m.x[3]): 3} + ) + self.assertEqual(repn.multiplier, 1) + self.assertEqual(repn.constant, 0) + self.assertEqual(repn.linear, {id(m.x[0]): InvalidNumber(None)}) + self.assertEqual(repn.nonlinear, InvalidNumber(None)) diff --git a/pyomo/repn/tests/test_quadratic.py b/pyomo/repn/tests/test_quadratic.py index 7832fedee36..95f35a62969 100644 --- a/pyomo/repn/tests/test_quadratic.py +++ b/pyomo/repn/tests/test_quadratic.py @@ -19,8 +19,9 @@ SumExpression, ) from pyomo.repn.quadratic import QuadraticRepnVisitor +from pyomo.repn.util import InvalidNumber -from pyomo.environ import ConcreteModel, Var +from pyomo.environ import ConcreteModel, Var, Param, Any, log class VisitorConfig(object): @@ -291,3 +292,56 @@ def test_pow(self): {(id(m.x), id(m.x)): 9, (id(m.y), id(m.y)): 16, (id(m.x), id(m.y)): 24}, ) self.assertEqual(repn.nonlinear, None) + + def test_zero_elimination(self): + m = ConcreteModel() + m.x = Var(range(4)) + + e = 0 * m.x[0] + 0 * m.x[1] * m.x[2] + 0 * log(m.x[3]) + + cfg = VisitorConfig() + repn = QuadraticRepnVisitor(*cfg).walk_expression(e) + self.assertEqual(cfg.subexpr, {}) + self.assertEqual( + cfg.var_map, + { + id(m.x[0]): m.x[0], + id(m.x[1]): m.x[1], + id(m.x[2]): m.x[2], + id(m.x[3]): m.x[3], + }, + ) + self.assertEqual( + cfg.var_order, {id(m.x[0]): 0, id(m.x[1]): 1, id(m.x[2]): 2, id(m.x[3]): 3} + ) + self.assertEqual(repn.multiplier, 1) + self.assertEqual(repn.constant, 0) + self.assertEqual(repn.linear, {}) + self.assertEqual(repn.quadratic, None) + self.assertEqual(repn.nonlinear, None) + + m.p = Param(mutable=True, within=Any, initialize=None) + e = m.p * m.x[0] + m.p * m.x[1] * m.x[2] + m.p * log(m.x[3]) + + cfg = VisitorConfig() + repn = QuadraticRepnVisitor(*cfg).walk_expression(e) + self.assertEqual(cfg.subexpr, {}) + self.assertEqual( + cfg.var_map, + { + id(m.x[0]): m.x[0], + id(m.x[1]): m.x[1], + id(m.x[2]): m.x[2], + id(m.x[3]): m.x[3], + }, + ) + self.assertEqual( + cfg.var_order, {id(m.x[0]): 0, id(m.x[1]): 1, id(m.x[2]): 2, id(m.x[3]): 3} + ) + self.assertEqual(repn.multiplier, 1) + self.assertEqual(repn.constant, 0) + self.assertEqual(repn.linear, {id(m.x[0]): InvalidNumber(None)}) + self.assertEqual( + repn.quadratic, {(id(m.x[1]), id(m.x[2])): InvalidNumber(None)} + ) + self.assertEqual(repn.nonlinear, InvalidNumber(None)) From 52fe9f0b90f191753d0ff08da224f62643af2a1d Mon Sep 17 00:00:00 2001 From: John Siirola Date: Thu, 31 Aug 2023 08:17:56 -0600 Subject: [PATCH 12/23] LP: warn for unused export suffixes --- pyomo/repn/plugins/lp_writer.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/pyomo/repn/plugins/lp_writer.py b/pyomo/repn/plugins/lp_writer.py index 5f3972abce6..d486b707765 100644 --- a/pyomo/repn/plugins/lp_writer.py +++ b/pyomo/repn/plugins/lp_writer.py @@ -320,6 +320,27 @@ def write(self, model): timer.toc('Initialized column order', level=logging.DEBUG) + # We don't export any suffix information to the LP file + # + if component_map[Suffix]: + suffixesByName = {} + for block in component_map[Suffix]: + for suffix in block.component_objects( + Suffix, active=True, descend_into=False, sort=sorter + ): + if not (suffix.direction & Suffix.EXPORT): + continue + if suffix.name in suffixesByName: + suffixesByName[suffix.name].append(suffix) + else: + suffixesByName[suffix.name] = [suffix] + for name, suffixes in suffixesByName.items(): + logger.warning( + f"EXPORT Suffix {name} found on {len(suffixes)} blocks:\n\t" + + "\n\t".join(s.name for s in suffixes) + + "LP writer cannot export suffixes to LP file. Skipping." + ) + ostream.write(f"\\* Source Pyomo model name={model.name} *\\\n\n") # From 8e6b11c68c978db1179162e87f1a65bced773e46 Mon Sep 17 00:00:00 2001 From: John Siirola Date: Thu, 31 Aug 2023 08:41:30 -0600 Subject: [PATCH 13/23] Ignore empty/import suffixes --- pyomo/repn/plugins/lp_writer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyomo/repn/plugins/lp_writer.py b/pyomo/repn/plugins/lp_writer.py index d486b707765..5275f300ed6 100644 --- a/pyomo/repn/plugins/lp_writer.py +++ b/pyomo/repn/plugins/lp_writer.py @@ -328,7 +328,7 @@ def write(self, model): for suffix in block.component_objects( Suffix, active=True, descend_into=False, sort=sorter ): - if not (suffix.direction & Suffix.EXPORT): + if not suffix.export_enabled() or not suffix: continue if suffix.name in suffixesByName: suffixesByName[suffix.name].append(suffix) From 6bcea522721607e2fb922e75a2e76eb7194aa962 Mon Sep 17 00:00:00 2001 From: John Siirola Date: Thu, 31 Aug 2023 08:42:12 -0600 Subject: [PATCH 14/23] report suffixes grouped by local_name --- pyomo/repn/plugins/lp_writer.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pyomo/repn/plugins/lp_writer.py b/pyomo/repn/plugins/lp_writer.py index 5275f300ed6..309afa8ae70 100644 --- a/pyomo/repn/plugins/lp_writer.py +++ b/pyomo/repn/plugins/lp_writer.py @@ -330,10 +330,11 @@ def write(self, model): ): if not suffix.export_enabled() or not suffix: continue - if suffix.name in suffixesByName: - suffixesByName[suffix.name].append(suffix) + name = suffix.local_name + if name in suffixesByName: + suffixesByName[name].append(suffix) else: - suffixesByName[suffix.name] = [suffix] + suffixesByName[name] = [suffix] for name, suffixes in suffixesByName.items(): logger.warning( f"EXPORT Suffix {name} found on {len(suffixes)} blocks:\n\t" From e936e5c47783ed799be63766ba12a31e60c686a7 Mon Sep 17 00:00:00 2001 From: John Siirola Date: Thu, 31 Aug 2023 08:42:30 -0600 Subject: [PATCH 15/23] Improve suffix warning message --- pyomo/repn/plugins/lp_writer.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/pyomo/repn/plugins/lp_writer.py b/pyomo/repn/plugins/lp_writer.py index 309afa8ae70..fab94d313d5 100644 --- a/pyomo/repn/plugins/lp_writer.py +++ b/pyomo/repn/plugins/lp_writer.py @@ -336,10 +336,12 @@ def write(self, model): else: suffixesByName[name] = [suffix] for name, suffixes in suffixesByName.items(): + n = len(suffixes) + plural = 's' if n > 1 else '' logger.warning( - f"EXPORT Suffix {name} found on {len(suffixes)} blocks:\n\t" - + "\n\t".join(s.name for s in suffixes) - + "LP writer cannot export suffixes to LP file. Skipping." + f"EXPORT Suffix '{name}' found on {n} block{plural}:\n " + + "\n ".join(s.name for s in suffixes) + + "\nLP writer cannot export suffixes to LP files. Skipping." ) ostream.write(f"\\* Source Pyomo model name={model.name} *\\\n\n") From c68a9f5775d6a80386ecb77a41d286b52436e184 Mon Sep 17 00:00:00 2001 From: John Siirola Date: Thu, 31 Aug 2023 08:42:47 -0600 Subject: [PATCH 16/23] Add tests --- pyomo/repn/tests/cpxlp/test_lpv2.py | 68 +++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 pyomo/repn/tests/cpxlp/test_lpv2.py diff --git a/pyomo/repn/tests/cpxlp/test_lpv2.py b/pyomo/repn/tests/cpxlp/test_lpv2.py new file mode 100644 index 00000000000..8ec0582acc2 --- /dev/null +++ b/pyomo/repn/tests/cpxlp/test_lpv2.py @@ -0,0 +1,68 @@ +# ___________________________________________________________________________ +# +# Pyomo: Python Optimization Modeling Objects +# Copyright (c) 2008-2022 +# National Technology and Engineering Solutions of Sandia, LLC +# Under the terms of Contract DE-NA0003525 with National Technology and +# Engineering Solutions of Sandia, LLC, the U.S. Government retains certain +# rights in this software. +# This software is distributed under the 3-clause BSD License. +# ___________________________________________________________________________ + +from io import StringIO + +import pyomo.common.unittest as unittest + +from pyomo.common.log import LoggingIntercept +from pyomo.environ import ConcreteModel, Block, Constraint, Var, Objective, Suffix + +from pyomo.repn.plugins.lp_writer import LPWriter + +class TestLPv2(unittest.TestCase): + def test_warn_export_suffixes(self): + m = ConcreteModel() + m.x = Var() + m.obj = Objective(expr=m.x) + m.con = Constraint(expr=m.x>=2) + m.b = Block() + m.ignored = Suffix(direction=Suffix.IMPORT) + m.duals = Suffix(direction=Suffix.IMPORT_EXPORT) + m.b.duals = Suffix(direction=Suffix.IMPORT_EXPORT) + m.b.scaling = Suffix(direction=Suffix.EXPORT) + + # Empty suffixes are ignored + writer = LPWriter() + with LoggingIntercept() as LOG: + writer.write(m, StringIO()) + self.assertEqual(LOG.getvalue(), "") + + # Import are ignored, export and import/export are warned + m.duals[m.con] = 5 + m.ignored[m.x] = 6 + m.b.scaling[m.x] = 7 + + writer = LPWriter() + with LoggingIntercept() as LOG: + writer.write(m, StringIO()) + self.assertEqual(LOG.getvalue(), """EXPORT Suffix 'duals' found on 1 block: + duals +LP writer cannot export suffixes to LP files. Skipping. +EXPORT Suffix 'scaling' found on 1 block: + b.scaling +LP writer cannot export suffixes to LP files. Skipping. +""") + + # Counting works correctly + m.b.duals[m.x] = 7 + + writer = LPWriter() + with LoggingIntercept() as LOG: + writer.write(m, StringIO()) + self.assertEqual(LOG.getvalue(), """EXPORT Suffix 'duals' found on 2 blocks: + duals + b.duals +LP writer cannot export suffixes to LP files. Skipping. +EXPORT Suffix 'scaling' found on 1 block: + b.scaling +LP writer cannot export suffixes to LP files. Skipping. +""") From c9b2252038f11a091dfc1bddac49173caeb1c2d6 Mon Sep 17 00:00:00 2001 From: John Siirola Date: Thu, 31 Aug 2023 09:51:41 -0600 Subject: [PATCH 17/23] Apply black --- pyomo/repn/tests/cpxlp/test_lpv2.py | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/pyomo/repn/tests/cpxlp/test_lpv2.py b/pyomo/repn/tests/cpxlp/test_lpv2.py index 8ec0582acc2..fbe4f15fbe9 100644 --- a/pyomo/repn/tests/cpxlp/test_lpv2.py +++ b/pyomo/repn/tests/cpxlp/test_lpv2.py @@ -18,18 +18,19 @@ from pyomo.repn.plugins.lp_writer import LPWriter + class TestLPv2(unittest.TestCase): def test_warn_export_suffixes(self): m = ConcreteModel() m.x = Var() m.obj = Objective(expr=m.x) - m.con = Constraint(expr=m.x>=2) + m.con = Constraint(expr=m.x >= 2) m.b = Block() m.ignored = Suffix(direction=Suffix.IMPORT) m.duals = Suffix(direction=Suffix.IMPORT_EXPORT) m.b.duals = Suffix(direction=Suffix.IMPORT_EXPORT) m.b.scaling = Suffix(direction=Suffix.EXPORT) - + # Empty suffixes are ignored writer = LPWriter() with LoggingIntercept() as LOG: @@ -44,13 +45,16 @@ def test_warn_export_suffixes(self): writer = LPWriter() with LoggingIntercept() as LOG: writer.write(m, StringIO()) - self.assertEqual(LOG.getvalue(), """EXPORT Suffix 'duals' found on 1 block: + self.assertEqual( + LOG.getvalue(), + """EXPORT Suffix 'duals' found on 1 block: duals LP writer cannot export suffixes to LP files. Skipping. EXPORT Suffix 'scaling' found on 1 block: b.scaling LP writer cannot export suffixes to LP files. Skipping. -""") +""", + ) # Counting works correctly m.b.duals[m.x] = 7 @@ -58,11 +62,14 @@ def test_warn_export_suffixes(self): writer = LPWriter() with LoggingIntercept() as LOG: writer.write(m, StringIO()) - self.assertEqual(LOG.getvalue(), """EXPORT Suffix 'duals' found on 2 blocks: + self.assertEqual( + LOG.getvalue(), + """EXPORT Suffix 'duals' found on 2 blocks: duals b.duals LP writer cannot export suffixes to LP files. Skipping. EXPORT Suffix 'scaling' found on 1 block: b.scaling LP writer cannot export suffixes to LP files. Skipping. -""") +""", + ) From 95dc7a69b5647608fa6824332e09db0b6b64a72f Mon Sep 17 00:00:00 2001 From: John Siirola Date: Thu, 31 Aug 2023 09:54:41 -0600 Subject: [PATCH 18/23] Ensure templatize_constraint always returns an expression --- pyomo/core/expr/relational_expr.py | 7 +++++++ pyomo/core/expr/template_expr.py | 6 +++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/pyomo/core/expr/relational_expr.py b/pyomo/core/expr/relational_expr.py index 2909be95c5a..6e4831d5c0c 100644 --- a/pyomo/core/expr/relational_expr.py +++ b/pyomo/core/expr/relational_expr.py @@ -458,3 +458,10 @@ def _generate_relational_expression(etype, lhs, rhs): ) else: return InequalityExpression((lhs, rhs), _relational_op[etype][2]) + + +def tuple_to_relational_expr(args): + if len(args) == 2: + return EqualityExpression(args) + else: + return inequality(*args) diff --git a/pyomo/core/expr/template_expr.py b/pyomo/core/expr/template_expr.py index 4682844fef0..5009a2497d7 100644 --- a/pyomo/core/expr/template_expr.py +++ b/pyomo/core/expr/template_expr.py @@ -34,6 +34,7 @@ value, is_constant, ) +from pyomo.core.expr.relational_expr import tuple_to_relational_expr from pyomo.core.expr.visitor import ( ExpressionReplacementVisitor, StreamBasedExpressionVisitor, @@ -1173,4 +1174,7 @@ def templatize_rule(block, rule, index_set): def templatize_constraint(con): - return templatize_rule(con.parent_block(), con.rule, con.index_set()) + expr, indices = templatize_rule(con.parent_block(), con.rule, con.index_set()) + if expr.__class__ is tuple: + expr = tuple_to_relational_expr(expr) + return expr, indices From 677289a3a94b80bff23c26ce6209f724db7b0638 Mon Sep 17 00:00:00 2001 From: John Siirola Date: Thu, 31 Aug 2023 09:54:48 -0600 Subject: [PATCH 19/23] Add missing import --- pyomo/core/expr/template_expr.py | 1 + 1 file changed, 1 insertion(+) diff --git a/pyomo/core/expr/template_expr.py b/pyomo/core/expr/template_expr.py index 5009a2497d7..6103e9c429c 100644 --- a/pyomo/core/expr/template_expr.py +++ b/pyomo/core/expr/template_expr.py @@ -25,6 +25,7 @@ Numeric_NPV_Mixin, register_arg_type, ARG_TYPE, + _balanced_parens, ) from pyomo.core.expr.numvalue import ( NumericValue, From ecbd7095ef12c615cb26f2b612ba4ec947b4e15d Mon Sep 17 00:00:00 2001 From: John Siirola Date: Thu, 31 Aug 2023 10:00:35 -0600 Subject: [PATCH 20/23] Add tests for templatizing rules returning tuples --- pyomo/core/tests/unit/test_template_expr.py | 47 +++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/pyomo/core/tests/unit/test_template_expr.py b/pyomo/core/tests/unit/test_template_expr.py index 069acc907cc..4b4ea494b0e 100644 --- a/pyomo/core/tests/unit/test_template_expr.py +++ b/pyomo/core/tests/unit/test_template_expr.py @@ -353,6 +353,53 @@ def c(m, i): indices[0].set_value(2) self.assertEqual(str(resolve_template(template)), 'x[2] <= 0') + def test_tuple_rules(self): + m = ConcreteModel() + m.I = RangeSet(3) + m.x = Var(m.I) + + @m.Constraint(m.I) + def c(m, i): + return (None, m.x[i], 0) + + template, indices = templatize_constraint(m.c) + self.assertEqual(len(indices), 1) + self.assertIs(indices[0]._set, m.I) + self.assertEqual(str(template), "x[_1] <= 0") + # Test that the RangeSet iterator was put back + self.assertEqual(list(m.I), list(range(1, 4))) + # Evaluate the template + indices[0].set_value(2) + self.assertEqual(str(resolve_template(template)), 'x[2] <= 0') + + @m.Constraint(m.I) + def d(m, i): + return (0, m.x[i], 10) + + template, indices = templatize_constraint(m.d) + self.assertEqual(len(indices), 1) + self.assertIs(indices[0]._set, m.I) + self.assertEqual(str(template), "0 <= x[_1] <= 10") + # Test that the RangeSet iterator was put back + self.assertEqual(list(m.I), list(range(1, 4))) + # Evaluate the template + indices[0].set_value(2) + self.assertEqual(str(resolve_template(template)), '0 <= x[2] <= 10') + + @m.Constraint(m.I) + def e(m, i): + return (m.x[i], 0) + + template, indices = templatize_constraint(m.e) + self.assertEqual(len(indices), 1) + self.assertIs(indices[0]._set, m.I) + self.assertEqual(str(template), "x[_1] == 0") + # Test that the RangeSet iterator was put back + self.assertEqual(list(m.I), list(range(1, 4))) + # Evaluate the template + indices[0].set_value(2) + self.assertEqual(str(resolve_template(template)), 'x[2] == 0') + def test_simple_rule_nonfinite_set(self): m = ConcreteModel() m.x = Var(Integers, dense=False) From e228786d0671c7b21fadbb83a41db638269303ef Mon Sep 17 00:00:00 2001 From: John Siirola Date: Thu, 31 Aug 2023 10:21:36 -0600 Subject: [PATCH 21/23] Fix kernel/aml inconsistency: make {export,import}_nabled a method --- pyomo/core/kernel/suffix.py | 6 ++---- pyomo/core/tests/unit/kernel/test_suffix.py | 16 ++++++++-------- pyomo/solvers/tests/models/base.py | 14 ++------------ 3 files changed, 12 insertions(+), 24 deletions(-) diff --git a/pyomo/core/kernel/suffix.py b/pyomo/core/kernel/suffix.py index 0416c1269f9..77079364703 100644 --- a/pyomo/core/kernel/suffix.py +++ b/pyomo/core/kernel/suffix.py @@ -102,13 +102,11 @@ def __init__(self, *args, **kwds): # Interface # - @property def export_enabled(self): """Returns :const:`True` when this suffix is enabled for export to solvers.""" return bool(self._direction & suffix.EXPORT) - @property def import_enabled(self): """Returns :const:`True` when this suffix is enabled for import from solutions.""" @@ -233,7 +231,7 @@ def export_suffix_generator(blk, datatype=_noarg, active=True, descend_into=True """ for suf in filter( lambda x: ( - x.export_enabled and ((datatype is _noarg) or (x.datatype is datatype)) + x.export_enabled() and ((datatype is _noarg) or (x.datatype is datatype)) ), blk.components(ctype=suffix._ctype, active=active, descend_into=descend_into), ): @@ -267,7 +265,7 @@ def import_suffix_generator(blk, datatype=_noarg, active=True, descend_into=True """ for suf in filter( lambda x: ( - x.import_enabled and ((datatype is _noarg) or (x.datatype is datatype)) + x.import_enabled() and ((datatype is _noarg) or (x.datatype is datatype)) ), blk.components(ctype=suffix._ctype, active=active, descend_into=descend_into), ): diff --git a/pyomo/core/tests/unit/kernel/test_suffix.py b/pyomo/core/tests/unit/kernel/test_suffix.py index 6565c6a920d..c4c75278d50 100644 --- a/pyomo/core/tests/unit/kernel/test_suffix.py +++ b/pyomo/core/tests/unit/kernel/test_suffix.py @@ -111,20 +111,20 @@ def test_import_export_enabled(self): s = suffix() s.direction = suffix.LOCAL self.assertEqual(s.direction, suffix.LOCAL) - self.assertEqual(s.export_enabled, False) - self.assertEqual(s.import_enabled, False) + self.assertEqual(s.export_enabled(), False) + self.assertEqual(s.import_enabled(), False) s.direction = suffix.IMPORT self.assertEqual(s.direction, suffix.IMPORT) - self.assertEqual(s.export_enabled, False) - self.assertEqual(s.import_enabled, True) + self.assertEqual(s.export_enabled(), False) + self.assertEqual(s.import_enabled(), True) s.direction = suffix.EXPORT self.assertEqual(s.direction, suffix.EXPORT) - self.assertEqual(s.export_enabled, True) - self.assertEqual(s.import_enabled, False) + self.assertEqual(s.export_enabled(), True) + self.assertEqual(s.import_enabled(), False) s.direction = suffix.IMPORT_EXPORT self.assertEqual(s.direction, suffix.IMPORT_EXPORT) - self.assertEqual(s.export_enabled, True) - self.assertEqual(s.import_enabled, True) + self.assertEqual(s.export_enabled(), True) + self.assertEqual(s.import_enabled(), True) with self.assertRaises(ValueError): s.direction = 'export' diff --git a/pyomo/solvers/tests/models/base.py b/pyomo/solvers/tests/models/base.py index 5adfae8c729..106e8860145 100644 --- a/pyomo/solvers/tests/models/base.py +++ b/pyomo/solvers/tests/models/base.py @@ -142,12 +142,7 @@ def save_current_solution(self, filename, **kwds): (suffix, getattr(model, suffix)) for suffix in kwds.pop('suffixes', []) ) for suf in suffixes.values(): - if isinstance(self.model, IBlock): - assert isinstance(suf, pmo.suffix) - assert suf.import_enabled - else: - assert isinstance(suf, Suffix) - assert suf.import_enabled() + assert suf.import_enabled() with open(filename, 'w') as f: # @@ -197,12 +192,7 @@ def validate_current_solution(self, **kwds): ) exclude = kwds.pop('exclude_suffixes', set()) for suf in suffixes.values(): - if isinstance(self.model, IBlock): - assert isinstance(suf, pmo.suffix) - assert suf.import_enabled - else: - assert isinstance(suf, Suffix) - assert suf.import_enabled() + assert suf.import_enabled() solution = None error_str = ( "Difference in solution for {0}.{1}:\n\tBaseline - {2}\n\tCurrent - {3}" From 134ec89fe9d510e633ed55749991562b362c4677 Mon Sep 17 00:00:00 2001 From: John Siirola Date: Thu, 31 Aug 2023 12:38:49 -0600 Subject: [PATCH 22/23] NFC: fix comment typo --- pyomo/repn/linear.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyomo/repn/linear.py b/pyomo/repn/linear.py index fc7b565c7d7..8bffbf1d49b 100644 --- a/pyomo/repn/linear.py +++ b/pyomo/repn/linear.py @@ -629,7 +629,7 @@ def _before_monomial(visitor, child): return True, None # We want to check / update the var_map before processing "0" - # coefficients to that we are consistent with what gets added to the + # coefficients so that we are consistent with what gets added to the # var_map (e.g., 0*x*y: y is processed by _before_var and will # always be added, but x is processed here) _id = id(arg2) From e15c4cf794e29cb571cc8e37c86c50270680a8d3 Mon Sep 17 00:00:00 2001 From: Miranda Mundt Date: Tue, 5 Sep 2023 12:07:00 -0600 Subject: [PATCH 23/23] Fixing typo from new typos-cli version --- pyomo/opt/base/solvers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyomo/opt/base/solvers.py b/pyomo/opt/base/solvers.py index 30b29f99b2d..0de60902af2 100644 --- a/pyomo/opt/base/solvers.py +++ b/pyomo/opt/base/solvers.py @@ -361,7 +361,7 @@ def __init__(self, **kwds): # multiple methods. self._smap_id = None - # These are ephimeral options that can be set by the user during + # These are ephemeral options that can be set by the user during # the call to solve, but will be reset to defaults if not given self._load_solutions = True self._select_index = 0