Fixes for the gurobi interface and OSQP tolerances (#235)

* fix config serialization in gurobi and tolerance errors in OSQP

CHANGELOG.rst

@@ -3,6 +3,12 @@
 Next Release
 ------------
 
+1.5.2
+-----
+* Gurobi can now serialize its configuration correctly. This also fixes pickling of Gurobi models.
+* Fix the shim for integrality tolerance in OSQP which makes it easier to clone OSQP Models to other solvers.
+* Fix an issue where one could not rename variables in Gurobi version 9.
+
 1.5.1
 -----
 * GLPK now respects `Configuration.tolerances.integrality` again

src/optlang/gurobi_interface.py

@@ -27,7 +27,6 @@
 
 import os
 import six
-from functools import partial
 from optlang import interface
 from optlang.util import inheritdocstring, TemporaryFilename
 from optlang.expression_parsing import parse_optimization_expression
@@ -61,6 +60,9 @@
                           'binary': gurobipy.GRB.BINARY}
 _GUROBI_VTYPE_TO_VTYPE = dict((val, key) for key, val in _VTYPE_TO_GUROBI_VTYPE.items())
 
+# hacky way since Gurobi does not allow getting the version via the API
+_IS_GUROBI_9_OR_NEWER = hasattr(gurobipy, "MLinExpr")
+
 
 def _constraint_lb_and_ub_to_gurobi_sense_rhs_and_range_value(lb, ub):
     """Helper function used by Constraint and Model"""
@@ -91,17 +93,15 @@ def _constraint_lb_and_ub_to_gurobi_sense_rhs_and_range_value(lb, ub):
 class Variable(interface.Variable):
     def __init__(self, name, *args, **kwargs):
         super(Variable, self).__init__(name, **kwargs)
-        self._original_name = name  # due to bug with getVarByName ... TODO: file bug report on gurobi google group
+        self._original_name = name
 
     @property
     def _internal_variable(self):
         if getattr(self, 'problem', None) is not None:
-
-            internal_variable = self.problem.problem.getVarByName(self._original_name)
-
-            assert internal_variable is not None
-            # if internal_variable is None:
-            #     raise Exception('Cannot find variable {}')
+            if _IS_GUROBI_9_OR_NEWER:
+                internal_variable = self.problem.problem.getVarByName(self.name)
+            else:
+                internal_variable = self.problem.problem.getVarByName(self._original_name)
             return internal_variable
         else:
             return None
@@ -478,12 +478,26 @@ def timeout(self, value):
         self._timeout = value
 
     def __getstate__(self):
-        return {'presolve': self.presolve, 'verbosity': self.verbosity, 'timeout': self.timeout}
+        return {
+            "presolve": self.presolve,
+            "timeout": self.timeout,
+            "verbosity": self.verbosity,
+            "lp_method": self.lp_method,
+            "qp_method": self.qp_method,
+            "tolerances": {
+                "feasibility": self.tolerances.feasibility,
+                "optimality": self.tolerances.optimality,
+                "integrality": self.tolerances.integrality,
+            },
+        }
 
     def __setstate__(self, state):
-        self.__init__()
         for key, val in six.iteritems(state):
-            setattr(self, key, val)
+            if key != "tolerances":
+                setattr(self, key, val)
+        for key, val in six.iteritems(state["tolerances"]):
+            if key in self._tolerance_functions():
+                setattr(self.tolerances, key, val)
 
     def _get_feasibility(self):
         return getattr(self.problem.problem.params, "FeasibilityTol")
@@ -593,21 +607,20 @@ def __getstate__(self):
             self.problem.write(tmp_file_name)
             with open(tmp_file_name) as tmp_file:
                 lp_form = tmp_file.read()
-        repr_dict = {'lp': lp_form, 'status': self.status, 'config': self.configuration}
+        repr_dict = {
+            'lp': lp_form,
+            'status': self.status,
+            'config': self.configuration.__getstate__()
+        }
         return repr_dict
 
     def __setstate__(self, repr_dict):
         with TemporaryFilename(suffix=".lp", content=repr_dict["lp"]) as tmp_file_name:
             problem = gurobipy.read(tmp_file_name)
-        # if repr_dict['status'] == 'optimal':  # TODO: uncomment this
-        #     # turn off logging completely, get's configured later
-        #     problem.set_error_stream(None)
-        #     problem.set_warning_stream(None)
-        #     problem.set_log_stream(None)
-        #     problem.set_results_stream(None)
-        #     problem.solve()  # since the start is an optimal solution, nothing will happen here
         self.__init__(problem=problem)
-        self.configuration = Configuration.clone(repr_dict['config'], problem=self)  # TODO: make configuration work
+        self.configuration = Configuration()
+        self.configuration.problem = self
+        self.configuration.__setstate__(repr_dict["config"])
 
     def to_lp(self):
         self.problem.update()

