BUG: _VarArray can't handle MatrixVar (#1044)

* Refactor _VarArray initialization and type checks

Simplifies and clarifies the logic for initializing _VarArray by restructuring type checks and error handling. Now uses f-strings for error messages and ensures consistent handling of empty lists and invalid types.

* Refactor _VarArray to support MatrixVariable input

Updated the _VarArray class constructor to handle MatrixVariable inputs in addition to Variable, list, and tuple. Refactored pointer allocation logic into a helper function for improved code clarity and maintainability.

* Add test for indicator constraint with matrix binary var

Introduces a new test to verify that addConsIndicator works correctly when the binary variable is a matrix variable, addressing issue #1043.

* Update CHANGELOG.md

* Declare ptr variable in create_ptr function

To fix "Storing unsafe C derivative of temporary Python reference"

* Refactor create_ptr to remove redundant size argument

The create_ptr function in _VarArray.__cinit__ no longer takes a separate size argument and instead uses len(vars) directly. This simplifies the function signature and reduces redundancy.

* Refactor _VarArray initialization logic

Inlined the pointer creation logic in _VarArray's __cinit__ method, removing the inner create_ptr function. This simplifies the code and improves readability while maintaining the same functionality.

* Update CHANGELOG

* Update test for addConsIndicator with activeone flag

The test now checks addConsIndicator with both activeone=True and activeone=False, and updates the objective to use binvar.sum().

* ENH: flatten list

* BUG: use `np.ravel` avoid to run out of vars range

* test binvar with (1, 1, 1) matrix

* test binvar with (2, 3) matrix

* test binvar with (2, 1) list of lists

* correct index

* Add TypeError test for addConsIndicator with binvar

* Only requiring a 1D array

* TST: raise an error while get not 1D array

* MAINT: Update error message

* TST: update error type

* Update CHANGELOG.md

* Fix _VarArray initialization for empty input

Ensures _VarArray initializes ptr and size to NULL and 0, respectively, before processing input. Prevents allocation and assignment errors when an empty or invalid input is provided.

* Remove _VarArray changing from changelog

* TST: update the comparing  method

Co-authored-by: DominikKamp <[email protected]>

---------

Co-authored-by: João Dionísio <[email protected]>
Co-authored-by: DominikKamp <[email protected]>

Author: 3 people

src/pyscipopt/scip.pxi

@@ -2498,22 +2498,24 @@ cdef class _VarArray:
     cdef int size
 
     def __cinit__(self, object vars):
+        self.ptr = NULL
+        self.size = 0
+
         if isinstance(vars, Variable):
-            self.size = 1
-            self.ptr = <SCIP_VAR**> malloc(sizeof(SCIP_VAR*))
-            self.ptr[0] = (<Variable>vars).scip_var
+            vars = [vars]
+        elif isinstance(vars, (list, tuple, MatrixVariable)):
+            if (ndim := np.ndim(vars)) != 1:
+                raise ValueError(f"Expected a 1D array, but got a {ndim}D array.")
         else:
-            if not isinstance(vars, (list, tuple)):
-                raise TypeError("Expected Variable or list of Variable, got %s." % type(vars))
+            raise TypeError(f"Expected Variable or list of Variable, got {type(vars)}.")
+
+        if vars:
             self.size = len(vars)
-            if self.size == 0:
-                self.ptr = NULL
-            else:
-                self.ptr = <SCIP_VAR**> malloc(self.size * sizeof(SCIP_VAR*))
-                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
+            self.ptr = <SCIP_VAR**> malloc(self.size * sizeof(SCIP_VAR*))
+            for i, var in enumerate(vars):
+                if not isinstance(var, Variable):
+                    raise TypeError(f"Expected Variable, got {type(var)}.")
+                self.ptr[i] = (<Variable>var).scip_var
 
     def __dealloc__(self):
         if self.ptr != NULL:

tests/test_cons.py

@@ -179,6 +179,37 @@ def test_cons_indicator():
     assert m.isEQ(m.getVal(x), 1)
     assert c1.getConshdlrName() == "indicator"
 
+def test_cons_indicator_with_matrix_binvar():
+    # test matrix variable binvar #1043
+    m = Model()
+    x = m.addVar(vtype="B")
+
+    # test binvar with int
+    with pytest.raises(TypeError):
+        m.addConsIndicator(x <= 0, 1)
+
+    # test binvar with (1, 1, 1) shape of matrix variable
+    with pytest.raises(ValueError):
+        m.addConsIndicator(x <= 0, m.addMatrixVar(((1, 1, 1)), vtype="B"))
+
+    # test binvar with (2, 3) shape of matrix variable
+    with pytest.raises(ValueError):
+        m.addConsIndicator(x <= 0, m.addMatrixVar(((2, 3)), vtype="B"))
+
+    # test binvar with (2, 1) shape of list of lists
+    with pytest.raises(ValueError):
+        m.addConsIndicator(x <= 0, [[m.addVar(vtype="B")], [m.addVar(vtype="B")]])
+
+    # test binvar with requiring type and dimension
+    binvar = m.addMatrixVar(1, vtype="B")
+    m.addConsIndicator(x >= 1, binvar, activeone=True)
+    m.addConsIndicator(x <= 0, binvar, activeone=False)
+
+    m.setObjective(binvar.sum(), "maximize")
+    m.optimize()
+
+    assert m.isEQ(m.getVal(x), 1)
+
 @pytest.mark.xfail(
     reason="addConsIndicator doesn't behave as expected when binary variable is False. See Issue #717."
 )