Require binary variables in logical constraints

CHANGELOG.md

@@ -5,7 +5,9 @@
 - Added support for knapsack constraints
 - Added isPositive(), isNegative(), isFeasLE(), isFeasLT(), isFeasGE(), isFeasGT(), isHugeValue(), and tests
 - Added SCIP_LOCKTYPE, addVarLocksType(), getNLocksDown(), getNLocksUp(), getNLocksDownType(), getNLocksUpType(), and tests
+- Added internal class _VarArrayWrapper to get the scip variables from PySCIPOpt variables
 ### Fixed
+- Fixed segmentation fault when logical constraints were used with expressions rather than variables.
 ### Changed
 ### Removed
 

src/pyscipopt/scip.pxi

@@ -2397,6 +2397,30 @@ cdef void relayErrorMessage(void *messagehdlr, FILE *file, const char *msg) noex
             fputs(msg, file)
         fflush(file)
 
+cdef class _VarArrayWrapper:
+    cdef SCIP_VAR** ptr
+    cdef int size
+
+    def __cinit__(self, object vars):
+        cdef int size = len(vars)
+        self.ptr = <SCIP_VAR**> malloc(size * sizeof(SCIP_VAR*))
+
+        if size == 1:
+            if not isinstance(vars, Variable):
+                raise TypeError("Expected Variable, got %s." % type(vars))
+            else:
+                self.ptr[0] = (<Variable>vars).scip_var
+        else:
+            for i, var in enumerate(vars):
+                if not isinstance(var, Variable):
+                    raise TypeError("Expected Variable, got %s." % type(var))
+
+                self.ptr[i] = (<Variable>var).scip_var
+
+    def __dealloc__(self):
+        if self.ptr != NULL:
+            free(self.ptr)
+
 # - remove create(), includeDefaultPlugins(), createProbBasic() methods
 # - replace free() by "destructor"
 # - interface SCIPfreeProb()
@@ -6308,6 +6332,9 @@ cdef class Model:
 
         _resvar = (<Variable>resvar).scip_var
         for i, var in enumerate(vars):
+            if not isinstance(var, Variable):
+                raise TypeError("Expected Variable, got %s." % type(var))
+
             _vars[i] = (<Variable>var).scip_var
 
         if name == '':
@@ -6366,14 +6393,17 @@ cdef class Model:
 
         """
         cdef int nvars = len(vars)
-        cdef SCIP_VAR** _vars = <SCIP_VAR**> malloc(nvars * sizeof(SCIP_VAR*))
+        cdef SCIP_VAR** _vars
         cdef SCIP_VAR* _resvar
         cdef SCIP_CONS* scip_cons
         cdef int i
+        cdef _VarArrayWrapper wrapper
 
-        _resvar = (<Variable>resvar).scip_var
-        for i, var in enumerate(vars):
-            _vars[i] = (<Variable>var).scip_var
+        wrapper = _VarArrayWrapper(resvar)
+        _resvar = wrapper.ptr
+
+        wrapper = _VarArrayWrapper(vars)
+        _vars = wrapper.ptr
 
         if name == '':
             name = 'c'+str(SCIPgetNConss(self._scip)+1)
@@ -6385,8 +6415,6 @@ cdef class Model:
         pyCons = Constraint.create(scip_cons)
         PY_SCIP_CALL(SCIPreleaseCons(self._scip, &scip_cons))
 
-        free(_vars)
-
         return pyCons
 
     def addConsXor(self, vars, rhsvar, name="",
@@ -6437,6 +6465,9 @@ cdef class Model:
 
         assert type(rhsvar) is type(bool()), "Provide BOOLEAN value as rhsvar, you gave %s." % type(rhsvar)
         for i, var in enumerate(vars):
+            if not isinstance(var, Variable):
+                raise TypeError("Expected Variable, got %s." % type(var))
+
             _vars[i] = (<Variable>var).scip_var
 
         if name == '':
@@ -8239,9 +8270,10 @@ cdef class Model:
         cdef SCIP_HEUR* _heur
         cdef SCIP_Bool success
         cdef int i
-
-        for i, var in enumerate(sub_model.getVars()):
-            vars[i] = (<Variable>var).scip_var
+        cdef _VarArrayWrapper wrapper
+
+        wrapper = _VarArrayWrapper(sub_model.getVars())
+        vars = wrapper.ptr
 
         name = str_conversion(heur.name)
         _heur = SCIPfindHeur(self._scip, name)
@@ -8763,7 +8795,7 @@ cdef class Model:
         return Node.create(downchild), Node.create(eqchild), Node.create(upchild)
 
 
-    def branchVarVal(self, variable, value):
+    def branchVarVal(self, Variable variable, value):
         """
         Branches on variable using a value which separates the domain of the variable.
 

tests/test_cons.py

@@ -72,6 +72,23 @@ def test_cons_logical():
     assert m.isEQ(m.getVal(result1), 1)
     assert m.isEQ(m.getVal(result2), 0)
 
+@pytest.mark.xfail()
+def test_cons_logical_fail():
+    m = Model()
+    x1 = m.addVar(vtype="B")
+    x2 = m.addVar(vtype="B")
+    x3 = m.addVar(vtype="B")
+    x4 = m.addVar(vtype="B")
+    result1 = m.addVar(vtype="B")
+
+    m.addCons(x3 == 1 - x1)
+    m.addCons(x4 == 1 - x2)
+
+    # result1 false
+    m.addConsOr([x1*x3, x2*x4], result1)
+
+    m.optimize()
+
 def test_SOScons():
     m = Model()
     x = {}