From 45691f712889c06a45b8cefaa27801c32492ad3c Mon Sep 17 00:00:00 2001 From: Emma Johnson Date: Mon, 2 Dec 2024 13:35:10 -0700 Subject: [PATCH 1/9] modifying the FME post-processing objective without creating and deleting it every iteration --- pyomo/contrib/fme/fourier_motzkin_elimination.py | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/pyomo/contrib/fme/fourier_motzkin_elimination.py b/pyomo/contrib/fme/fourier_motzkin_elimination.py index 021650e8f9a..ad3a6b5aeb2 100644 --- a/pyomo/contrib/fme/fourier_motzkin_elimination.py +++ b/pyomo/contrib/fme/fourier_motzkin_elimination.py @@ -730,12 +730,9 @@ def post_process_fme_constraints( continue # deactivate the constraint projected_constraints[i].deactivate() - m.del_component(obj) - # make objective to maximize its infeasibility - obj = Objective( - expr=projected_constraints[i].body - projected_constraints[i].lower - ) - m.add_component(obj_name, obj) + # Our constraint looks like: 0 <= a^Tx - b, so make objective to + # maximize its infeasibility + obj.expr = projected_constraints[i].body - projected_constraints[i].lower results = solver_factory.solve(m) if results.solver.termination_condition == TerminationCondition.unbounded: obj_val = -float('inf') @@ -753,13 +750,13 @@ def post_process_fme_constraints( obj_val = value(obj) # if we couldn't make it infeasible, it's useless if obj_val >= tolerance: - m.del_component(projected_constraints[i]) del projected_constraints[i] else: projected_constraints[i].activate() # clean up m.del_component(obj) + del obj for obj in active_objs: obj.activate() # undo relax integrality From f382c1e29f240f560250b9aa82bab3747e3a6098 Mon Sep 17 00:00:00 2001 From: Emma Johnson Date: Mon, 2 Dec 2024 13:43:00 -0700 Subject: [PATCH 2/9] Removing outdated docs from the ComponentData docstring --- pyomo/core/base/component.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/pyomo/core/base/component.py b/pyomo/core/base/component.py index a5763264b14..5aa260cd9b2 100644 --- a/pyomo/core/base/component.py +++ b/pyomo/core/base/component.py @@ -732,11 +732,7 @@ class ComponentData(ComponentBase): This is the base class for the component data used in Pyomo modeling components. Subclasses of ComponentData are used in indexed components, and this class assumes that indexed - components are subclasses of IndexedComponent. Note that - ComponentData instances do not store their index. This makes - some operations significantly more expensive, but these are (a) - associated with I/O generation and (b) this cost can be managed - with caches. + components are subclasses of IndexedComponent. Constructor arguments: owner The component that owns this data object From 4dcfc6590bcaab1593c0aacb536648ab098fdb88 Mon Sep 17 00:00:00 2001 From: Emma Johnson Date: Mon, 2 Dec 2024 14:57:08 -0700 Subject: [PATCH 3/9] Raising a ValueError when a ComponentData is passed to del_component --- pyomo/core/base/block.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/pyomo/core/base/block.py b/pyomo/core/base/block.py index 656bf6008c4..de94dbe8ae8 100644 --- a/pyomo/core/base/block.py +++ b/pyomo/core/base/block.py @@ -34,6 +34,7 @@ from pyomo.common.timing import ConstructionTimer from pyomo.core.base.component import ( Component, + ComponentData, ActiveComponentData, ModelComponentFactory, ) @@ -1139,6 +1140,14 @@ def del_component(self, name_or_object): # return name = obj.local_name + if isinstance(obj, ComponentData) and not isinstance(obj, Component): + raise ValueError( + "Argument '%s' to del_component is a ComponentData object. Please " + "use the Python 'del' function to delete members of indexed " + "Pyomo objects. The del_component function can only be used to " + "delete IndexedComponents and ScalarComponents." % name + ) + if name in self._Block_reserved_words: raise ValueError( "Attempting to delete a reserved block component:\n\t%s" % (obj.name,) From bd2fed08f0ac4f2753e568c133f127bb47dedabe Mon Sep 17 00:00:00 2001 From: Emma Johnson Date: Tue, 3 Dec 2024 11:35:45 -0700 Subject: [PATCH 4/9] Removing unnecessary line in FME post-processing --- pyomo/contrib/fme/fourier_motzkin_elimination.py | 1 - 1 file changed, 1 deletion(-) diff --git a/pyomo/contrib/fme/fourier_motzkin_elimination.py b/pyomo/contrib/fme/fourier_motzkin_elimination.py index ad3a6b5aeb2..4ff9302334e 100644 --- a/pyomo/contrib/fme/fourier_motzkin_elimination.py +++ b/pyomo/contrib/fme/fourier_motzkin_elimination.py @@ -756,7 +756,6 @@ def post_process_fme_constraints( # clean up m.del_component(obj) - del obj for obj in active_objs: obj.activate() # undo relax integrality From cc88554e091fcbe0c6ee2e5f16588389610217a1 Mon Sep 17 00:00:00 2001 From: Emma Johnson Date: Tue, 7 Jan 2025 07:30:55 -0700 Subject: [PATCH 5/9] Adding a test confirming that John is right (no surprises there) --- pyomo/core/tests/unit/test_block.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/pyomo/core/tests/unit/test_block.py b/pyomo/core/tests/unit/test_block.py index 36e779943a3..788c38d6889 100644 --- a/pyomo/core/tests/unit/test_block.py +++ b/pyomo/core/tests/unit/test_block.py @@ -50,6 +50,7 @@ ComponentUID, Any, ) +from pyomo.common.collections import ComponentSet from pyomo.common.log import LoggingIntercept from pyomo.common.tempfiles import TempfileManager from pyomo.core.base.block import ( @@ -1345,6 +1346,25 @@ def test_add_del_component(self): self.assertFalse('x' in m.__dict__) self.assertIs(m.component('x'), None) + def test_del_component_data(self): + m = ConcreteModel() + self.assertFalse(m.contains_component(Var)) + x = m.x = Var([1, 2, 3]) + self.assertTrue(m.contains_component(Var)) + self.assertIs(m.component('x'), x) + del m.x[1] + self.assertTrue(m.contains_component(Var)) + self.assertTrue('x' in m.__dict__) + self.assertEqual(len(m.x), 2) + self.assertIn(m.x[2], ComponentSet(m.x.values())) + self.assertIn(m.x[3], ComponentSet(m.x.values())) + + m.del_component(m.x[2]) + self.assertTrue(m.contains_component(Var)) + self.assertTrue('x' in m.__dict__) + self.assertEqual(len(m.x), 1) + self.assertIn(m.x[3], ComponentSet(m.x.values())) + def test_reclassify_component(self): m = Block() m.a = Var() From 9759c8810cd2e9a597518f96a35935fff4e961eb Mon Sep 17 00:00:00 2001 From: Emma Johnson Date: Mon, 24 Mar 2025 13:30:21 -0600 Subject: [PATCH 6/9] In-lining self.component so that we can check for ComponentDatas, fixing my test --- pyomo/core/base/block.py | 35 ++++++++++++++++------------- pyomo/core/tests/unit/test_block.py | 14 +++++++++++- 2 files changed, 33 insertions(+), 16 deletions(-) diff --git a/pyomo/core/base/block.py b/pyomo/core/base/block.py index e62c755858c..f0e3067a8b5 100644 --- a/pyomo/core/base/block.py +++ b/pyomo/core/base/block.py @@ -1126,23 +1126,28 @@ def del_component(self, name_or_object): """ Delete a component from this block. """ - obj = self.component(name_or_object) - # FIXME: Is this necessary? Should this raise an exception? - if obj is None: - return - - # FIXME: Is this necessary? Should this raise an exception? - # if name not in self._decl: - # return + # in-lining self.component(name_or_object) so that we can add the + # additional check of whether or not name_or_object is a ComponentData + if isinstance(name_or_object, str): + if name_or_object in self._decl: + obj = self._decl_order[self._decl[name_or_object]][0] + else: + try: + obj = name_or_object.parent_component() + if obj is not name_or_object: + raise ValueError( + "Argument '%s' to del_component is a ComponentData object. " + "Please use the Python 'del' function to delete members of " + "indexed Pyomo components. The del_component function can " + "only be used to delete IndexedComponents and " + "ScalarComponents." % name_or_object.local_name + ) + except AttributeError: + # obj is None. Maintaining current behavior, but perhaps this + # should raise an exception? + return name = obj.local_name - if isinstance(obj, ComponentData) and not isinstance(obj, Component): - raise ValueError( - "Argument '%s' to del_component is a ComponentData object. Please " - "use the Python 'del' function to delete members of indexed " - "Pyomo objects. The del_component function can only be used to " - "delete IndexedComponents and ScalarComponents." % name - ) if name in self._Block_reserved_words: raise ValueError( diff --git a/pyomo/core/tests/unit/test_block.py b/pyomo/core/tests/unit/test_block.py index 0d7c38cc9f5..b12c5714d40 100644 --- a/pyomo/core/tests/unit/test_block.py +++ b/pyomo/core/tests/unit/test_block.py @@ -1359,7 +1359,19 @@ def test_del_component_data(self): self.assertIn(m.x[2], ComponentSet(m.x.values())) self.assertIn(m.x[3], ComponentSet(m.x.values())) - m.del_component(m.x[2]) + # This fails: + with self.assertRaisesRegex( + ValueError, + r"Argument 'x\[2\]' to del_component is a ComponentData object. " + r"Please use the Python 'del' function to delete members of " + r"indexed Pyomo components. The del_component function can " + r"only be used to delete IndexedComponents and " + r"ScalarComponents.", + ): + m.del_component(m.x[2]) + + # But we can use del + del m.x[2] self.assertTrue(m.contains_component(Var)) self.assertTrue('x' in m.__dict__) self.assertEqual(len(m.x), 1) From b083f0cae24a40ef6146d68625b34a2e2b7c26b7 Mon Sep 17 00:00:00 2001 From: Emma Johnson Date: Mon, 24 Mar 2025 14:01:13 -0600 Subject: [PATCH 7/9] Fixing a bug where obj could be undefined (probably for deleting something that doesn't exist --- pyomo/core/base/block.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pyomo/core/base/block.py b/pyomo/core/base/block.py index f0e3067a8b5..e80ea4531b7 100644 --- a/pyomo/core/base/block.py +++ b/pyomo/core/base/block.py @@ -1146,6 +1146,8 @@ def del_component(self, name_or_object): # obj is None. Maintaining current behavior, but perhaps this # should raise an exception? return + if obj is None: + return name = obj.local_name From 1bebc33f9b321534721ebb9176debea736a4fae8 Mon Sep 17 00:00:00 2001 From: Emma Johnson Date: Mon, 24 Mar 2025 14:21:03 -0600 Subject: [PATCH 8/9] Whoops, also fixing the case where the object isn't on the correct Block --- pyomo/core/base/block.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/pyomo/core/base/block.py b/pyomo/core/base/block.py index e80ea4531b7..7aa48d8a92f 100644 --- a/pyomo/core/base/block.py +++ b/pyomo/core/base/block.py @@ -1128,6 +1128,7 @@ def del_component(self, name_or_object): """ # in-lining self.component(name_or_object) so that we can add the # additional check of whether or not name_or_object is a ComponentData + obj = None if isinstance(name_or_object, str): if name_or_object in self._decl: obj = self._decl_order[self._decl[name_or_object]][0] @@ -1142,11 +1143,13 @@ def del_component(self, name_or_object): "only be used to delete IndexedComponents and " "ScalarComponents." % name_or_object.local_name ) + if obj.parent_block() is not self: + obj = None except AttributeError: - # obj is None. Maintaining current behavior, but perhaps this - # should raise an exception? - return + pass if obj is None: + # Maintaining current behavior, but perhaps this should raise an + # exception? return name = obj.local_name From f8e8c831e0c5f9a6a9a8de3a5c3bc1e411bb86c0 Mon Sep 17 00:00:00 2001 From: Emma Johnson Date: Mon, 24 Mar 2025 16:52:03 -0600 Subject: [PATCH 9/9] Fixing a use of del_component on a ComponentData in parmest --- pyomo/contrib/parmest/utils/model_utils.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pyomo/contrib/parmest/utils/model_utils.py b/pyomo/contrib/parmest/utils/model_utils.py index a604e912b86..b5ba8da6924 100644 --- a/pyomo/contrib/parmest/utils/model_utils.py +++ b/pyomo/contrib/parmest/utils/model_utils.py @@ -134,8 +134,7 @@ def convert_params_to_vars(model, param_names=None, fix_vars=False): v.name in param_names for v in identify_mutable_parameters(expr) ): new_expr = replace_expressions(expr=expr, substitution_map=substitution_map) - model.del_component(expr) - model.add_component(expr.name, pyo.Expression(rule=new_expr)) + expr.expr = new_expr # Convert Params to Vars in Constraint expressions num_constraints = len(list(model.component_objects(pyo.Constraint, active=True)))