Merge branch 'main' into only-mbm-bound-constraints

Author: jsiirola

pyomo/core/base/block.py

@@ -347,10 +347,6 @@ class PseudoMap(AutoSlots.Mixin):
 
     __slots__ = ('_block', '_ctypes', '_active', '_sorted')
 
-    # If a writer cached a repn on this block, remove it when cloning
-    #  TODO: remove repn caching from the model
-    __autoslot_mappers = {'_repn': AutoSlots.encode_as_none}
-
     def __init__(self, block, ctype, active=None, sort=False):
         """
         TODO
@@ -577,6 +573,10 @@ class _BlockData(ActiveComponentData):
     """
     _Block_reserved_words = set()
 
+    # If a writer cached a repn on this block, remove it when cloning
+    #  TODO: remove repn caching from the model
+    __autoslot_mappers = {'_repn': AutoSlots.encode_as_none}
+
     def __init__(self, component):
         #
         # BLOCK DATA ELEMENTS
@@ -1348,7 +1348,7 @@ def reclassify_component_type(self, name_or_object, new_ctype,
             self._decl_order[prev] = (self._decl_order[prev][0], idx)
             self._decl_order[idx] = (obj, tmp)
 
-    def clone(self):
+    def clone(self, memo=None):
         """
         TODO
         """
@@ -1365,15 +1365,23 @@ def clone(self):
         # NonNegativeReals, etc) that are not "owned" by any blocks and
         # should be preserved as singletons.
         #
+        pc = self.parent_component()
+        if pc is self:
+            parent = self.parent_block()
+        else:
+            parent = pc
+
+        if memo is None:
+            memo = {}
+        memo['__block_scope__'] = {id(self): True, id(None): False}
+        memo[id(parent)] = parent
+
         with PauseGC():
-            new_block = copy.deepcopy(
-                self, dict(
-                    __block_scope__={id(self): True, id(None): False},
-                ))
+            new_block = copy.deepcopy(self, memo)
 
         # We need to "detangle" the new block from the original block
         # hierarchy
-        if self.parent_component() is self:
+        if pc is self:
             new_block._parent = None
         else:
             new_block._component = None

pyomo/gdp/basic_step.py

@@ -22,14 +22,10 @@
 
 def _clone_all_but_indicator_vars(self):
     """Clone everything in a Disjunct except for the indicator_vars"""
-    memo = {
-        '__block_scope__': {id(self): True, id(None): False},
+    return self.clone({
         id(self.indicator_var): self.indicator_var,
         id(self.binary_indicator_var): self.binary_indicator_var,
-    }
-    new_block = copy.deepcopy(self, memo)
-    new_block._parent = None
-    return new_block
+    })
 
 
 def _squish_singletons(tuple_iter):
@@ -46,9 +42,15 @@ def apply_basic_step(disjunctions_or_constraints):
     # Basic steps only apply to XOR'd disjunctions
     #
     disjunctions = list(obj for obj in disjunctions_or_constraints
-                        if obj.ctype == Disjunction)
+                        if obj.ctype is Disjunction)
     constraints = list(obj for obj in disjunctions_or_constraints
-                       if obj.ctype == Constraint)
+                       if obj.ctype is Constraint)
+    if len(disjunctions) + len(constraints) != len(disjunctions_or_constraints):
+        raise ValueError('apply_basic_step only accepts a list containing '
+                         'Disjunctions or Constraints')
+    if not disjunctions:
+        raise ValueError('apply_basic_step: argument list must contain at '
+                         'least one Disjunction')
     for d in disjunctions:
         if not d.xor:
             raise ValueError(
@@ -76,8 +78,9 @@ def apply_basic_step(disjunctions_or_constraints):
         #
         ans.disjuncts[idx].src = Block(ans.DISJUNCTIONS)
         for i in ans.DISJUNCTIONS:
-            tmp = _clone_all_but_indicator_vars(disjunctions[i].disjuncts[
-                idx[i] if isinstance(idx, tuple) else idx])
+            src_disj = disjunctions[i].disjuncts[
+                idx[i] if isinstance(idx, tuple) else idx]
+            tmp = _clone_all_but_indicator_vars(src_disj)
             for k,v in list(tmp.component_map().items()):
                 if v.parent_block() is not tmp:
                     # Skip indicator_var and binary_indicator_var

pyomo/gdp/disjunct.py

@@ -518,7 +518,6 @@ def set_value(self, expr):
 @ModelComponentFactory.register("Disjunction expressions.")
 class Disjunction(ActiveIndexedComponent):
     _ComponentDataClass = _DisjunctionData
-    __autoslot_mappers__ = {'_algebraic_constraint': AutoSlots.weakref_mapper}
 
     def __new__(cls, *args, **kwds):
         if cls != Disjunction:
@@ -533,7 +532,6 @@ def __init__(self, *args, **kwargs):
         self._init_expr = kwargs.pop('expr', None)
         self._init_xor = _Initializer.process(kwargs.pop('xor', True))
         self._autodisjuncts = None
-        self._algebraic_constraint = None
         kwargs.setdefault('ctype', Disjunction)
         super(Disjunction, self).__init__(*args, **kwargs)
 

pyomo/gdp/plugins/bigm.py

@@ -165,6 +165,7 @@ def __init__(self):
         }
         self._generate_debug_messages = False
         self._transformation_blocks = {}
