From 885865a475054a974132df627283bbe4e76bfe36 Mon Sep 17 00:00:00 2001 From: 40% Date: Thu, 31 Jul 2025 17:00:00 +0800 Subject: [PATCH 01/16] 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. --- src/pyscipopt/scip.pxi | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/pyscipopt/scip.pxi b/src/pyscipopt/scip.pxi index 49f822991..f5a16f89d 100644 --- a/src/pyscipopt/scip.pxi +++ b/src/pyscipopt/scip.pxi @@ -2471,18 +2471,17 @@ cdef class _VarArray: self.size = 1 self.ptr = malloc(sizeof(SCIP_VAR*)) self.ptr[0] = (vars).scip_var - else: - if not isinstance(vars, (list, tuple)): - raise TypeError("Expected Variable or list of Variable, got %s." % type(vars)) + + elif isinstance(vars, (list, tuple)): self.size = len(vars) - if self.size == 0: - self.ptr = NULL - else: - self.ptr = 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] = (var).scip_var + self.ptr = malloc(self.size * sizeof(SCIP_VAR*)) if self.size > 0 else NULL + for i, var in enumerate(vars): + if not isinstance(var, Variable): + raise TypeError(f"Expected Variable, got {type(var)}.") + self.ptr[i] = (var).scip_var + + else: + raise TypeError(f"Expected Variable or list of Variable, got {type(vars)}.") def __dealloc__(self): if self.ptr != NULL: From 9451cc6fdf3685bcb43ba44d33524781fabbae08 Mon Sep 17 00:00:00 2001 From: 40% Date: Thu, 31 Jul 2025 17:10:05 +0800 Subject: [PATCH 02/16] 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. --- src/pyscipopt/scip.pxi | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/src/pyscipopt/scip.pxi b/src/pyscipopt/scip.pxi index f5a16f89d..5c49ad8c3 100644 --- a/src/pyscipopt/scip.pxi +++ b/src/pyscipopt/scip.pxi @@ -2467,19 +2467,23 @@ cdef class _VarArray: cdef int size def __cinit__(self, object vars): - if isinstance(vars, Variable): - self.size = 1 - self.ptr = malloc(sizeof(SCIP_VAR*)) - self.ptr[0] = (vars).scip_var - - elif isinstance(vars, (list, tuple)): - self.size = len(vars) - self.ptr = malloc(self.size * sizeof(SCIP_VAR*)) if self.size > 0 else NULL + def create_ptr(size: int, vars: Union[list, tuple, MatrixVariable]): + ptr = malloc(self.size * sizeof(SCIP_VAR*)) if size > 0 else NULL for i, var in enumerate(vars): if not isinstance(var, Variable): raise TypeError(f"Expected Variable, got {type(var)}.") - self.ptr[i] = (var).scip_var + ptr[i] = (var).scip_var + return ptr + if isinstance(vars, Variable): + self.size = 1 + self.ptr = create_ptr([vars]) + elif isinstance(vars, (list, tuple)): + self.size = len(vars) + self.ptr = create_ptr(vars) + elif isinstance(vars, MatrixVariable): + self.size = vars.size + self.ptr = create_ptr(np.ravel(vars)) else: raise TypeError(f"Expected Variable or list of Variable, got {type(vars)}.") From 7ee3a6f2a2c6ff2d95230e97877c1eaa2ddd7623 Mon Sep 17 00:00:00 2001 From: 40% Date: Thu, 31 Jul 2025 17:19:27 +0800 Subject: [PATCH 03/16] 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. --- tests/test_cons.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/test_cons.py b/tests/test_cons.py index 36a0dfcec..be473c6b7 100644 --- a/tests/test_cons.py +++ b/tests/test_cons.py @@ -179,6 +179,19 @@ def test_cons_indicator(): assert m.isEQ(m.getVal(x), 1) assert c1.getConshdlrName() == "indicator" +def test_cons_indicator_with_matrix_binvar(): + m = Model() + + x = m.addVar(vtype="B") + binvar = m.addMatrixVar(1, vtype="B") + # binvar is a matrix variable to fix #1043 + m.addConsIndicator(x >= 1, binvar) + + m.setObjective(binvar, "maximize") + m.optimize() + + assert m.getVal(x) == 1 + @pytest.mark.xfail( reason="addConsIndicator doesn't behave as expected when binary variable is False. See Issue #717." ) From 2cde7a6865eead09dce33475a898cf5e757ce94a Mon Sep 17 00:00:00 2001 From: 40% Date: Thu, 31 Jul 2025 17:21:47 +0800 Subject: [PATCH 04/16] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7e5101da0..08618f7a9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ ### Fixed - Raised an error when an expression is used when a variable is required - Fixed some compile warnings +- _VarArray can accept MatrixVar ### Changed - MatrixExpr.sum() now supports axis arguments and can return either a scalar or MatrixExpr depending on the result dimensions ### Removed From 50bb995c970590bbed5dfb6adadf9ea30c586575 Mon Sep 17 00:00:00 2001 From: 40% Date: Thu, 31 Jul 2025 17:29:31 +0800 Subject: [PATCH 05/16] Declare ptr variable in create_ptr function To fix "Storing unsafe C derivative of temporary Python reference" --- src/pyscipopt/scip.pxi | 1 + 1 file changed, 1 insertion(+) diff --git a/src/pyscipopt/scip.pxi b/src/pyscipopt/scip.pxi index 5c49ad8c3..752ededc8 100644 --- a/src/pyscipopt/scip.pxi +++ b/src/pyscipopt/scip.pxi @@ -2468,6 +2468,7 @@ cdef class _VarArray: def __cinit__(self, object vars): def create_ptr(size: int, vars: Union[list, tuple, MatrixVariable]): + cdef SCIP_VAR** ptr ptr = malloc(self.size * sizeof(SCIP_VAR*)) if size > 0 else NULL for i, var in enumerate(vars): if not isinstance(var, Variable): From d72f9513602b5e60c4b82982728324ceae70059d Mon Sep 17 00:00:00 2001 From: 40% Date: Thu, 31 Jul 2025 17:43:31 +0800 Subject: [PATCH 06/16] 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. --- src/pyscipopt/scip.pxi | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pyscipopt/scip.pxi b/src/pyscipopt/scip.pxi index 752ededc8..ec8f33ef7 100644 --- a/src/pyscipopt/scip.pxi +++ b/src/pyscipopt/scip.pxi @@ -2467,9 +2467,9 @@ cdef class _VarArray: cdef int size def __cinit__(self, object vars): - def create_ptr(size: int, vars: Union[list, tuple, MatrixVariable]): + def create_ptr(vars: Union[list, tuple, MatrixVariable]): cdef SCIP_VAR** ptr - ptr = malloc(self.size * sizeof(SCIP_VAR*)) if size > 0 else NULL + ptr = malloc(len(vars) * sizeof(SCIP_VAR*)) if len(vars) > 0 else NULL for i, var in enumerate(vars): if not isinstance(var, Variable): raise TypeError(f"Expected Variable, got {type(var)}.") From 110679f28d0a68aa957dbfe2a8eb0195ffdb9a0c Mon Sep 17 00:00:00 2001 From: 40% Date: Thu, 31 Jul 2025 18:42:10 +0800 Subject: [PATCH 07/16] 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. --- src/pyscipopt/scip.pxi | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/src/pyscipopt/scip.pxi b/src/pyscipopt/scip.pxi index ec8f33ef7..fedff5881 100644 --- a/src/pyscipopt/scip.pxi +++ b/src/pyscipopt/scip.pxi @@ -2467,27 +2467,23 @@ cdef class _VarArray: cdef int size def __cinit__(self, object vars): - def create_ptr(vars: Union[list, tuple, MatrixVariable]): - cdef SCIP_VAR** ptr - ptr = malloc(len(vars) * sizeof(SCIP_VAR*)) if len(vars) > 0 else NULL - for i, var in enumerate(vars): - if not isinstance(var, Variable): - raise TypeError(f"Expected Variable, got {type(var)}.") - ptr[i] = (var).scip_var - return ptr - if isinstance(vars, Variable): self.size = 1 - self.ptr = create_ptr([vars]) + vars = [vars] elif isinstance(vars, (list, tuple)): self.size = len(vars) - self.ptr = create_ptr(vars) elif isinstance(vars, MatrixVariable): self.size = vars.size - self.ptr = create_ptr(np.ravel(vars)) + vars = np.ravel(vars) else: raise TypeError(f"Expected Variable or list of Variable, got {type(vars)}.") + self.ptr = malloc(self.size * sizeof(SCIP_VAR*)) if self.size else NULL + for i, var in enumerate(vars): + if not isinstance(var, Variable): + raise TypeError(f"Expected Variable, got {type(var)}.") + self.ptr[i] = (var).scip_var + def __dealloc__(self): if self.ptr != NULL: free(self.ptr) From f181cf47d154a8e394f445a35056b9dc32162613 Mon Sep 17 00:00:00 2001 From: 40% Date: Thu, 31 Jul 2025 18:49:43 +0800 Subject: [PATCH 08/16] Update CHANGELOG --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 08618f7a9..1d2ee2772 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,7 +17,7 @@ ### Fixed - Raised an error when an expression is used when a variable is required - Fixed some compile warnings -- _VarArray can accept MatrixVar +- _VarArray can accept MatrixVariable now ### Changed - MatrixExpr.sum() now supports axis arguments and can return either a scalar or MatrixExpr depending on the result dimensions ### Removed From 0346c52d935ca145e27213b4a7186167b60e9174 Mon Sep 17 00:00:00 2001 From: 40% Date: Thu, 31 Jul 2025 18:50:25 +0800 Subject: [PATCH 09/16] 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(). --- tests/test_cons.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/test_cons.py b/tests/test_cons.py index be473c6b7..74308e5df 100644 --- a/tests/test_cons.py +++ b/tests/test_cons.py @@ -185,9 +185,10 @@ def test_cons_indicator_with_matrix_binvar(): x = m.addVar(vtype="B") binvar = m.addMatrixVar(1, vtype="B") # binvar is a matrix variable to fix #1043 - m.addConsIndicator(x >= 1, binvar) + m.addConsIndicator(x >= 1, binvar, activeone=True) + m.addConsIndicator(x <= 0, binvar, activeone=False) - m.setObjective(binvar, "maximize") + m.setObjective(binvar.sum(), "maximize") m.optimize() assert m.getVal(x) == 1 From 68f2e443049f89371c4937a07dd01400e7e8d235 Mon Sep 17 00:00:00 2001 From: 40% Date: Fri, 1 Aug 2025 09:54:59 +0800 Subject: [PATCH 10/16] ENH: flatten list --- src/pyscipopt/scip.pxi | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/pyscipopt/scip.pxi b/src/pyscipopt/scip.pxi index fedff5881..fdcecaae4 100644 --- a/src/pyscipopt/scip.pxi +++ b/src/pyscipopt/scip.pxi @@ -2474,12 +2474,11 @@ cdef class _VarArray: self.size = len(vars) elif isinstance(vars, MatrixVariable): self.size = vars.size - vars = np.ravel(vars) else: raise TypeError(f"Expected Variable or list of Variable, got {type(vars)}.") self.ptr = malloc(self.size * sizeof(SCIP_VAR*)) if self.size else NULL - for i, var in enumerate(vars): + for i, var in enumerate(np.ravel(vars)): if not isinstance(var, Variable): raise TypeError(f"Expected Variable, got {type(var)}.") self.ptr[i] = (var).scip_var From 5580e12df0ca01cdab8a4e481e61c49454016e52 Mon Sep 17 00:00:00 2001 From: 40% Date: Fri, 1 Aug 2025 15:27:20 +0800 Subject: [PATCH 11/16] BUG: use `np.ravel` avoid to run out of vars range --- src/pyscipopt/scip.pxi | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/pyscipopt/scip.pxi b/src/pyscipopt/scip.pxi index fdcecaae4..4681a6061 100644 --- a/src/pyscipopt/scip.pxi +++ b/src/pyscipopt/scip.pxi @@ -2468,17 +2468,15 @@ cdef class _VarArray: def __cinit__(self, object vars): if isinstance(vars, Variable): - self.size = 1 vars = [vars] - elif isinstance(vars, (list, tuple)): - self.size = len(vars) - elif isinstance(vars, MatrixVariable): - self.size = vars.size + elif isinstance(vars, (list, tuple, MatrixVariable)): + vars = np.ravel(vars) else: raise TypeError(f"Expected Variable or list of Variable, got {type(vars)}.") + self.size = len(vars) self.ptr = malloc(self.size * sizeof(SCIP_VAR*)) if self.size else NULL - for i, var in enumerate(np.ravel(vars)): + for i, var in enumerate(vars): if not isinstance(var, Variable): raise TypeError(f"Expected Variable, got {type(var)}.") self.ptr[i] = (var).scip_var From d52eb541205ad7333d20a5a2f04a05852dd5bb20 Mon Sep 17 00:00:00 2001 From: 40% Date: Sun, 3 Aug 2025 09:12:24 +0800 Subject: [PATCH 12/16] test binvar with (1, 1, 1) matrix --- tests/test_cons.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/test_cons.py b/tests/test_cons.py index 74308e5df..af7e2eff8 100644 --- a/tests/test_cons.py +++ b/tests/test_cons.py @@ -180,11 +180,12 @@ def test_cons_indicator(): assert c1.getConshdlrName() == "indicator" def test_cons_indicator_with_matrix_binvar(): + # test matrix variable binvar #1043 m = Model() x = m.addVar(vtype="B") - binvar = m.addMatrixVar(1, vtype="B") - # binvar is a matrix variable to fix #1043 + # test binvar with (1, 1, 1) shape of matrix variable + binvar = m.addMatrixVar(((1, 1, 1)), vtype="B") m.addConsIndicator(x >= 1, binvar, activeone=True) m.addConsIndicator(x <= 0, binvar, activeone=False) From 8424c6d6015984433c7fb28cd76ae1b222052af7 Mon Sep 17 00:00:00 2001 From: 40% Date: Sun, 3 Aug 2025 09:17:10 +0800 Subject: [PATCH 13/16] test binvar with (2, 3) matrix --- tests/test_cons.py | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/tests/test_cons.py b/tests/test_cons.py index af7e2eff8..355ffd6a5 100644 --- a/tests/test_cons.py +++ b/tests/test_cons.py @@ -183,16 +183,23 @@ def test_cons_indicator_with_matrix_binvar(): # test matrix variable binvar #1043 m = Model() - x = m.addVar(vtype="B") # test binvar with (1, 1, 1) shape of matrix variable - binvar = m.addMatrixVar(((1, 1, 1)), vtype="B") - m.addConsIndicator(x >= 1, binvar, activeone=True) - m.addConsIndicator(x <= 0, binvar, activeone=False) + x = m.addVar(vtype="B") + binvar1 = m.addMatrixVar(((1, 1, 1)), vtype="B") + m.addConsIndicator(x >= 1, binvar1, activeone=True) + m.addConsIndicator(x <= 0, binvar1, activeone=False) + + # test binvar with (2, 3) shape of matrix variable + y = m.addVar(vtype="B") + binvar2 = m.addMatrixVar(((2, 3)), vtype="B") + m.addConsIndicator(y >= 1, binvar2, activeone=True) + m.addConsIndicator(y <= 0, binvar2, activeone=False) - m.setObjective(binvar.sum(), "maximize") + m.setObjective(binvar1.sum() + binvar2.sum(), "maximize") m.optimize() assert m.getVal(x) == 1 + assert m.getVal(y) == 1 @pytest.mark.xfail( reason="addConsIndicator doesn't behave as expected when binary variable is False. See Issue #717." From 710a836c3841a3ce6d73b5bb2585d3e5e356414f Mon Sep 17 00:00:00 2001 From: 40% Date: Sun, 3 Aug 2025 09:19:30 +0800 Subject: [PATCH 14/16] test binvar with (2, 1) list of lists --- tests/test_cons.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/tests/test_cons.py b/tests/test_cons.py index 355ffd6a5..52196b24e 100644 --- a/tests/test_cons.py +++ b/tests/test_cons.py @@ -195,11 +195,20 @@ def test_cons_indicator_with_matrix_binvar(): m.addConsIndicator(y >= 1, binvar2, activeone=True) m.addConsIndicator(y <= 0, binvar2, activeone=False) - m.setObjective(binvar1.sum() + binvar2.sum(), "maximize") + # test binvar with (2, 1) shape of list of lists + z = m.addVar(vtype="B") + binvar3 = [[m.addVar(vtype="B")], [m.addVar(vtype="B")]] + m.addConsIndicator(z >= 1, binvar3, activeone=True) + m.addConsIndicator(z <= 0, binvar3, activeone=False) + + m.setObjective( + binvar1.sum() + binvar2.sum() + binvar3[0][0] + binvar3[0][1], "maximize" + ) m.optimize() assert m.getVal(x) == 1 assert m.getVal(y) == 1 + assert m.getVal(z) == 1 @pytest.mark.xfail( reason="addConsIndicator doesn't behave as expected when binary variable is False. See Issue #717." From 3b64e79548644eac8ef9c4e3bce159de4c98964b Mon Sep 17 00:00:00 2001 From: 40% Date: Sun, 3 Aug 2025 09:34:35 +0800 Subject: [PATCH 15/16] correct index --- tests/test_cons.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_cons.py b/tests/test_cons.py index 52196b24e..c75ab663d 100644 --- a/tests/test_cons.py +++ b/tests/test_cons.py @@ -202,7 +202,7 @@ def test_cons_indicator_with_matrix_binvar(): m.addConsIndicator(z <= 0, binvar3, activeone=False) m.setObjective( - binvar1.sum() + binvar2.sum() + binvar3[0][0] + binvar3[0][1], "maximize" + binvar1.sum() + binvar2.sum() + binvar3[0][0] + binvar3[1][0], "maximize" ) m.optimize() From 2be1117980dbb6a38bcaa99912e372fe4611cb00 Mon Sep 17 00:00:00 2001 From: 40% Date: Mon, 4 Aug 2025 08:55:44 +0800 Subject: [PATCH 16/16] Add TypeError test for addConsIndicator with binvar --- tests/test_cons.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/test_cons.py b/tests/test_cons.py index c75ab663d..756088612 100644 --- a/tests/test_cons.py +++ b/tests/test_cons.py @@ -183,6 +183,9 @@ def test_cons_indicator_with_matrix_binvar(): # test matrix variable binvar #1043 m = Model() + with pytest.raises(TypeError): + m.addConsIndicator(m.addVar(vtype="B") <= 1, 1) + # test binvar with (1, 1, 1) shape of matrix variable x = m.addVar(vtype="B") binvar1 = m.addMatrixVar(((1, 1, 1)), vtype="B")