🔨 Use Suffixes instead of _GeneralVarData

- Declare `priority` and `direction` suffixes on the model instead of attributes on the variable data in order to represent branching decisions

pyomo/core/base/var.py

@@ -11,7 +11,6 @@
 __all__ = ['Var', '_VarData', '_GeneralVarData', 'VarList', 'SimpleVar']
 
 import logging
-from typing import Optional
 from weakref import ref as weakref_ref
 
 from pyomo.common.modeling import NoArgumentGiven
@@ -279,10 +278,6 @@ def to_string(self, verbose=None, labeler=None, smap=None, compute_values=False)
         return self.name
 
 
-PriorityType = int
-BranchDirectionType = int
-
-
 class _GeneralVarData(_VarData):
     """
     This class defines the data for a single variable.
@@ -313,7 +308,7 @@ class _GeneralVarData(_VarData):
     these attributes in certain cases.
     """
 
-    __slots__ = ('_value', '_lb', '_ub', '_domain', 'fixed', 'stale', 'branch_priority', 'branch_direction')
+    __slots__ = ('_value', '_lb', '_ub', '_domain', 'fixed', 'stale')
 
     def __init__(self, domain=Reals, component=None):
         #
@@ -346,9 +341,6 @@ def __init__(self, domain=Reals, component=None):
                 "for bounds (like a Pyomo Set). Examples: NonNegativeReals, "
                 "Integers, Binary" % (domain, (RealSet, IntegerSet, BooleanSet)))
 
-        self.branch_priority = None  # type: Optional[PriorityType]
-        self.branch_direction = None  # type: Optional[BranchDirectionType]
-
     def __getstate__(self):
         state = super(_GeneralVarData, self).__getstate__()
         for i in _GeneralVarData.__slots__:

pyomo/core/tests/unit/test_var.py

@@ -440,24 +440,6 @@ def test_value(self):
         self.assertEqual( type(tmp), int)
         self.assertEqual( tmp, 3 )
 
-    def test_branch_priority_attr(self):
-        """Test branch_priority attribute"""
-        self.model.x = Var()
-        self.instance = self.model.create_instance()
-
-        self.assertIsNone(self.instance.x.branch_priority)
-        self.instance.x.branch_priority = 1
-        self.assertEqual(self.instance.x.branch_priority, 1)
-
-    def test_branch_direction_attr(self):
-        """Test branch_direction attribute"""
-        self.model.x = Var()
-        self.instance = self.model.create_instance()
-
-        self.assertIsNone(self.instance.x.branch_direction)
-        self.instance.x.branch_direction = -1
-        self.assertEqual(self.instance.x.branch_direction, -1)
-
 
 class TestArrayVar(TestSimpleVar):
 

pyomo/solvers/plugins/solvers/CPLEX.py

@@ -18,6 +18,7 @@
 import pyutilib.common
 import pyutilib.misc
 
+from pyomo.core import ComponentMap, Suffix, active_export_suffix_generator
 from pyomo.opt.base import *
 from pyomo.opt.base.solvers import _extract_version
 from pyomo.opt.results import *
@@ -122,10 +123,9 @@ class CPLEXBranchDirection:
     @staticmethod
     def to_str(branch_direction):
         try:
-            return {
-                CPLEXBranchDirection.down: "DN",
-                CPLEXBranchDirection.up: "UP",
-            }[branch_direction]
+            return {CPLEXBranchDirection.down: "DN", CPLEXBranchDirection.up: "UP"}[
+                branch_direction
+            ]
         except KeyError:
             return ""
 
@@ -136,7 +136,11 @@ class ORDFileSchema:
 
     @classmethod
     def ROW(cls, name, priority, branch_direction=None):
-        return " %s %s %s\n" % (CPLEXBranchDirection.to_str(branch_direction), name, priority)
+        return " %s %s %s\n" % (
+            CPLEXBranchDirection.to_str(branch_direction),
+            name,
+            priority,
+        )
 
 
 @SolverFactory.register('_cplex_shell', doc='Shell interface to the CPLEX LP/MIP solver')
