From 5e44956fe377a514ebebe35cc080071fcb197cea Mon Sep 17 00:00:00 2001 From: Ruaridh Williamson Date: Thu, 20 Feb 2020 15:24:03 +0000 Subject: [PATCH] :hammer: Use `Suffix`es 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 | 10 +---- pyomo/core/tests/unit/test_var.py | 18 --------- pyomo/solvers/plugins/solvers/CPLEX.py | 37 +++++++++++++---- pyomo/solvers/tests/checks/test_cplex.py | 51 +++++++++++++----------- 4 files changed, 57 insertions(+), 59 deletions(-) diff --git a/pyomo/core/base/var.py b/pyomo/core/base/var.py index b372d7c1183..82e5c788c20 100644 --- a/pyomo/core/base/var.py +++ b/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__: diff --git a/pyomo/core/tests/unit/test_var.py b/pyomo/core/tests/unit/test_var.py index fe814428867..5faf0c44ef8 100644 --- a/pyomo/core/tests/unit/test_var.py +++ b/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): diff --git a/pyomo/solvers/plugins/solvers/CPLEX.py b/pyomo/solvers/plugins/solvers/CPLEX.py index 015e0938449..04184accc80 100644 --- a/pyomo/solvers/plugins/solvers/CPLEX.py +++ b/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("\n") mst_file.write("\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) diff --git a/pyomo/solvers/tests/checks/test_cplex.py b/pyomo/solvers/tests/checks/test_cplex.py index f3e5106072c..9ab614c8a42 100644 --- a/pyomo/solvers/tests/checks/test_cplex.py +++ b/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):