+        self._algebraic_constraints = {}
 
     def _get_bigm_arg_list(self, bigm_args, block):
         # Gather what we know about blocks from args exactly once. We'll still
@@ -192,6 +193,7 @@ def _apply_to(self, instance, **kwds):
         finally:
             self.used_args.clear()
             self._transformation_blocks.clear()
+            self._algebraic_constraints.clear()
 
     def _apply_to_impl(self, instance, **kwds):
         if not instance.ctype in (Block, Disjunct):
@@ -289,8 +291,8 @@ def _add_xor_constraint(self, disjunction, transBlock):
         # DisjunctionData, we did something wrong.
 
         # first check if the constraint already exists
-        if disjunction._algebraic_constraint is not None:
-            return disjunction._algebraic_constraint()
+        if disjunction in self._algebraic_constraints:
+            return self._algebraic_constraints[disjunction]
 
         # add the XOR (or OR) constraints to parent block (with unique name)
         # It's indexed if this is an IndexedDisjunction, not otherwise
@@ -301,7 +303,7 @@ def _add_xor_constraint(self, disjunction, transBlock):
         orCname = unique_component_name(
             transBlock,disjunction.getname(fully_qualified=False) + '_xor')
         transBlock.add_component(orCname, orC)
-        disjunction._algebraic_constraint = weakref_ref(orC)
+        self._algebraic_constraints[disjunction] = orC
 
         return orC
 

pyomo/gdp/plugins/hull.py

@@ -199,6 +199,7 @@ def __init__(self):
             }
         self._generate_debug_messages = False
         self._transformation_blocks = {}
+        self._algebraic_constraints = {}
         self._targets = set()
 
     def _add_local_vars(self, block, local_var_dict):
@@ -235,6 +236,7 @@ def _apply_to(self, instance, **kwds):
             self._apply_to_impl(instance, **kwds)
         finally:
             self._transformation_blocks.clear()
+            self._algebraic_constraints.clear()
             self._targets = set()
 
     def _apply_to_impl(self, instance, **kwds):
@@ -341,8 +343,8 @@ def _add_xor_constraint(self, disjunction, transBlock):
         # Put XOR constraint on the transformation block
 
         # check if the constraint already exists
-        if disjunction._algebraic_constraint is not None:
-            return disjunction._algebraic_constraint()
+        if disjunction in self._algebraic_constraints:
+            return self._algebraic_constraints[disjunction]
 
         # add the XOR constraints to parent block (with unique name) It's
         # indexed if this is an IndexedDisjunction, not otherwise
@@ -351,7 +353,7 @@ def _add_xor_constraint(self, disjunction, transBlock):
             unique_component_name(transBlock,
                                   disjunction.getname(
                                       fully_qualified=True) + '_xor'), orC)
-        disjunction._algebraic_constraint = weakref_ref(orC)
+        self._algebraic_constraints[disjunction] = orC
 
         return orC
 

pyomo/gdp/plugins/multiple_bigm.py

