Merge pull request #3366 from emma58/hull-performance-reference-issue

GDP: Fix performance degredation in hull transformation

pyomo/gdp/plugins/hull.py

@@ -20,7 +20,7 @@
 from pyomo.common.modeling import unique_component_name
 from pyomo.core.expr.numvalue import ZeroConstant
 import pyomo.core.expr as EXPR
-from pyomo.core.base import TransformationFactory, Reference
+from pyomo.core.base import TransformationFactory
 from pyomo.core import (
     Block,
     BooleanVar,
@@ -447,19 +447,11 @@ def _transform_disjunctionData(
                     disaggregatedVar=disaggregated_var,
                     disjunct=obj,
                     bigmConstraint=disaggregated_var_bounds,
-                    lb_idx=(idx, 'lb'),
-                    ub_idx=(idx, 'ub'),
                     var_free_indicator=var_free,
+                    var_idx=idx,
                 )
-                # Update mappings:
-                var_info = var.parent_block().private_data()
-                disaggregated_var_map = var_info.disaggregated_var_map
-                dis_var_info = disaggregated_var.parent_block().private_data()
-
-                dis_var_info.bigm_constraint_map[disaggregated_var][obj] = Reference(
-                    disaggregated_var_bounds[idx, :]
-                )
-                dis_var_info.original_var_map[disaggregated_var] = var
+                original_var_info = var.parent_block().private_data()
+                disaggregated_var_map = original_var_info.disaggregated_var_map
 
                 # For every Disjunct the Var does not appear in, we want to map
                 # that this new variable is its disaggreggated variable.
@@ -544,8 +536,6 @@ def _transform_disjunct(
                 disaggregatedVar=disaggregatedVar,
                 disjunct=obj,
                 bigmConstraint=bigmConstraint,
-                lb_idx='lb',
-                ub_idx='ub',
                 var_free_indicator=obj.indicator_var.get_associated_binary(),
             )
             # update the bigm constraint mappings
@@ -573,8 +563,6 @@ def _transform_disjunct(
                 disaggregatedVar=var,
                 disjunct=obj,
                 bigmConstraint=bigmConstraint,
-                lb_idx='lb',
-                ub_idx='ub',
                 var_free_indicator=obj.indicator_var.get_associated_binary(),
             )
             # update the bigm constraint mappings
@@ -607,10 +595,16 @@ def _declare_disaggregated_var_bounds(
         disaggregatedVar,
         disjunct,
         bigmConstraint,
-        lb_idx,
-        ub_idx,
         var_free_indicator,
+        var_idx=None,
     ):
+        # For updating mappings:
+        original_var_info = original_var.parent_block().private_data()
+        disaggregated_var_map = original_var_info.disaggregated_var_map
+        disaggregated_var_info = disaggregatedVar.parent_block().private_data()
+
+        disaggregated_var_info.bigm_constraint_map[disaggregatedVar][disjunct] = {}
+
         lb = original_var.lb
         ub = original_var.ub
         if lb is None or ub is None:
@@ -624,13 +618,21 @@ def _declare_disaggregated_var_bounds(
         disaggregatedVar.setub(max(0, ub))
 
         if lb:
+            lb_idx = 'lb'
+            if var_idx is not None:
+                lb_idx = (var_idx, 'lb')
             bigmConstraint.add(lb_idx, var_free_indicator * lb <= disaggregatedVar)
+            disaggregated_var_info.bigm_constraint_map[disaggregatedVar][disjunct][
+                'lb'
+            ] = bigmConstraint[lb_idx]
         if ub:
+            ub_idx = 'ub'
+            if var_idx is not None:
+                ub_idx = (var_idx, 'ub')
             bigmConstraint.add(ub_idx, disaggregatedVar <= ub * var_free_indicator)
-
-        original_var_info = original_var.parent_block().private_data()
-        disaggregated_var_map = original_var_info.disaggregated_var_map
-        disaggregated_var_info = disaggregatedVar.parent_block().private_data()
+            disaggregated_var_info.bigm_constraint_map[disaggregatedVar][disjunct][
+                'ub'
+            ] = bigmConstraint[ub_idx]
 
         # store the mappings from variables to their disaggregated selves on
         # the transformation block
@@ -928,10 +930,9 @@ def get_disaggregation_constraint(
 
     def get_var_bounds_constraint(self, v, disjunct=None):
         """
-        Returns the IndexedConstraint which sets a disaggregated
-        variable to be within its bounds when its Disjunct is active and to
-        be 0 otherwise. (It is always an IndexedConstraint because each
-        bound becomes a separate constraint.)
+        Returns a dictionary mapping keys 'lb' and/or 'ub' to the Constraints that
+        set a disaggregated variable to be within its lower and upper bounds
+        (respectively) when its Disjunct is active and to be 0 otherwise.
 
         Parameters
         ----------

pyomo/gdp/tests/test_hull.py

@@ -32,7 +32,6 @@
     Param,
     Objective,
     TerminationCondition,
-    Reference,
 )
 from pyomo.core.expr.compare import (
     assertExpressionsEqual,
@@ -530,21 +529,27 @@ def test_bigMConstraint_mappings(self):
             mappings[disjBlock[i].disaggregatedVars.x] = disjBlock[i].x_bounds
             if i == 1:  # this disjunct has x, w, and no y
                 mappings[disjBlock[i].disaggregatedVars.w] = disjBlock[i].w_bounds
-                mappings[transBlock._disaggregatedVars[0]] = Reference(
-                    transBlock._boundsConstraints[0, ...]
-                )
+                mappings[transBlock._disaggregatedVars[0]] = {
+                    key: val
+                    for key, val in transBlock._boundsConstraints.items()
+                    if key[0] == 0
+                }
             elif i == 0:  # this disjunct has x, y, and no w
                 mappings[disjBlock[i].disaggregatedVars.y] = disjBlock[i].y_bounds
-                mappings[transBlock._disaggregatedVars[1]] = Reference(
-                    transBlock._boundsConstraints[1, ...]
-                )
+                mappings[transBlock._disaggregatedVars[1]] = {
+                    key: val
+                    for key, val in transBlock._boundsConstraints.items()
+                    if key[0] == 1
+                }
             for var, cons in mappings.items():
                 returned_cons = hull.get_var_bounds_constraint(var)
                 # This sometimes refers a reference to the right part of a
                 # larger indexed constraint, so the indexed constraints
                 # themselves might not be the same object. The ConstraintDatas
                 # are though:
                 for key, constraintData in cons.items():
+                    if type(key) is tuple:
+                        key = key[1]
                     self.assertIs(returned_cons[key], constraintData)
 
     def test_create_using_nonlinear(self):