From 49039a75f36ebce27ada4faf7cec4c9efdc48c34 Mon Sep 17 00:00:00 2001 From: Ruaridh Williamson Date: Tue, 28 Apr 2020 20:18:52 +0100 Subject: [PATCH 01/14] :zap: Initialise `CplexExpr` with full list - This is more efficient than always calling `.append()` on an empty list --- pyomo/solvers/plugins/solvers/cplex_direct.py | 60 ++++++++++++------- 1 file changed, 38 insertions(+), 22 deletions(-) diff --git a/pyomo/solvers/plugins/solvers/cplex_direct.py b/pyomo/solvers/plugins/solvers/cplex_direct.py index 990f8f3590c..5815017bb63 100644 --- a/pyomo/solvers/plugins/solvers/cplex_direct.py +++ b/pyomo/solvers/plugins/solvers/cplex_direct.py @@ -37,13 +37,22 @@ class DegreeError(ValueError): class _CplexExpr(object): - def __init__(self): - self.variables = [] - self.coefficients = [] - self.offset = 0 - self.q_variables1 = [] - self.q_variables2 = [] - self.q_coefficients = [] + def __init__( + self, + variables, + coefficients, + offset=None, + q_variables1=None, + q_variables2=None, + q_coefficients=None, + ): + self.variables = variables + self.coefficients = coefficients + self.offset = offset or 0. + self.q_variables1 = q_variables1 or [] + self.q_variables2 = q_variables2 or [] + self.q_coefficients = q_coefficients or [] + def _is_numeric(x): try: @@ -193,29 +202,36 @@ def _process_stream(arg): return Bunch(rc=None, log=None) def _get_expr_from_pyomo_repn(self, repn, max_degree=2): - referenced_vars = ComponentSet() - degree = repn.polynomial_degree() - if (degree is None) or (degree > max_degree): - raise DegreeError('CPLEXDirect does not support expressions of degree {0}.'.format(degree)) + if degree is None or degree > max_degree: + raise DegreeError( + "CPLEXDirect does not support expressions of degree {0}.".format(degree) + ) - new_expr = _CplexExpr() - if len(repn.linear_vars) > 0: - referenced_vars.update(repn.linear_vars) - new_expr.variables.extend(self._pyomo_var_to_ndx_map[i] for i in repn.linear_vars) - new_expr.coefficients.extend(repn.linear_coefs) + referenced_vars = ComponentSet(repn.linear_vars) + q_coefficients = [] + q_variables1 = [] + q_variables2 = [] for i, v in enumerate(repn.quadratic_vars): x, y = v - new_expr.q_coefficients.append(repn.quadratic_coefs[i]) - new_expr.q_variables1.append(self._pyomo_var_to_ndx_map[x]) - new_expr.q_variables2.append(self._pyomo_var_to_ndx_map[y]) + q_coefficients.append(repn.quadratic_coefs[i]) + q_variables1.append(self._pyomo_var_to_ndx_map[x]) + q_variables2.append(self._pyomo_var_to_ndx_map[y]) referenced_vars.add(x) referenced_vars.add(y) - new_expr.offset = repn.constant - - return new_expr, referenced_vars + return ( + _CplexExpr( + variables=[self._pyomo_var_to_ndx_map[var] for var in repn.linear_vars], + coefficients=repn.linear_coefs, + offset=repn.constant, + q_variables1=q_variables1, + q_variables2=q_variables2, + q_coefficients=q_coefficients, + ), + referenced_vars, + ) def _get_expr_from_pyomo_expr(self, expr, max_degree=2): if max_degree == 2: From b225180fdc2aa57e40f5fca442dc34e149dfa511 Mon Sep 17 00:00:00 2001 From: Ruaridh Williamson Date: Tue, 28 Apr 2020 20:19:57 +0100 Subject: [PATCH 02/14] :hammer: No need to set an empty quadratic coef --- pyomo/solvers/plugins/solvers/cplex_direct.py | 1 - 1 file changed, 1 deletion(-) diff --git a/pyomo/solvers/plugins/solvers/cplex_direct.py b/pyomo/solvers/plugins/solvers/cplex_direct.py index 5815017bb63..0cb2b50c650 100644 --- a/pyomo/solvers/plugins/solvers/cplex_direct.py +++ b/pyomo/solvers/plugins/solvers/cplex_direct.py @@ -442,7 +442,6 @@ def _set_objective(self, obj): self._objective = None self._solver_model.objective.set_linear([(i, 0.0) for i in range(len(self._pyomo_var_to_solver_var_map.values()))]) - self._solver_model.objective.set_quadratic([[[0], [0]] for i in self._pyomo_var_to_solver_var_map.keys()]) if obj.active is False: raise ValueError('Cannot add inactive objective to solver.') From c5e5841099c9d91948d0bad4f32bc052f9273448 Mon Sep 17 00:00:00 2001 From: Ruaridh Williamson Date: Tue, 28 Apr 2020 20:21:03 +0100 Subject: [PATCH 03/14] :zap: `get_values()` efficiently Asking the `cplex` solution for *specific* variables' values is extremely slow due to the conversion on `cplex`'s end to lookup the variables you've asked for. It is orders of magnitude faster to get the full solution vector. Even worse, using the "specific variable interface" to get the full solution vector. If we must get specific variables (ie. `_load_vars()`) then their index should be used instead of their name. --- pyomo/solvers/plugins/solvers/cplex_direct.py | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/pyomo/solvers/plugins/solvers/cplex_direct.py b/pyomo/solvers/plugins/solvers/cplex_direct.py index 0cb2b50c650..30f7de915ae 100644 --- a/pyomo/solvers/plugins/solvers/cplex_direct.py +++ b/pyomo/solvers/plugins/solvers/cplex_direct.py @@ -602,13 +602,13 @@ def _postsolve(self): soln_constraints = soln.constraint var_names = self._solver_model.variables.get_names() - var_names = list(set(var_names).intersection(set(self._pyomo_var_to_solver_var_map.values()))) - var_vals = self._solver_model.solution.get_values(var_names) - for i, name in enumerate(var_names): + assert set(var_names) == set(self._pyomo_var_to_solver_var_map.values()) + var_vals = self._solver_model.solution.get_values() + for name, val in zip(var_names, var_vals): pyomo_var = self._solver_var_to_pyomo_var_map[name] if self._referenced_variables[pyomo_var] > 0: pyomo_var.stale = False - soln_variables[name] = {"Value":var_vals[i]} + soln_variables[name] = {"Value": val} if extract_reduced_costs: reduced_costs = self._solver_model.solution.get_reduced_costs(var_names) @@ -696,18 +696,18 @@ def _warm_start(self): self._solver_model.MIP_starts.effort_level.auto) def _load_vars(self, vars_to_load=None): - var_map = self._pyomo_var_to_solver_var_map - ref_vars = self._referenced_variables + var_map = self._pyomo_var_to_ndx_map if vars_to_load is None: + vals = self._solver_model.solution.get_values() vars_to_load = var_map.keys() + else: + cplex_vars_to_load = [var_map[pyomo_var] for pyomo_var in vars_to_load] + vals = self._solver_model.solution.get_values(cplex_vars_to_load) - cplex_vars_to_load = [var_map[pyomo_var] for pyomo_var in vars_to_load] - vals = self._solver_model.solution.get_values(cplex_vars_to_load) - - for i, pyomo_var in enumerate(vars_to_load): - if ref_vars[pyomo_var] > 0: + for pyomo_var, val in zip(vars_to_load, vals): + if self._referenced_variables[pyomo_var] > 0: pyomo_var.stale = False - pyomo_var.value = vals[i] + pyomo_var.value = val def _load_rc(self, vars_to_load=None): if not hasattr(self._pyomo_model, 'rc'): From 622c4d07dc1a43332340ae5da9a12e65f39defa3 Mon Sep 17 00:00:00 2001 From: Ruaridh Williamson Date: Tue, 28 Apr 2020 20:23:02 +0100 Subject: [PATCH 04/14] :zap: Add linear constraints and variables in one transaction The `cplex` package's Linear Constraints and Variable interfaces allow for batched transactions. I think an appropriate design is to generate all the necessary data and add these objects as one call to the `solver_model`. I've also removed unnecessary transactions such as resetting variable bounds immediately after adding that variable with an obsolete bound. --- pyomo/solvers/plugins/solvers/cplex_direct.py | 182 +++++++++++++++--- 1 file changed, 151 insertions(+), 31 deletions(-) diff --git a/pyomo/solvers/plugins/solvers/cplex_direct.py b/pyomo/solvers/plugins/solvers/cplex_direct.py index 30f7de915ae..269b23c0899 100644 --- a/pyomo/solvers/plugins/solvers/cplex_direct.py +++ b/pyomo/solvers/plugins/solvers/cplex_direct.py @@ -62,6 +62,77 @@ def _is_numeric(x): return True +class _VariableData(object): + def __init__(self, solver_model): + self._solver_model = solver_model + self.lb = [] + self.ub = [] + self.types = [] + self.names = [] + + def add(self, lb, ub, type_, name): + self.lb.append(lb) + self.ub.append(ub) + self.types.append(type_) + self.names.append(name) + + def __enter__(self): + return self + + def __exit__(self, *excinfo): + self._solver_model.variables.add( + lb=self.lb, ub=self.ub, types=self.types, names=self.names + ) + + +class _LinearConstraintData(object): + def __init__(self, solver_model): + self._solver_model = solver_model + self.lin_expr = [] + self.senses = [] + self.rhs = [] + self.range_values = [] + self.names = [] + + def add(self, cplex_expr, sense, rhs, range_values, name): + self.lin_expr.append([cplex_expr.variables, cplex_expr.coefficients]) + self.senses.append(sense) + self.rhs.append(rhs) + self.range_values.append(range_values) + self.names.append(name) + + def __enter__(self): + return self + + def __exit__(self, *excinfo): + self._solver_model.linear_constraints.add( + lin_expr=self.lin_expr, + senses=self.senses, + rhs=self.rhs, + range_values=self.range_values, + names=self.names, + ) + + +class nullcontext(object): + """Context manager that does no additional processing. + Used as a stand-in for a normal context manager, when a particular + block of code is only sometimes used with a normal context manager: + cm = optional_cm if condition else nullcontext() + with cm: + # Perform operation, using optional_cm if condition is True + """ + + def __init__(self, enter_result=None): + self.enter_result = enter_result + + def __enter__(self): + return self.enter_result + + def __exit__(self, *excinfo): + pass + + @SolverFactory.register('cplex_direct', doc='Direct python interface to CPLEX') class CPLEXDirect(DirectSolver): @@ -248,7 +319,7 @@ def _get_expr_from_pyomo_expr(self, expr, max_degree=2): return cplex_expr, referenced_vars - def _add_var(self, var): + def _add_var(self, var, cplex_var_data=None): varname = self._symbol_map.getSymbol(var, self._labeler) vtype = self._cplex_vtype_from_var(var) if var.has_lb(): @@ -260,7 +331,14 @@ def _add_var(self, var): else: ub = self._cplex.infinity - self._solver_model.variables.add(lb=[lb], ub=[ub], types=[vtype], names=[varname]) + + ctx = ( + _VariableData(self._solver_model) + if cplex_var_data is None + else nullcontext(cplex_var_data) + ) + with ctx as cplex_var_data: + cplex_var_data.add(lb=lb, ub=ub, type_=vtype, name=varname) self._pyomo_var_to_solver_var_map[var] = varname self._solver_var_to_pyomo_var_map[varname] = var @@ -303,7 +381,49 @@ def _set_instance(self, model, kwds={}): "by overwriting its bounds in the CPLEX instance." % (var.name, self._pyomo_model.name,)) - def _add_constraint(self, con): + def _add_block(self, block): + with _VariableData(self._solver_model) as cplex_var_data: + for var in block.component_data_objects( + ctype=pyomo.core.base.var.Var, descend_into=True, active=True, sort=True + ): + self._add_var(var, cplex_var_data) + + with _LinearConstraintData(self._solver_model) as cplex_lin_con_data: + for sub_block in block.block_data_objects(descend_into=True, active=True): + for con in sub_block.component_data_objects( + ctype=pyomo.core.base.constraint.Constraint, + descend_into=False, + active=True, + sort=True, + ): + if not con.has_lb() and not con.has_ub(): + assert not con.equality + continue # non-binding, so skip + + self._add_constraint(con, cplex_lin_con_data) + + for con in sub_block.component_data_objects( + ctype=pyomo.core.base.sos.SOSConstraint, + descend_into=False, + active=True, + sort=True, + ): + self._add_sos_constraint(con) + + obj_counter = 0 + for obj in sub_block.component_data_objects( + ctype=pyomo.core.base.objective.Objective, + descend_into=False, + active=True, + ): + obj_counter += 1 + if obj_counter > 1: + raise ValueError( + "Solver interface does not support multiple objectives." + ) + self._set_objective(obj) + + def _add_constraint(self, con, cplex_lin_con_data=None): if not con.active: return None @@ -314,12 +434,12 @@ def _add_constraint(self, con): if con._linear_canonical_form: cplex_expr, referenced_vars = self._get_expr_from_pyomo_repn( - con.canonical_form(), - self._max_constraint_degree) + con.canonical_form(), self._max_constraint_degree + ) else: cplex_expr, referenced_vars = self._get_expr_from_pyomo_expr( - con.body, - self._max_constraint_degree) + con.body, self._max_constraint_degree + ) if con.has_lb(): if not is_fixed(con.lower): @@ -330,39 +450,39 @@ def _add_constraint(self, con): raise ValueError("Upper bound of constraint {0} " "is not constant.".format(con)) + range_ = 0.0 if con.equality: - my_sense = 'E' - my_rhs = [value(con.lower) - cplex_expr.offset] - my_range = [] + sense = "E" + rhs = value(con.lower) - cplex_expr.offset elif con.has_lb() and con.has_ub(): - my_sense = 'R' + sense = "R" lb = value(con.lower) ub = value(con.upper) - my_rhs = [ub - cplex_expr.offset] - my_range = [lb - ub] + rhs = ub - cplex_expr.offset + range_ = lb - ub self._range_constraints.add(con) elif con.has_lb(): - my_sense = 'G' - my_rhs = [value(con.lower) - cplex_expr.offset] - my_range = [] + sense = "G" + rhs = value(con.lower) - cplex_expr.offset elif con.has_ub(): - my_sense = 'L' - my_rhs = [value(con.upper) - cplex_expr.offset] - my_range = [] + sense = "L" + rhs = value(con.upper) - cplex_expr.offset else: - raise ValueError("Constraint does not have a lower " - "or an upper bound: {0} \n".format(con)) + raise ValueError( + "Constraint does not have a lower " + "or an upper bound: {0} \n".format(con) + ) if len(cplex_expr.q_coefficients) == 0: - self._solver_model.linear_constraints.add( - lin_expr=[[cplex_expr.variables, - cplex_expr.coefficients]], - senses=my_sense, - rhs=my_rhs, - range_values=my_range, - names=[conname]) + ctx = ( + _LinearConstraintData(self._solver_model) + if cplex_lin_con_data is None + else nullcontext(cplex_lin_con_data) + ) + with ctx as cplex_lin_con_data: + cplex_lin_con_data.add(cplex_expr, sense, rhs, range_, conname) else: - if my_sense == 'R': + if sense == 'R': raise ValueError("The CPLEXDirect interface does not " "support quadratic range constraints: " "{0}".format(con)) @@ -372,8 +492,8 @@ def _add_constraint(self, con): quad_expr=[cplex_expr.q_variables1, cplex_expr.q_variables2, cplex_expr.q_coefficients], - sense=my_sense, - rhs=my_rhs[0], + sense=sense, + rhs=rhs, name=conname) for var in referenced_vars: From f61fe426ef159fcd4d6e1bfbd34f17bf00a920d8 Mon Sep 17 00:00:00 2001 From: Ruaridh Williamson Date: Tue, 28 Apr 2020 20:23:42 +0100 Subject: [PATCH 05/14] :zap: Avoid calling `set_bounds()` when not necessary --- pyomo/solvers/plugins/solvers/cplex_direct.py | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/pyomo/solvers/plugins/solvers/cplex_direct.py b/pyomo/solvers/plugins/solvers/cplex_direct.py index 269b23c0899..f9200204903 100644 --- a/pyomo/solvers/plugins/solvers/cplex_direct.py +++ b/pyomo/solvers/plugins/solvers/cplex_direct.py @@ -331,6 +331,9 @@ def _add_var(self, var, cplex_var_data=None): else: ub = self._cplex.infinity + if var.is_fixed(): + lb = value(var) + ub = value(var) ctx = ( _VariableData(self._solver_model) @@ -346,10 +349,6 @@ def _add_var(self, var, cplex_var_data=None): self._ndx_count += 1 self._referenced_variables[var] = 0 - if var.is_fixed(): - self._solver_model.variables.set_lower_bounds(varname, var.value) - self._solver_model.variables.set_upper_bounds(varname, var.value) - def _set_instance(self, model, kwds={}): self._pyomo_var_to_ndx_map = ComponentMap() self._ndx_count = 0 @@ -441,14 +440,15 @@ def _add_constraint(self, con, cplex_lin_con_data=None): con.body, self._max_constraint_degree ) - if con.has_lb(): - if not is_fixed(con.lower): - raise ValueError("Lower bound of constraint {0} " - "is not constant.".format(con)) - if con.has_ub(): - if not is_fixed(con.upper): - raise ValueError("Upper bound of constraint {0} " - "is not constant.".format(con)) + if con.has_lb() and not is_fixed(con.lower): + raise ValueError( + "Lower bound of constraint {0} is not constant.".format(con) + ) + + if con.has_ub() and not is_fixed(con.upper): + raise ValueError( + "Upper bound of constraint {0} is not constant.".format(con) + ) range_ = 0.0 if con.equality: From 7ff9742f57cc23cc424cc073fdd383a84580b3c9 Mon Sep 17 00:00:00 2001 From: Ruaridh Williamson Date: Wed, 6 May 2020 11:50:39 +0100 Subject: [PATCH 06/14] :rotating_light: Test `nullcontext()` --- pyomo/solvers/plugins/solvers/cplex_direct.py | 2 ++ pyomo/solvers/tests/checks/test_CPLEXDirect.py | 14 ++++++++++++++ 2 files changed, 16 insertions(+) diff --git a/pyomo/solvers/plugins/solvers/cplex_direct.py b/pyomo/solvers/plugins/solvers/cplex_direct.py index f9200204903..f522a05110b 100644 --- a/pyomo/solvers/plugins/solvers/cplex_direct.py +++ b/pyomo/solvers/plugins/solvers/cplex_direct.py @@ -114,6 +114,8 @@ def __exit__(self, *excinfo): ) +# `nullcontext()` is part of the standard library as of Py3.7 +# This is verbatim from `cpython/Lib/contextlib.py` class nullcontext(object): """Context manager that does no additional processing. Used as a stand-in for a normal context manager, when a particular diff --git a/pyomo/solvers/tests/checks/test_CPLEXDirect.py b/pyomo/solvers/tests/checks/test_CPLEXDirect.py index 4e88405f4d4..c78f5931659 100644 --- a/pyomo/solvers/tests/checks/test_CPLEXDirect.py +++ b/pyomo/solvers/tests/checks/test_CPLEXDirect.py @@ -13,6 +13,8 @@ from pyomo.environ import * import sys +from pyomo.solvers.plugins.solvers.cplex_direct import nullcontext + try: import cplex cplexpy_available = True @@ -227,5 +229,17 @@ def test_dont_skip_trivial_and_call_count_for_unfixed_con_is_one(self): self.assertEqual(mock_is_fixed.call_count, 1) +# `nullcontext()` is part of the standard library as of Py3.7 +# This is verbatim from `cpython/Lib/test/test_contextlib.py` +class NullcontextTestCase(unittest.TestCase): + def test_nullcontext(self): + class C: + pass + + c = C() + with nullcontext(c) as c_in: + self.assertIs(c_in, c) + + if __name__ == "__main__": unittest.main() From 4cd2c5adae8d563d36c8081d8a9994ead46278ce Mon Sep 17 00:00:00 2001 From: Ruaridh Williamson Date: Wed, 6 May 2020 12:09:41 +0100 Subject: [PATCH 07/14] :rotating_light: Test CPLEX Data Containers --- .../solvers/tests/checks/test_CPLEXDirect.py | 71 ++++++++++++++++++- 1 file changed, 70 insertions(+), 1 deletion(-) diff --git a/pyomo/solvers/tests/checks/test_CPLEXDirect.py b/pyomo/solvers/tests/checks/test_CPLEXDirect.py index c78f5931659..2d9dcf063b3 100644 --- a/pyomo/solvers/tests/checks/test_CPLEXDirect.py +++ b/pyomo/solvers/tests/checks/test_CPLEXDirect.py @@ -13,7 +13,7 @@ from pyomo.environ import * import sys -from pyomo.solvers.plugins.solvers.cplex_direct import nullcontext +from pyomo.solvers.plugins.solvers.cplex_direct import nullcontext, _VariableData, _LinearConstraintData, _CplexExpr try: import cplex @@ -241,5 +241,74 @@ class C: self.assertIs(c_in, c) +@unittest.skipIf(not cplexpy_available, "The 'cplex' python bindings are not available") +class TestDataContainers(unittest.TestCase): + def test_variable_data(self): + solver_model = cplex.Cplex() + with _VariableData(solver_model) as var_data: + var_data.add( + lb=0, ub=1, type_=solver_model.variables.type.binary, name="var1" + ) + var_data.add( + lb=0, ub=10, type_=solver_model.variables.type.integer, name="var2" + ) + var_data.add( + lb=-cplex.infinity, + ub=cplex.infinity, + type_=solver_model.variables.type.continuous, + name="var3", + ) + + self.assertEqual(solver_model.variables.get_num(), 0) + self.assertEqual(solver_model.variables.get_num(), 3) + + def test_constraint_data(self): + solver_model = cplex.Cplex() + + solver_model.variables.add( + lb=[-cplex.infinity, -cplex.infinity, -cplex.infinity], + ub=[cplex.infinity, cplex.infinity, cplex.infinity], + types=[ + solver_model.variables.type.continuous, + solver_model.variables.type.continuous, + solver_model.variables.type.continuous, + ], + names=["var1", "var2", "var3"], + ) + + with _LinearConstraintData(solver_model) as con_data: + con_data.add( + cplex_expr=_CplexExpr(variables=[0, 1], coefficients=[10, 100]), + sense="L", + rhs=0, + range_values=0, + name="c1", + ) + con_data.add( + cplex_expr=_CplexExpr(variables=[0], coefficients=[-30]), + sense="G", + rhs=1, + range_values=0, + name="c2", + ) + con_data.add( + cplex_expr=_CplexExpr(variables=[1], coefficients=[80]), + sense="E", + rhs=2, + range_values=0, + name="c3", + ) + con_data.add( + cplex_expr=_CplexExpr(variables=[2], coefficients=[50]), + sense="R", + rhs=3, + range_values=10, + name="c4", + ) + + self.assertEqual(solver_model.linear_constraints.get_num(), 0) + self.assertEqual(solver_model.linear_constraints.get_num(), 4) + + if __name__ == "__main__": unittest.main() From 274f9e511f0bc52f40943a5d8201d633be7d7a81 Mon Sep 17 00:00:00 2001 From: Ruaridh Williamson Date: Wed, 6 May 2020 12:42:26 +0100 Subject: [PATCH 08/14] :rotating_light: Test `_add_var()` --- .../solvers/tests/checks/test_CPLEXDirect.py | 103 ++++++++++++++++++ 1 file changed, 103 insertions(+) diff --git a/pyomo/solvers/tests/checks/test_CPLEXDirect.py b/pyomo/solvers/tests/checks/test_CPLEXDirect.py index 2d9dcf063b3..807f3be205f 100644 --- a/pyomo/solvers/tests/checks/test_CPLEXDirect.py +++ b/pyomo/solvers/tests/checks/test_CPLEXDirect.py @@ -310,5 +310,108 @@ def test_constraint_data(self): self.assertEqual(solver_model.linear_constraints.get_num(), 4) +@unittest.skipIf(not unittest.mock_available, "'mock' is not available") +@unittest.skipIf(not cplexpy_available, "The 'cplex' python bindings are not available") +class TestAddVar(unittest.TestCase): + def test_add_single_variable(self): + """ Test that the variable is added correctly to `solver_model`. """ + model = ConcreteModel() + + opt = SolverFactory("cplex", solver_io="python") + opt._set_instance(model) + + self.assertEqual(opt._solver_model.variables.get_num(), 0) + self.assertEqual(opt._solver_model.variables.get_num_binary(), 0) + + model.X = Var(within=Binary) + + var_interface = opt._solver_model.variables + with unittest.mock.patch.object( + var_interface, "add", wraps=var_interface.add + ) as wrapped_add_call, unittest.mock.patch.object( + var_interface, "set_lower_bounds", wraps=var_interface.set_lower_bounds + ) as wrapped_lb_call, unittest.mock.patch.object( + var_interface, "set_upper_bounds", wraps=var_interface.set_upper_bounds + ) as wrapped_ub_call: + opt._add_var(model.X) + + self.assertEqual(wrapped_add_call.call_count, 1) + self.assertEqual( + wrapped_add_call.call_args, + ({"lb": [0], "names": ["x1"], "types": ["B"], "ub": [1]},), + ) + + self.assertFalse(wrapped_lb_call.called) + self.assertFalse(wrapped_ub_call.called) + + self.assertEqual(opt._solver_model.variables.get_num(), 1) + self.assertEqual(opt._solver_model.variables.get_num_binary(), 1) + + def test_add_block_containing_single_variable(self): + """ Test that the variable is added correctly to `solver_model`. """ + model = ConcreteModel() + + opt = SolverFactory("cplex", solver_io="python") + opt._set_instance(model) + + self.assertEqual(opt._solver_model.variables.get_num(), 0) + self.assertEqual(opt._solver_model.variables.get_num_binary(), 0) + + model.X = Var(within=Binary) + + with unittest.mock.patch.object( + opt._solver_model.variables, "add", wraps=opt._solver_model.variables.add + ) as wrapped_add_call: + opt._add_block(model) + + self.assertEqual(wrapped_add_call.call_count, 1) + self.assertEqual( + wrapped_add_call.call_args, + ({"lb": [0], "names": ["x1"], "types": ["B"], "ub": [1]},), + ) + + self.assertEqual(opt._solver_model.variables.get_num(), 1) + self.assertEqual(opt._solver_model.variables.get_num_binary(), 1) + + def test_add_block_containing_multiple_variables(self): + """ Test that: + - The variable is added correctly to `solver_model` + - The CPLEX `variables` interface is called only once + - Fixed variable bounds are set correctly + """ + model = ConcreteModel() + + opt = SolverFactory("cplex", solver_io="python") + opt._set_instance(model) + + self.assertEqual(opt._solver_model.variables.get_num(), 0) + + model.X1 = Var(within=Binary) + model.X2 = Var(within=NonNegativeReals) + model.X3 = Var(within=NonNegativeIntegers) + + model.X3.fix(5) + + with unittest.mock.patch.object( + opt._solver_model.variables, "add", wraps=opt._solver_model.variables.add + ) as wrapped_add_call: + opt._add_block(model) + + self.assertEqual(wrapped_add_call.call_count, 1) + self.assertEqual( + wrapped_add_call.call_args, + ( + { + "lb": [0, 0, 5], + "names": ["x1", "x2", "x3"], + "types": ["B", "C", "I"], + "ub": [1, cplex.infinity, 5], + }, + ), + ) + + self.assertEqual(opt._solver_model.variables.get_num(), 3) + + if __name__ == "__main__": unittest.main() From 6414dda3dabb3cc144463572c561e580e31aef87 Mon Sep 17 00:00:00 2001 From: Ruaridh Williamson Date: Wed, 6 May 2020 13:10:30 +0100 Subject: [PATCH 09/14] :rotating_light: Test `_add_constraint()` --- .../solvers/tests/checks/test_CPLEXDirect.py | 107 ++++++++++++++++++ 1 file changed, 107 insertions(+) diff --git a/pyomo/solvers/tests/checks/test_CPLEXDirect.py b/pyomo/solvers/tests/checks/test_CPLEXDirect.py index 807f3be205f..449964e4ca7 100644 --- a/pyomo/solvers/tests/checks/test_CPLEXDirect.py +++ b/pyomo/solvers/tests/checks/test_CPLEXDirect.py @@ -413,5 +413,112 @@ def test_add_block_containing_multiple_variables(self): self.assertEqual(opt._solver_model.variables.get_num(), 3) +@unittest.skipIf(not unittest.mock_available, "'mock' is not available") +@unittest.skipIf(not cplexpy_available, "The 'cplex' python bindings are not available") +class TestAddCon(unittest.TestCase): + def test_add_single_constraint(self): + model = ConcreteModel() + model.X = Var(within=Binary) + + opt = SolverFactory("cplex", solver_io="python") + opt._set_instance(model) + + self.assertEqual(opt._solver_model.linear_constraints.get_num(), 0) + + model.C = Constraint(expr=model.X == 1) + + con_interface = opt._solver_model.linear_constraints + with unittest.mock.patch.object( + con_interface, "add", wraps=con_interface.add + ) as wrapped_add_call: + opt._add_constraint(model.C) + + self.assertEqual(wrapped_add_call.call_count, 1) + self.assertEqual( + wrapped_add_call.call_args, + ( + { + "lin_expr": [[[0], (1,)]], + "names": ["x2"], + "range_values": [0.0], + "rhs": [1.0], + "senses": ["E"], + }, + ), + ) + + self.assertEqual(opt._solver_model.linear_constraints.get_num(), 1) + + def test_add_block_containing_single_constraint(self): + model = ConcreteModel() + model.X = Var(within=Binary) + + opt = SolverFactory("cplex", solver_io="python") + opt._set_instance(model) + + self.assertEqual(opt._solver_model.linear_constraints.get_num(), 0) + + model.B = Block() + model.B.C = Constraint(expr=model.X == 1) + + con_interface = opt._solver_model.linear_constraints + with unittest.mock.patch.object( + con_interface, "add", wraps=con_interface.add + ) as wrapped_add_call: + opt._add_block(model.B) + + self.assertEqual(wrapped_add_call.call_count, 1) + self.assertEqual( + wrapped_add_call.call_args, + ( + { + "lin_expr": [[[0], (1,)]], + "names": ["x2"], + "range_values": [0.0], + "rhs": [1.0], + "senses": ["E"], + }, + ), + ) + + self.assertEqual(opt._solver_model.linear_constraints.get_num(), 1) + + def test_add_block_containing_multiple_constraints(self): + model = ConcreteModel() + model.X = Var(within=Binary) + + opt = SolverFactory("cplex", solver_io="python") + opt._set_instance(model) + + self.assertEqual(opt._solver_model.linear_constraints.get_num(), 0) + + model.B = Block() + model.B.C1 = Constraint(expr=model.X == 1) + model.B.C2 = Constraint(expr=model.X <= 1) + model.B.C3 = Constraint(expr=model.X >= 1) + + con_interface = opt._solver_model.linear_constraints + with unittest.mock.patch.object( + con_interface, "add", wraps=con_interface.add + ) as wrapped_add_call: + opt._add_block(model.B) + + self.assertEqual(wrapped_add_call.call_count, 1) + self.assertEqual( + wrapped_add_call.call_args, + ( + { + "lin_expr": [[[0], (1,)], [[0], (1,)], [[0], (1,)]], + "names": ["x2", "x3", "x4"], + "range_values": [0.0, 0.0, 0.0], + "rhs": [1.0, 1.0, 1.0], + "senses": ["E", "L", "G"], + }, + ), + ) + + self.assertEqual(opt._solver_model.linear_constraints.get_num(), 3) + + if __name__ == "__main__": unittest.main() From e363afebf020345205d57d4876ce35d844a65242 Mon Sep 17 00:00:00 2001 From: Ruaridh Williamson Date: Wed, 6 May 2020 13:21:33 +0100 Subject: [PATCH 10/14] :rotating_light: Test `load_vars()` --- .../solvers/tests/checks/test_CPLEXDirect.py | 77 +++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/pyomo/solvers/tests/checks/test_CPLEXDirect.py b/pyomo/solvers/tests/checks/test_CPLEXDirect.py index 449964e4ca7..7fc70e18712 100644 --- a/pyomo/solvers/tests/checks/test_CPLEXDirect.py +++ b/pyomo/solvers/tests/checks/test_CPLEXDirect.py @@ -520,5 +520,82 @@ def test_add_block_containing_multiple_constraints(self): self.assertEqual(opt._solver_model.linear_constraints.get_num(), 3) +@unittest.skipIf(not unittest.mock_available, "'mock' is not available") +@unittest.skipIf(not cplexpy_available, "The 'cplex' python bindings are not available") +class TestLoadVars(unittest.TestCase): + def setUp(self): + opt = SolverFactory("cplex", solver_io="python") + model = ConcreteModel() + model.X = Var(within=NonNegativeReals, initialize=0) + model.Y = Var(within=NonNegativeReals, initialize=0) + + model.C1 = Constraint(expr=2 * model.X + model.Y >= 8) + model.C2 = Constraint(expr=model.X + 3 * model.Y >= 6) + + model.O = Objective(expr=model.X + model.Y) + + opt.solve(model, load_solutions=False, save_results=False) + + self._model = model + self._opt = opt + + def test_all_vars_are_loaded(self): + self.assertTrue(self._model.X.stale) + self.assertTrue(self._model.Y.stale) + self.assertEqual(value(self._model.X), 0) + self.assertEqual(value(self._model.Y), 0) + + with unittest.mock.patch.object( + self._opt._solver_model.solution, + "get_values", + wraps=self._opt._solver_model.solution.get_values, + ) as wrapped_values_call: + self._opt.load_vars() + + self.assertEqual(wrapped_values_call.call_count, 1) + self.assertEqual(wrapped_values_call.call_args, tuple()) + + self.assertFalse(self._model.X.stale) + self.assertFalse(self._model.Y.stale) + self.assertAlmostEqual(value(self._model.X), 3.6) + self.assertAlmostEqual(value(self._model.Y), 0.8) + + def test_only_specified_vars_are_loaded(self): + self.assertTrue(self._model.X.stale) + self.assertTrue(self._model.Y.stale) + self.assertEqual(value(self._model.X), 0) + self.assertEqual(value(self._model.Y), 0) + + with unittest.mock.patch.object( + self._opt._solver_model.solution, + "get_values", + wraps=self._opt._solver_model.solution.get_values, + ) as wrapped_values_call: + self._opt.load_vars([self._model.X]) + + self.assertEqual(wrapped_values_call.call_count, 1) + self.assertEqual(wrapped_values_call.call_args, (([0],), {})) + + self.assertFalse(self._model.X.stale) + self.assertTrue(self._model.Y.stale) + self.assertAlmostEqual(value(self._model.X), 3.6) + self.assertEqual(value(self._model.Y), 0) + + with unittest.mock.patch.object( + self._opt._solver_model.solution, + "get_values", + wraps=self._opt._solver_model.solution.get_values, + ) as wrapped_values_call: + self._opt.load_vars([self._model.Y]) + + self.assertEqual(wrapped_values_call.call_count, 1) + self.assertEqual(wrapped_values_call.call_args, (([1],), {})) + + self.assertFalse(self._model.X.stale) + self.assertFalse(self._model.Y.stale) + self.assertAlmostEqual(value(self._model.X), 3.6) + self.assertAlmostEqual(value(self._model.Y), 0.8) + + if __name__ == "__main__": unittest.main() From e0ab4f6d3787986c3f20cf0c276fee314178d06a Mon Sep 17 00:00:00 2001 From: Ruaridh Williamson Date: Mon, 11 May 2020 09:32:46 +0100 Subject: [PATCH 11/14] :hammer: Use `store_in_cplex()` instead of ctxmanager - Calling a method to "finalise" the data objects is more explicit than `__exit__()` and doesn't rely on `nullcontext()` from CPython --- pyomo/solvers/plugins/solvers/cplex_direct.py | 137 +++++++----------- .../solvers/tests/checks/test_CPLEXDirect.py | 113 +++++++-------- 2 files changed, 107 insertions(+), 143 deletions(-) diff --git a/pyomo/solvers/plugins/solvers/cplex_direct.py b/pyomo/solvers/plugins/solvers/cplex_direct.py index f522a05110b..5f8866a0ea4 100644 --- a/pyomo/solvers/plugins/solvers/cplex_direct.py +++ b/pyomo/solvers/plugins/solvers/cplex_direct.py @@ -76,10 +76,7 @@ def add(self, lb, ub, type_, name): self.types.append(type_) self.names.append(name) - def __enter__(self): - return self - - def __exit__(self, *excinfo): + def store_in_cplex(self): self._solver_model.variables.add( lb=self.lb, ub=self.ub, types=self.types, names=self.names ) @@ -101,10 +98,7 @@ def add(self, cplex_expr, sense, rhs, range_values, name): self.range_values.append(range_values) self.names.append(name) - def __enter__(self): - return self - - def __exit__(self, *excinfo): + def store_in_cplex(self): self._solver_model.linear_constraints.add( lin_expr=self.lin_expr, senses=self.senses, @@ -114,27 +108,6 @@ def __exit__(self, *excinfo): ) -# `nullcontext()` is part of the standard library as of Py3.7 -# This is verbatim from `cpython/Lib/contextlib.py` -class nullcontext(object): - """Context manager that does no additional processing. - Used as a stand-in for a normal context manager, when a particular - block of code is only sometimes used with a normal context manager: - cm = optional_cm if condition else nullcontext() - with cm: - # Perform operation, using optional_cm if condition is True - """ - - def __init__(self, enter_result=None): - self.enter_result = enter_result - - def __enter__(self): - return self.enter_result - - def __exit__(self, *excinfo): - pass - - @SolverFactory.register('cplex_direct', doc='Direct python interface to CPLEX') class CPLEXDirect(DirectSolver): @@ -321,7 +294,7 @@ def _get_expr_from_pyomo_expr(self, expr, max_degree=2): return cplex_expr, referenced_vars - def _add_var(self, var, cplex_var_data=None): + def _add_var(self, var, var_data=None): varname = self._symbol_map.getSymbol(var, self._labeler) vtype = self._cplex_vtype_from_var(var) if var.has_lb(): @@ -337,13 +310,12 @@ def _add_var(self, var, cplex_var_data=None): lb = value(var) ub = value(var) - ctx = ( - _VariableData(self._solver_model) - if cplex_var_data is None - else nullcontext(cplex_var_data) + cplex_var_data = ( + _VariableData(self._solver_model) if var_data is None else var_data ) - with ctx as cplex_var_data: - cplex_var_data.add(lb=lb, ub=ub, type_=vtype, name=varname) + cplex_var_data.add(lb=lb, ub=ub, type_=vtype, name=varname) + if var_data is None: + cplex_var_data.store_in_cplex() self._pyomo_var_to_solver_var_map[var] = varname self._solver_var_to_pyomo_var_map[varname] = var @@ -383,48 +355,50 @@ def _set_instance(self, model, kwds={}): % (var.name, self._pyomo_model.name,)) def _add_block(self, block): - with _VariableData(self._solver_model) as cplex_var_data: - for var in block.component_data_objects( - ctype=pyomo.core.base.var.Var, descend_into=True, active=True, sort=True + var_data = _VariableData(self._solver_model) + for var in block.component_data_objects( + ctype=pyomo.core.base.var.Var, descend_into=True, active=True, sort=True + ): + self._add_var(var, var_data) + var_data.store_in_cplex() + + lin_con_data = _LinearConstraintData(self._solver_model) + for sub_block in block.block_data_objects(descend_into=True, active=True): + for con in sub_block.component_data_objects( + ctype=pyomo.core.base.constraint.Constraint, + descend_into=False, + active=True, + sort=True, ): - self._add_var(var, cplex_var_data) - - with _LinearConstraintData(self._solver_model) as cplex_lin_con_data: - for sub_block in block.block_data_objects(descend_into=True, active=True): - for con in sub_block.component_data_objects( - ctype=pyomo.core.base.constraint.Constraint, - descend_into=False, - active=True, - sort=True, - ): - if not con.has_lb() and not con.has_ub(): - assert not con.equality - continue # non-binding, so skip - - self._add_constraint(con, cplex_lin_con_data) - - for con in sub_block.component_data_objects( - ctype=pyomo.core.base.sos.SOSConstraint, - descend_into=False, - active=True, - sort=True, - ): - self._add_sos_constraint(con) - - obj_counter = 0 - for obj in sub_block.component_data_objects( - ctype=pyomo.core.base.objective.Objective, - descend_into=False, - active=True, - ): - obj_counter += 1 - if obj_counter > 1: - raise ValueError( - "Solver interface does not support multiple objectives." - ) - self._set_objective(obj) + if not con.has_lb() and not con.has_ub(): + assert not con.equality + continue # non-binding, so skip + + self._add_constraint(con, lin_con_data) - def _add_constraint(self, con, cplex_lin_con_data=None): + for con in sub_block.component_data_objects( + ctype=pyomo.core.base.sos.SOSConstraint, + descend_into=False, + active=True, + sort=True, + ): + self._add_sos_constraint(con) + + obj_counter = 0 + for obj in sub_block.component_data_objects( + ctype=pyomo.core.base.objective.Objective, + descend_into=False, + active=True, + ): + obj_counter += 1 + if obj_counter > 1: + raise ValueError( + "Solver interface does not support multiple objectives." + ) + self._set_objective(obj) + lin_con_data.store_in_cplex() + + def _add_constraint(self, con, lin_con_data=None): if not con.active: return None @@ -476,13 +450,14 @@ def _add_constraint(self, con, cplex_lin_con_data=None): ) if len(cplex_expr.q_coefficients) == 0: - ctx = ( + cplex_lin_con_data = ( _LinearConstraintData(self._solver_model) - if cplex_lin_con_data is None - else nullcontext(cplex_lin_con_data) + if lin_con_data is None + else lin_con_data ) - with ctx as cplex_lin_con_data: - cplex_lin_con_data.add(cplex_expr, sense, rhs, range_, conname) + cplex_lin_con_data.add(cplex_expr, sense, rhs, range_, conname) + if lin_con_data is None: + cplex_lin_con_data.store_in_cplex() else: if sense == 'R': raise ValueError("The CPLEXDirect interface does not " diff --git a/pyomo/solvers/tests/checks/test_CPLEXDirect.py b/pyomo/solvers/tests/checks/test_CPLEXDirect.py index 7fc70e18712..08b0d8e079f 100644 --- a/pyomo/solvers/tests/checks/test_CPLEXDirect.py +++ b/pyomo/solvers/tests/checks/test_CPLEXDirect.py @@ -8,12 +8,15 @@ # This software is distributed under the 3-clause BSD License. # ___________________________________________________________________________ -import pyutilib.th as unittest -from pyomo.opt import * -from pyomo.environ import * import sys -from pyomo.solvers.plugins.solvers.cplex_direct import nullcontext, _VariableData, _LinearConstraintData, _CplexExpr +import pyutilib.th as unittest + +from pyomo.environ import * +from pyomo.opt import * +from pyomo.solvers.plugins.solvers.cplex_direct import (_CplexExpr, + _LinearConstraintData, + _VariableData) try: import cplex @@ -229,37 +232,23 @@ def test_dont_skip_trivial_and_call_count_for_unfixed_con_is_one(self): self.assertEqual(mock_is_fixed.call_count, 1) -# `nullcontext()` is part of the standard library as of Py3.7 -# This is verbatim from `cpython/Lib/test/test_contextlib.py` -class NullcontextTestCase(unittest.TestCase): - def test_nullcontext(self): - class C: - pass - - c = C() - with nullcontext(c) as c_in: - self.assertIs(c_in, c) - - @unittest.skipIf(not cplexpy_available, "The 'cplex' python bindings are not available") class TestDataContainers(unittest.TestCase): def test_variable_data(self): solver_model = cplex.Cplex() - with _VariableData(solver_model) as var_data: - var_data.add( - lb=0, ub=1, type_=solver_model.variables.type.binary, name="var1" - ) - var_data.add( - lb=0, ub=10, type_=solver_model.variables.type.integer, name="var2" - ) - var_data.add( - lb=-cplex.infinity, - ub=cplex.infinity, - type_=solver_model.variables.type.continuous, - name="var3", - ) - - self.assertEqual(solver_model.variables.get_num(), 0) + var_data = _VariableData(solver_model) + var_data.add(lb=0, ub=1, type_=solver_model.variables.type.binary, name="var1") + var_data.add( + lb=0, ub=10, type_=solver_model.variables.type.integer, name="var2" + ) + var_data.add( + lb=-cplex.infinity, + ub=cplex.infinity, + type_=solver_model.variables.type.continuous, + name="var3", + ) + self.assertEqual(solver_model.variables.get_num(), 0) + var_data.store_in_cplex() self.assertEqual(solver_model.variables.get_num(), 3) def test_constraint_data(self): @@ -275,38 +264,38 @@ def test_constraint_data(self): ], names=["var1", "var2", "var3"], ) + con_data = _LinearConstraintData(solver_model) + con_data.add( + cplex_expr=_CplexExpr(variables=[0, 1], coefficients=[10, 100]), + sense="L", + rhs=0, + range_values=0, + name="c1", + ) + con_data.add( + cplex_expr=_CplexExpr(variables=[0], coefficients=[-30]), + sense="G", + rhs=1, + range_values=0, + name="c2", + ) + con_data.add( + cplex_expr=_CplexExpr(variables=[1], coefficients=[80]), + sense="E", + rhs=2, + range_values=0, + name="c3", + ) + con_data.add( + cplex_expr=_CplexExpr(variables=[2], coefficients=[50]), + sense="R", + rhs=3, + range_values=10, + name="c4", + ) - with _LinearConstraintData(solver_model) as con_data: - con_data.add( - cplex_expr=_CplexExpr(variables=[0, 1], coefficients=[10, 100]), - sense="L", - rhs=0, - range_values=0, - name="c1", - ) - con_data.add( - cplex_expr=_CplexExpr(variables=[0], coefficients=[-30]), - sense="G", - rhs=1, - range_values=0, - name="c2", - ) - con_data.add( - cplex_expr=_CplexExpr(variables=[1], coefficients=[80]), - sense="E", - rhs=2, - range_values=0, - name="c3", - ) - con_data.add( - cplex_expr=_CplexExpr(variables=[2], coefficients=[50]), - sense="R", - rhs=3, - range_values=10, - name="c4", - ) - - self.assertEqual(solver_model.linear_constraints.get_num(), 0) + self.assertEqual(solver_model.linear_constraints.get_num(), 0) + con_data.store_in_cplex() self.assertEqual(solver_model.linear_constraints.get_num(), 4) From 742d8bc46f7f5a60d12d32eea101b57f6880292c Mon Sep 17 00:00:00 2001 From: Ruaridh Williamson Date: Mon, 11 May 2020 09:33:33 +0100 Subject: [PATCH 12/14] :books: Formatting --- pyomo/solvers/plugins/solvers/cplex_direct.py | 2 +- pyomo/solvers/tests/checks/test_CPLEXDirect.py | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/pyomo/solvers/plugins/solvers/cplex_direct.py b/pyomo/solvers/plugins/solvers/cplex_direct.py index 5f8866a0ea4..39c5d0bc41c 100644 --- a/pyomo/solvers/plugins/solvers/cplex_direct.py +++ b/pyomo/solvers/plugins/solvers/cplex_direct.py @@ -48,7 +48,7 @@ def __init__( ): self.variables = variables self.coefficients = coefficients - self.offset = offset or 0. + self.offset = offset or 0.0 self.q_variables1 = q_variables1 or [] self.q_variables2 = q_variables2 or [] self.q_coefficients = q_coefficients or [] diff --git a/pyomo/solvers/tests/checks/test_CPLEXDirect.py b/pyomo/solvers/tests/checks/test_CPLEXDirect.py index 08b0d8e079f..f1954885483 100644 --- a/pyomo/solvers/tests/checks/test_CPLEXDirect.py +++ b/pyomo/solvers/tests/checks/test_CPLEXDirect.py @@ -153,6 +153,7 @@ def test_optimal_mip(self): @unittest.skipIf(not cplexpy_available, "The 'cplex' python bindings are not available") class TestIsFixedCallCount(unittest.TestCase): """ Tests for PR#1402 (669e7b2b) """ + def setup(self, skip_trivial_constraints): m = ConcreteModel() m.x = Var() From fc0b8ccad3d7cfb5b46ae48bdc15cb01ae9e575e Mon Sep 17 00:00:00 2001 From: Ruaridh Williamson Date: Thu, 21 May 2020 11:38:49 +0100 Subject: [PATCH 13/14] :hammer: Refactor quadratic objective handling --- pyomo/solvers/plugins/solvers/cplex_direct.py | 35 ++++++++++++++----- .../tests/checks/test_CPLEXPersistent.py | 35 +++++++++++++++++++ 2 files changed, 62 insertions(+), 8 deletions(-) create mode 100644 pyomo/solvers/tests/checks/test_CPLEXPersistent.py diff --git a/pyomo/solvers/plugins/solvers/cplex_direct.py b/pyomo/solvers/plugins/solvers/cplex_direct.py index 39c5d0bc41c..2d1a78c8c22 100644 --- a/pyomo/solvers/plugins/solvers/cplex_direct.py +++ b/pyomo/solvers/plugins/solvers/cplex_direct.py @@ -538,8 +538,6 @@ def _set_objective(self, obj): self._vars_referenced_by_obj = ComponentSet() self._objective = None - self._solver_model.objective.set_linear([(i, 0.0) for i in range(len(self._pyomo_var_to_solver_var_map.values()))]) - if obj.active is False: raise ValueError('Cannot add inactive objective to solver.') @@ -560,12 +558,33 @@ def _set_objective(self, obj): self._solver_model.objective.set_sense(sense) if hasattr(self._solver_model.objective, 'set_offset'): self._solver_model.objective.set_offset(cplex_expr.offset) - if len(cplex_expr.coefficients) != 0: - self._solver_model.objective.set_linear(list(zip(cplex_expr.variables, cplex_expr.coefficients))) - if len(cplex_expr.q_coefficients) != 0: - self._solver_model.objective.set_quadratic_coefficients(list(zip(cplex_expr.q_variables1, - cplex_expr.q_variables2, - cplex_expr.q_coefficients))) + + linear_objective_already_exists = any(self._solver_model.objective.get_linear()) + quadratic_objective_already_exists = self._solver_model.objective.get_num_quadratic_nonzeros() + + contains_linear_terms = any(cplex_expr.coefficients) + contains_quadratic_terms = any(cplex_expr.q_coefficients) + num_cols = len(self._pyomo_var_to_solver_var_map) + + if linear_objective_already_exists or contains_linear_terms: + self._solver_model.objective.set_linear([(i, 0.0) for i in range(num_cols)]) + + if contains_linear_terms: + self._solver_model.objective.set_linear(list(zip(cplex_expr.variables, cplex_expr.coefficients))) + + if quadratic_objective_already_exists or contains_quadratic_terms: + self._solver_model.objective.set_quadratic([0] * num_cols) + + if contains_quadratic_terms: + self._solver_model.objective.set_quadratic_coefficients( + list( + zip( + cplex_expr.q_variables1, + cplex_expr.q_variables2, + cplex_expr.q_coefficients + ) + ) + ) self._objective = obj self._vars_referenced_by_obj = referenced_vars diff --git a/pyomo/solvers/tests/checks/test_CPLEXPersistent.py b/pyomo/solvers/tests/checks/test_CPLEXPersistent.py new file mode 100644 index 00000000000..51ff1ab7e43 --- /dev/null +++ b/pyomo/solvers/tests/checks/test_CPLEXPersistent.py @@ -0,0 +1,35 @@ +import pyutilib.th as unittest + +from pyomo.environ import * +from pyomo.opt import * + +try: + import cplex + + cplexpy_available = True +except ImportError: + cplexpy_available = False + + +@unittest.skipIf(not cplexpy_available, "The 'cplex' python bindings are not available") +class TestQuadraticObjective(unittest.TestCase): + def test_quadratic_objective_is_set(self): + model = ConcreteModel() + model.X = Var(bounds=(-2, 2)) + model.Y = Var(bounds=(-2, 2)) + model.O = Objective(expr=model.X ** 2 + model.Y ** 2) + model.C1 = Constraint(expr=model.Y >= 2 * model.X - 1) + model.C2 = Constraint(expr=model.Y >= -model.X + 2) + opt = SolverFactory("cplex_persistent") + opt.set_instance(model) + opt.solve() + + self.assertAlmostEqual(model.X.value, 1, places=3) + self.assertAlmostEqual(model.Y.value, 1, places=3) + + del model.O + model.O = Objective(expr=model.X ** 2) + opt.set_objective(model.O) + opt.solve() + self.assertAlmostEqual(model.X.value, 0, places=3) + self.assertAlmostEqual(model.Y.value, 2, places=3) From 41428a015abf40a0d194c368f2407587cb3e8ad0 Mon Sep 17 00:00:00 2001 From: Ruaridh Williamson Date: Thu, 21 May 2020 17:44:00 +0100 Subject: [PATCH 14/14] :bug: Cast quadratic coefficients to floats explicitly --- pyomo/solvers/plugins/solvers/cplex_direct.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pyomo/solvers/plugins/solvers/cplex_direct.py b/pyomo/solvers/plugins/solvers/cplex_direct.py index 2d1a78c8c22..1d21c98b224 100644 --- a/pyomo/solvers/plugins/solvers/cplex_direct.py +++ b/pyomo/solvers/plugins/solvers/cplex_direct.py @@ -51,7 +51,7 @@ def __init__( self.offset = offset or 0.0 self.q_variables1 = q_variables1 or [] self.q_variables2 = q_variables2 or [] - self.q_coefficients = q_coefficients or [] + self.q_coefficients = [float(coef) for coef in q_coefficients or []] def _is_numeric(x): @@ -573,7 +573,7 @@ def _set_objective(self, obj): self._solver_model.objective.set_linear(list(zip(cplex_expr.variables, cplex_expr.coefficients))) if quadratic_objective_already_exists or contains_quadratic_terms: - self._solver_model.objective.set_quadratic([0] * num_cols) + self._solver_model.objective.set_quadratic([0.0] * num_cols) if contains_quadratic_terms: self._solver_model.objective.set_quadratic_coefficients(