@@ -228,20 +232,37 @@ def _warm_start(self, instance):
             mst_file.write("</variables>\n")
             mst_file.write("</CPLEXSolution>\n")
 
+    # Expected names of `Suffix` components for branching priorities and directions respectively
+    SUFFIX_PRIORITY_NAME = "priority"
+    SUFFIX_DIRECTION_NAME = "direction"
+
     def _write_priorities_file(self, instance) -> None:
         """ Write a variable priorities file in the CPLEX ORD format. """
         from pyomo.core.base import Var
 
         if isinstance(instance, IBlock):
             smap = getattr(instance, "._symbol_maps")[self._smap_id]
+            suffixes = pyomo.core.kernel.suffix.export_suffix_generator(
+                instance, datatype=Suffix.INT, active=True, descend_into=False
+            )
         else:
             smap = instance.solutions.symbol_map[self._smap_id]
+            suffixes = active_export_suffix_generator(instance, datatype=Suffix.INT)
         byObject = smap.byObject
+        suffixes = dict(suffixes)
+
+        if self.SUFFIX_PRIORITY_NAME not in suffixes:
+            raise ValueError(
+                "Cannot write branching priorities file as `model.%s` Suffix has not been declared."
+                % (self.SUFFIX_PRIORITY_NAME,)
+            )
+
+        priorities = suffixes[self.SUFFIX_PRIORITY_NAME]
+        directions = suffixes.get(self.SUFFIX_DIRECTION_NAME, ComponentMap())
 
         with open(self._priorities_file_name, "w") as ord_file:
             ord_file.write(ORDFileSchema.HEADER)
-            for var in instance.component_data_objects(Var):
-                priority = var.branch_priority
+            for var, priority in priorities.items():
                 if priority is None:
                     continue
 
@@ -252,7 +273,7 @@ def _write_priorities_file(self, instance) -> None:
                     continue
 
                 ord_file.write(
-                    ORDFileSchema.ROW(byObject[id(var)], priority, var.branch_direction)
+                    ORDFileSchema.ROW(byObject[id(var)], priority, directions.get(var))
                 )
 
             ord_file.write(ORDFileSchema.FOOTER)

pyomo/solvers/tests/checks/test_cplex.py

@@ -14,7 +14,7 @@
 import pyutilib
 import pyutilib.th as unittest
 
-from pyomo.core import Binary, ConcreteModel, Constraint, Objective, Var, Integers, RangeSet, minimize, quicksum
+from pyomo.core import Binary, ConcreteModel, Constraint, Objective, Var, Integers, RangeSet, minimize, quicksum, Suffix
 from pyomo.opt import ProblemFormat, convert_problem, SolverFactory
 from pyomo.solvers.plugins.solvers.CPLEX import (CPLEXSHELL, MockCPLEX,
                                                  _validate_file_name)
@@ -98,19 +98,13 @@ def get_priorities_file_as_string(self, mock_cplex_shell):
             priorities_file = ord_file.read()
         return priorities_file
 
-    def test_write_empty_priorities_file(self):
-        CPLEXSHELL._write_priorities_file(self.mock_cplex_shell, self.mock_model)
-
-        with open(self.mock_cplex_shell._priorities_file_name, "r") as ord_file:
-            priorities_file = ord_file.read()
-
-        self.assertEqual(
-            priorities_file,
-            "* ENCODING=ISO-8859-1\nNAME             Priority Order\nENDATA\n",
-        )
+    def test_write_without_priority_suffix(self):
+        with self.assertRaises(ValueError):
+            CPLEXSHELL._write_priorities_file(self.mock_cplex_shell, self.mock_model)
 
     def test_write_priority_to_priorities_file(self):