src/optlang/osqp_interface.py

@@ -652,7 +652,7 @@ def _set_feasibility(self, value):
         self.problem.problem.settings["eps_dual_inf"] = value
 
     def _get_integrality(self):
-        return None
+        return 1e-6
 
     def _set_integrality(self, value):
         pass

src/optlang/symbolics.py

@@ -36,8 +36,7 @@
 else:  # pragma: no cover
     try:
         import symengine
-        import symengine.sympy_compat
-        from symengine.sympy_compat import Symbol as symengine_Symbol
+        from symengine import Symbol as symengine_Symbol
     except ImportError as e:
         if SYMENGINE_PREFERENCE.lower() in ("true", "yes", "on"):
             logger.warn("Symengine could not be imported: " + str(e))
@@ -59,11 +58,11 @@
     Zero = Real(0)
     One = Real(1)
     NegativeOne = Real(-1)
-    sympify = symengine.sympy_compat.sympify
+    sympify = symengine.sympify
 
     Add = symengine.Add
     Mul = symengine.Mul
-    Pow = symengine.sympy_compat.Pow
+    Pow = symengine.Pow
 
     class Symbol(symengine_Symbol):
         def __new__(cls, name, *args, **kwargs):

src/optlang/tests/test_gurobi_interface.py

@@ -1,6 +1,7 @@
 # Copyright (c) 2016 Novo Nordisk Foundation Center for Biosustainability, DTU.
 # See LICENSE for details.
 
+import pickle
 import unittest
 
 try:  # noqa: C901
@@ -179,6 +180,15 @@ def test_init_from_existing_problem(self):
             self.assertEqual(self.model.constraints.keys(),
                              [constr.ConstrName for constr in self.model.problem.getConstrs()])
 
+        def test_model_can_be_pickled(self):
+            s = pickle.dumps(self.model)
+            mod = pickle.loads(s)
+            self.assertEqual(len(self.model.variables), len(mod.variables))
+            self.assertEqual(len(self.model.constraints), len(mod.constraints))
+            self.assertEqual(self.model.configuration.presolve, mod.configuration.presolve)
+            self.assertEqual(self.model.configuration.tolerances.feasibility,
+                             mod.configuration.tolerances.feasibility)
+
         def test_gurobi_add_variable(self):
             var = Variable('x')
             self.model.add(var)
@@ -252,7 +262,7 @@ def test_gurobi_add_constraints(self):
                 coeff_dict[row.getVar(i).VarName] = row.getCoeff(i)
             self.assertDictEqual(coeff_dict, {'x': 0.3, 'y': 0.4, 'z': 66., 'test_aux': -1.0})
             self.assertEqual(internal_constraint.RHS, constr1.lb)
-            self.assertEqual(self.model.problem.getVarByName(internal_constraint.getAttr('ConstrName') + '_aux'), 100)
+            self.assertEqual(self.model.problem.getVarByName(internal_constraint.getAttr('ConstrName') + '_aux').ub, 100)
             # constr2
             coeff_dict = dict()
             internal_constraint = self.model.problem.getConstrByName(constr2.name)
@@ -297,7 +307,7 @@ def test_change_of_constraint_is_reflected_in_low_level_solver(self):
             z = Variable('z', lb=3, ub=10, type='integer')
             self.assertEqual(z._internal_variable, None)
             constraint += 77. * z
-            self.assertEqual(z._internal_variable, self.model.problem.getVarByName('z'))
+            self.assertIs(z._internal_variable, self.model.problem.getVarByName('z'))
             self.assertEqual(self.model.constraints['test'].lb, -100)
             self.assertEqual(
                 (self.model.constraints['test'].expression - (0.4 * y + 0.3 * x + 77.0 * z)).expand() - 0,

src/optlang/tests/test_osqp_interface.py

@@ -252,7 +252,7 @@ def test_pickle_ability(self):
                              [(constr.lb, constr.ub, constr.name) for constr in self.model.constraints])
 
         def test_config_gets_copied_too(self):
-            self.assertEquals(self.model.configuration.verbosity, 0)
+            self.assertEqual(self.model.configuration.verbosity, 0)
             self.model.configuration.verbosity = 1
             model_copy = copy.copy(self.model)
             self.assertEqual(model_copy.configuration.verbosity, 1)

tox.ini

@@ -3,7 +3,7 @@ envlist = isort, black, flake8, safety, docs, py27, py3{6,7,8,9}-symengine
 
 [gh-actions]
 python =
-     2.7: safety, py27
+     2.7: py27
      3.6: safety, py36, py36-symengine
      3.7: safety, py37, py37-symengine
      3.8: safety, py38, py38-symengine
@@ -75,6 +75,7 @@ commands=
 
 [testenv:safety]
 deps=
+     pip>=21.1
      safety
 commands=
      safety check --full-report