fix: make model.Clone use json as default (#44)

* fix: make model.Clone use json as default

Using LP format as intermediate has problems on cross-solver clones.

* chore: add warning to clone with use_lp

* test: change clone test to use json

* fix: import warnings

* test: assert that cloned optimal value is the same

optlang/interface.py

@@ -28,6 +28,7 @@
 import logging
 import sys
 import uuid
+import warnings
 
 import six
 
@@ -1041,7 +1042,7 @@ class Model(object):
     """
 
     @classmethod
-    def clone(cls, model, use_lp=True):
+    def clone(cls, model, use_json=True, use_lp=False):
         """
         Make a copy of a model. The model being copied can be of the same type or belong to
         a different solver interface. This is the preferred way of copying models.
@@ -1052,11 +1053,18 @@ def clone(cls, model, use_lp=True):
         """
         model.update()
         interface = sys.modules[cls.__module__]
-        if use_lp and hasattr(cls, "from_lp") and hasattr(model, "to_lp"):
+
+        if use_lp:
+            warnings.warn("Cloning with LP formats can change variable and constraint ID's.")
             new_model = cls.from_lp(model.to_lp())
             new_model.configuration = interface.Configuration.clone(model.configuration, problem=new_model)
             return new_model
 
+        if use_json:
+            new_model = cls.from_json(model.to_json())
+            new_model.configuration = interface.Configuration.clone(model.configuration, problem=new_model)
+            return new_model
+
         new_model = cls()
         for variable in model.variables:
             new_variable = interface.Variable.clone(variable)

optlang/tests/abstract_test_cases.py

@@ -518,21 +518,41 @@ def test_change_objective_can_handle_removed_vars(self):
         self.model.update()
         self.model.objective = self.interface.Objective(self.model.variables[1] * 2)
 
-    def test_clone_model_with_lp(self):
+    def test_clone_model_with_json(self):
         self.assertEquals(self.model.configuration.verbosity, 0)
         self.model.configuration.verbosity = 3
+        self.model.optimize()
+        opt = self.model.objective.value
         cloned_model = self.interface.Model.clone(self.model)
         self.assertEquals(cloned_model.configuration.verbosity, 3)
         self.assertEquals(len(cloned_model.variables), len(self.model.variables))
         self.assertEquals(len(cloned_model.constraints), len(self.model.constraints))
+        cloned_model.optimize()
+        self.assertAlmostEqual(cloned_model.objective.value, opt)
 
-    def test_clone_model_without_lp(self):
+    def test_clone_model_with_lp(self):
         self.assertEquals(self.model.configuration.verbosity, 0)
         self.model.configuration.verbosity = 3
-        cloned_model = self.interface.Model.clone(self.model, use_lp=False)
+        self.model.optimize()
+        opt = self.model.objective.value
+        cloned_model = self.interface.Model.clone(self.model, use_lp=True)
+        self.assertEquals(cloned_model.configuration.verbosity, 3)
+        self.assertEquals(len(cloned_model.variables), len(self.model.variables))
+        self.assertEquals(len(cloned_model.constraints), len(self.model.constraints))
+        cloned_model.optimize()
+        self.assertAlmostEqual(cloned_model.objective.value, opt)
+
+    def test_clone_model_without_json(self):
+        self.assertEquals(self.model.configuration.verbosity, 0)
+        self.model.configuration.verbosity = 3
+        self.model.optimize()
+        opt = self.model.objective.value
+        cloned_model = self.interface.Model.clone(self.model, use_json=False)
         self.assertEquals(cloned_model.configuration.verbosity, 3)
         self.assertEquals(len(cloned_model.variables), len(self.model.variables))
         self.assertEquals(len(cloned_model.constraints), len(self.model.constraints))
+        cloned_model.optimize()
+        self.assertAlmostEqual(cloned_model.objective.value, opt)
 
 
 @six.add_metaclass(abc.ABCMeta)