-        self.mock_model.x.branch_priority = 10
+        self.mock_model.priority = Suffix(direction=Suffix.EXPORT, datatype=Suffix.INT)
+        self.mock_model.priority.set_value(self.mock_model.x, 10)
 
         CPLEXSHELL._write_priorities_file(self.mock_cplex_shell, self.mock_model)
         priorities_file = self.get_priorities_file_as_string(self.mock_cplex_shell)
@@ -119,11 +113,13 @@ def test_write_priority_to_priorities_file(self):
             priorities_file,
             "* ENCODING=ISO-8859-1\nNAME             Priority Order\n  x1 10\nENDATA\n",
         )
-        self.assertIsNone(self.mock_model.x.branch_direction)
 
     def test_write_priority_and_direction_to_priorities_file(self):
-        self.mock_model.x.branch_priority = 10
-        self.mock_model.x.branch_direction = -1
+        self.mock_model.priority = Suffix(direction=Suffix.EXPORT, datatype=Suffix.INT)
+        self.mock_model.priority.set_value(self.mock_model.x, 10)
+
+        self.mock_model.direction = Suffix(direction=Suffix.EXPORT, datatype=Suffix.INT)
+        self.mock_model.direction.set_value(self.mock_model.x, -1)
 
         CPLEXSHELL._write_priorities_file(self.mock_cplex_shell, self.mock_model)
         priorities_file = self.get_priorities_file_as_string(self.mock_cplex_shell)
@@ -134,17 +130,21 @@ def test_write_priority_and_direction_to_priorities_file(self):
         )
 
     def test_raise_due_to_invalid_priority(self):
-        self.mock_model.x.branch_priority = -1
+        self.mock_model.priority = Suffix(direction=Suffix.EXPORT, datatype=Suffix.INT)
+        self.mock_model.priority.set_value(self.mock_model.x, -1)
         with self.assertRaises(ValueError):
             CPLEXSHELL._write_priorities_file(self.mock_cplex_shell, self.mock_model)
 
-        self.mock_model.x.branch_priority = 1.1
+        self.mock_model.priority.set_value(self.mock_model.x, 1.1)
         with self.assertRaises(ValueError):
             CPLEXSHELL._write_priorities_file(self.mock_cplex_shell, self.mock_model)
 
     def test_use_default_due_to_invalid_direction(self):
-        self.mock_model.x.branch_priority = 10
-        self.mock_model.x.branch_direction = "invalid_branching_direction"
+        self.mock_model.priority = Suffix(direction=Suffix.EXPORT, datatype=Suffix.INT)
+        self.mock_model.priority.set_value(self.mock_model.x, 10)
+
+        self.mock_model.direction = Suffix(direction=Suffix.EXPORT, datatype=Suffix.INT)
+        self.mock_model.direction.set_value(self.mock_model.x, "invalid_branching_direction")
 
         CPLEXSHELL._write_priorities_file(self.mock_cplex_shell, self.mock_model)
         priorities_file = self.get_priorities_file_as_string(self.mock_cplex_shell)
@@ -169,13 +169,16 @@ def get_mock_model_with_priorities(self):
         m.c = Constraint(expr=m.x >= 1)
         m.c2 = Constraint(expr=quicksum(m.y[i] for i in m.s) >= 10)
 
-        m.x.branch_priority = 1
+        m.priority = Suffix(direction=Suffix.EXPORT, datatype=Suffix.INT)
+        m.direction = Suffix(direction=Suffix.EXPORT, datatype=Suffix.INT)
+
+        m.priority.set_value(m.x, 1)
 
-        for var in m.y.values():
-            var.branch_priority = 2
-            var.branch_direction = -1
+        # Ensure tests work for both options of `expand`
+        m.priority.set_value(m.y, 2, expand=False)
+        m.direction.set_value(m.y, -1, expand=True)
 
-        m.y[10].branch_direction = 1
+        m.direction.set_value(m.y[10], 1)
         return m
 
     def test_use_variable_priorities(self):