Require binary variables in logical constraints

CHANGELOG.md

@@ -6,6 +6,7 @@
 - Added isPositive(), isNegative(), isFeasLE(), isFeasLT(), isFeasGE(), isFeasGT(), isHugeValue(), and tests
 - Added SCIP_LOCKTYPE, addVarLocksType(), getNLocksDown(), getNLocksUp(), getNLocksDownType(), getNLocksUpType(), and tests
 ### Fixed
+- Fixed segmentation fault when logical constraints were used with expressions rather than variables.
 ### Changed
 ### Removed
 

src/pyscipopt/scip.pxi

@@ -2397,6 +2397,18 @@ 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, int size):
+        self.size = size
+        self.ptr = <SCIP_VAR**> malloc(size * sizeof(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 +6320,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 == '':
@@ -6324,6 +6339,17 @@ cdef class Model:
 
         return pyCons
 
+    def _convert_var_to_scipvar(self, vars, nvars):
+        cdef VarArrayWrapper wrapper = VarArrayWrapper(nvars)
+
+        for i, var in enumerate(vars):
+            if not isinstance(var, Variable):
+                raise TypeError("Expected Variable, got %s." % type(var))
+
+            wrapper.ptr[i] = (<Variable>var).scip_var
+
+        return wrapper
+
     def addConsOr(self, vars, resvar, name="",
             initial=True, separate=True, enforce=True, check=True,
             propagate=True, local=False, modifiable=False, dynamic=False,
@@ -6366,14 +6392,13 @@ cdef class Model:
 
         """
         cdef int nvars = len(vars)
-        cdef SCIP_VAR** _vars = <SCIP_VAR**> malloc(nvars * sizeof(SCIP_VAR*))
         cdef SCIP_VAR* _resvar
         cdef SCIP_CONS* scip_cons
         cdef int i
 
         _resvar = (<Variable>resvar).scip_var
-        for i, var in enumerate(vars):
-            _vars[i] = (<Variable>var).scip_var
+        cdef VarArrayWrapper wrapper = self._convert_var_to_scipvar(vars, nvars)
+        cdef SCIP_VAR** _vars = wrapper.ptr
 
         if name == '':
             name = 'c'+str(SCIPgetNConss(self._scip)+1)
@@ -6385,8 +6410,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 +6460,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 == '':
@@ -8763,7 +8789,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 = {}