@@ -191,7 +191,7 @@ def __init__(self):
                                 # the network.expand_arcs transformation
         }
         self._transformation_blocks = {}
-        self._arg_list = {}
+        self._algebraic_constraints = {}
 
     def _apply_to(self, instance, **kwds):
         self.used_args = ComponentMap()
@@ -200,7 +200,7 @@ def _apply_to(self, instance, **kwds):
         finally:
             self.used_args.clear()
             self._transformation_blocks.clear()
-            self._arg_list.clear()
+            self._algebraic_constraints.clear()
 
     def _apply_to_impl(self, instance, **kwds):
         if not instance.ctype in (Block, Disjunct):
@@ -630,8 +630,8 @@ def _add_exactly_one_constraint(self, disjunction, transBlock):
         # Put XOR constraint on the transformation block
 
         # check if the constraint already exists
-        if disjunction.algebraic_constraint is not None:
-            return disjunction.algebraic_constraint
+        if disjunction in self._algebraic_constraints:
+            return self._algebraic_constraints[disjunction]
 
         # add the XOR constraints to parent block (with unique name) It's
         # indexed if this is an IndexedDisjunction, not otherwise
@@ -640,7 +640,7 @@ def _add_exactly_one_constraint(self, disjunction, transBlock):
             unique_component_name(transBlock,
                                   disjunction.getname(
                                       fully_qualified=False) + '_xor'), orC)
-        disjunction._algebraic_constraint = weakref_ref(orC)
+        self._algebraic_constraints[disjunction] = orC
 
         return orC
 

pyomo/gdp/tests/common_tests.py

@@ -876,8 +876,7 @@ def check_disjunction_data_target(self, transformation):
     # suppose we transform the next one separately
     TransformationFactory('gdp.%s' % transformation).apply_to(
         m, targets=[m.disjunction[1]])
-    # we added to the same XOR constraint before
-    self.assertIsInstance(transBlock.disjunction_xor[1],
+    self.assertIsInstance(m.disjunction[1].algebraic_constraint,
                           constraint._GeneralConstraintData)
     transBlock = m.component("_pyomo_gdp_%s_reformulation_4" % transformation)
     self.assertIsInstance(transBlock, Block)

pyomo/gdp/tests/test_basic_step.py

@@ -139,5 +139,19 @@ def test_indicator_var_references(self):
         self.assertIs(refs[0][None], m.d[0].indicator_var)
         self.assertIs(refs[1][None], m.d[1].indicator_var)
 
+    def test_arg_errors(self):
+        m = models.makeTwoTermDisj()
+        m.simple = Constraint(expr=m.x <= m.a + 1)
+
+        with self.assertRaisesRegex(
+                ValueError, 'apply_basic_step only accepts a list containing '
+                'Disjunctions or Constraints'):
+            apply_basic_step([m.disjunction, m.simple, m.x])
+        with self.assertRaisesRegex(
+                ValueError, 'apply_basic_step: argument list must contain at '
+                'least one Disjunction'):
+            apply_basic_step([m.simple, m.simple])
+
+
 if __name__ == '__main__':
     unittest.main()

pyomo/gdp/tests/test_hull.py

@@ -18,6 +18,7 @@
                            RealSet, ComponentMap, value, log, ConcreteModel,
                            Any, Suffix, SolverFactory, RangeSet, Param,
                            Objective, TerminationCondition, Reference)
+from pyomo.core.base import constraint
 from pyomo.core.expr.sympy_tools import sympy_available
 from pyomo.repn import generate_standard_repn
 
@@ -1072,7 +1073,10 @@ def check_second_iteration(self, model):
             secondTerm)), 1)
 
         orig = model.component("_pyomo_gdp_hull_reformulation")
-        self.assertEqual(len(orig.disjunctionList_xor), 2)
+        self.assertIsInstance(model.disjunctionList[1].algebraic_constraint,
+                              constraint._GeneralConstraintData)
+        self.assertIsInstance(model.disjunctionList[0].algebraic_constraint,
+                              constraint._GeneralConstraintData)
         self.assertFalse(model.disjunctionList[1].active)
         self.assertFalse(model.disjunctionList[0].active)