From e049bc00b94708ee733e157df2f09fd3e74c19c5 Mon Sep 17 00:00:00 2001 From: Jonas Charfreitag Date: Mon, 24 Aug 2020 14:14:27 +0200 Subject: [PATCH 1/4] Prototype for addition of addExprConsNode to add ExprCons to Nodes. --- src/pyscipopt/scip.pyx | 78 +++++++++++++++++++++++++++++++++++------- 1 file changed, 66 insertions(+), 12 deletions(-) diff --git a/src/pyscipopt/scip.pyx b/src/pyscipopt/scip.pyx index 21ade0321..3faa8b4fc 100644 --- a/src/pyscipopt/scip.pyx +++ b/src/pyscipopt/scip.pyx @@ -1961,18 +1961,8 @@ cdef class Model: """ assert isinstance(cons, ExprCons), "given constraint is not ExprCons but %s" % cons.__class__.__name__ - # replace empty name with generic one - if name == '': - name = 'c'+str(SCIPgetNConss(self._scip)+1) - - kwargs = dict(name=name, initial=initial, separate=separate, - enforce=enforce, check=check, - propagate=propagate, local=local, - modifiable=modifiable, dynamic=dynamic, - removable=removable, - stickingatnode=stickingatnode) - kwargs['lhs'] = -SCIPinfinity(self._scip) if cons._lhs is None else cons._lhs - kwargs['rhs'] = SCIPinfinity(self._scip) if cons._rhs is None else cons._rhs + kwargs = self._cons_kwargs(check, cons, dynamic, enforce, initial, local, modifiable, name, + propagate, removable, separate,stickingatnode) deg = cons.expr.degree() if deg <= 1: @@ -1984,6 +1974,21 @@ cdef class Model: else: return self._addNonlinearCons(cons, **kwargs) + def _cons_kwargs(self, check, cons, dynamic, enforce, initial, local, modifiable, name, propagate, removable, separate, + stickingatnode): + # replace empty name with generic one + if name == '': + name = 'c' + str(SCIPgetNConss(self._scip) + 1) + kwargs = dict(name=name, initial=initial, separate=separate, + enforce=enforce, check=check, + propagate=propagate, local=local, + modifiable=modifiable, dynamic=dynamic, + removable=removable, + stickingatnode=stickingatnode) + kwargs['lhs'] = -SCIPinfinity(self._scip) if cons._lhs is None else cons._lhs + kwargs['rhs'] = SCIPinfinity(self._scip) if cons._rhs is None else cons._rhs + return kwargs + def _addLinCons(self, ExprCons lincons, **kwargs): assert isinstance(lincons, ExprCons), "given constraint is not ExprCons but %s" % lincons.__class__.__name__ @@ -2226,6 +2231,55 @@ cdef class Model: PY_SCIP_CALL(SCIPaddConsNode(self._scip, node.scip_node, cons.scip_cons, NULL)) Py_INCREF(cons) + def addExprConsNode(self, Node node, ExprCons cons, Node validnode=None, name="", initial=True, separate=True, + enforce=False, check=False, propagate=True, local=True, + modifiable=False, dynamic=False, removable=False, + stickingatnode=True): + """Add a linear constraint to the given node + + :param Node node: node to add the constraint to + :param Constraint cons: constraint to add + :param Node validnode: more global node where cons is also valid + + """ + kwargs = self._cons_kwargs(check, cons, dynamic, enforce, initial, local, modifiable, name, + propagate, removable, separate,stickingatnode) + deg = cons.expr.degree() + assert deg <= 1, f"Given constraint is not linear, but has degree {deg}" + + terms = cons.expr.terms + + cdef SCIP_CONS* scip_cons + cdef int nvars = len(terms.items()) + + vars_array = malloc(nvars * sizeof(SCIP_VAR*)) + coeffs_array = malloc(nvars * sizeof(SCIP_Real)) + + for i, (key, coeff) in enumerate(terms.items()): + vars_array[i] = (key[0]).scip_var + coeffs_array[i] = coeff + + PY_SCIP_CALL(SCIPcreateConsLinear( + self._scip, &scip_cons, str_conversion(kwargs['name']), nvars, vars_array, coeffs_array, + kwargs['lhs'], kwargs['rhs'], kwargs['initial'], + kwargs['separate'], kwargs['enforce'], kwargs['check'], + kwargs['propagate'], kwargs['local'], kwargs['modifiable'], + kwargs['dynamic'], kwargs['removable'], kwargs['stickingatnode'])) + + + if isinstance(validnode, Node): + PY_SCIP_CALL(SCIPaddConsNode(self._scip, node.scip_node, scip_cons, validnode.scip_node)) + else: + PY_SCIP_CALL(SCIPaddConsNode(self._scip, node.scip_node, scip_cons, NULL)) + + py_cons = Constraint.create(scip_cons) + PY_SCIP_CALL(SCIPreleaseCons(self._scip, &scip_cons)) + + free(vars_array) + free(coeffs_array) + + return py_cons + def addConsLocal(self, Constraint cons, Node validnode=None): """Add a constraint to the current node From 9394b49b4d809342d53e00419df6820d155337eb Mon Sep 17 00:00:00 2001 From: Jonas Charfreitag Date: Tue, 25 Aug 2020 18:11:46 +0200 Subject: [PATCH 2/4] Extract _createLinCons --- src/pyscipopt/scip.pyx | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/src/pyscipopt/scip.pyx b/src/pyscipopt/scip.pyx index 3faa8b4fc..641bae0c4 100644 --- a/src/pyscipopt/scip.pyx +++ b/src/pyscipopt/scip.pyx @@ -1989,7 +1989,7 @@ cdef class Model: kwargs['rhs'] = SCIPinfinity(self._scip) if cons._rhs is None else cons._rhs return kwargs - def _addLinCons(self, ExprCons lincons, **kwargs): + def _createLinCons(self, ExprCons lincons, **kwargs): assert isinstance(lincons, ExprCons), "given constraint is not ExprCons but %s" % lincons.__class__.__name__ assert lincons.expr.degree() <= 1, "given constraint is not linear, degree == %d" % lincons.expr.degree() @@ -2012,15 +2012,22 @@ cdef class Model: kwargs['separate'], kwargs['enforce'], kwargs['check'], kwargs['propagate'], kwargs['local'], kwargs['modifiable'], kwargs['dynamic'], kwargs['removable'], kwargs['stickingatnode'])) - - PY_SCIP_CALL(SCIPaddCons(self._scip, scip_cons)) - PyCons = Constraint.create(scip_cons) - PY_SCIP_CALL(SCIPreleaseCons(self._scip, &scip_cons)) - + #py_cons = free(vars_array) free(coeffs_array) + #PY_SCIP_CALL(SCIPaddCons(self._scip, scip_cons)) + #PY_SCIP_CALL(SCIPreleaseCons(self._scip, &scip_cons)) - return PyCons + return Constraint.create(scip_cons) + + def _addLinCons(self, ExprCons lincons, **kwargs): + cdef Constraint py_cons + py_cons = self._createLinCons(lincons, **kwargs) + cdef SCIP_CONS* scip_cons + scip_cons = py_cons.scip_cons + PY_SCIP_CALL(SCIPaddCons(self._scip, scip_cons)) + PY_SCIP_CALL(SCIPreleaseCons(self._scip, &scip_cons)) + return py_cons def _addQuadCons(self, ExprCons quadcons, **kwargs): terms = quadcons.expr.terms From 9684575afde7d295be6d07495b82fd4465e37e77 Mon Sep 17 00:00:00 2001 From: Jonas Charfreitag Date: Tue, 25 Aug 2020 19:09:53 +0200 Subject: [PATCH 3/4] Unify createCons and make addConsNode more compact. --- src/pyscipopt/scip.pyx | 134 +++++++++++++++-------------------------- 1 file changed, 49 insertions(+), 85 deletions(-) diff --git a/src/pyscipopt/scip.pyx b/src/pyscipopt/scip.pyx index 641bae0c4..57f0b859a 100644 --- a/src/pyscipopt/scip.pyx +++ b/src/pyscipopt/scip.pyx @@ -1963,16 +1963,13 @@ cdef class Model: kwargs = self._cons_kwargs(check, cons, dynamic, enforce, initial, local, modifiable, name, propagate, removable, separate,stickingatnode) - - deg = cons.expr.degree() - if deg <= 1: - return self._addLinCons(cons, **kwargs) - elif deg <= 2: - return self._addQuadCons(cons, **kwargs) - elif deg == float('inf'): # general nonlinear - return self._addGenNonlinearCons(cons, **kwargs) - else: - return self._addNonlinearCons(cons, **kwargs) + cdef Constraint py_cons + py_cons = self._createCons(cons, **kwargs) + cdef SCIP_CONS* scip_cons + scip_cons = py_cons.scip_cons + PY_SCIP_CALL(SCIPaddCons(self._scip, scip_cons)) + PY_SCIP_CALL(SCIPreleaseCons(self._scip, &scip_cons)) + return py_cons def _cons_kwargs(self, check, cons, dynamic, enforce, initial, local, modifiable, name, propagate, removable, separate, stickingatnode): @@ -1989,9 +1986,23 @@ cdef class Model: kwargs['rhs'] = SCIPinfinity(self._scip) if cons._rhs is None else cons._rhs return kwargs + def _createCons(self, cons, **kwargs): + deg = cons.expr.degree() + cdef Constraint py_cons + + if deg <= 1: + py_cons = self._createLinCons(cons, **kwargs) + elif deg <= 2: + py_cons = self._createQuadCons(cons, **kwargs) + elif deg == float('inf'): # general nonlinear + return self._createGenNonlinearCons(cons, **kwargs) + else: + return self._createNonlinearCons(cons, **kwargs) + return py_cons + + def _createLinCons(self, ExprCons lincons, **kwargs): assert isinstance(lincons, ExprCons), "given constraint is not ExprCons but %s" % lincons.__class__.__name__ - assert lincons.expr.degree() <= 1, "given constraint is not linear, degree == %d" % lincons.expr.degree() terms = lincons.expr.terms @@ -2012,24 +2023,12 @@ cdef class Model: kwargs['separate'], kwargs['enforce'], kwargs['check'], kwargs['propagate'], kwargs['local'], kwargs['modifiable'], kwargs['dynamic'], kwargs['removable'], kwargs['stickingatnode'])) - #py_cons = free(vars_array) free(coeffs_array) - #PY_SCIP_CALL(SCIPaddCons(self._scip, scip_cons)) - #PY_SCIP_CALL(SCIPreleaseCons(self._scip, &scip_cons)) return Constraint.create(scip_cons) - def _addLinCons(self, ExprCons lincons, **kwargs): - cdef Constraint py_cons - py_cons = self._createLinCons(lincons, **kwargs) - cdef SCIP_CONS* scip_cons - scip_cons = py_cons.scip_cons - PY_SCIP_CALL(SCIPaddCons(self._scip, scip_cons)) - PY_SCIP_CALL(SCIPreleaseCons(self._scip, &scip_cons)) - return py_cons - - def _addQuadCons(self, ExprCons quadcons, **kwargs): + def _createQuadCons(self, ExprCons quadcons, **kwargs): terms = quadcons.expr.terms assert quadcons.expr.degree() <= 2, "given constraint is not quadratic, degree == %d" % quadcons.expr.degree() @@ -2052,12 +2051,9 @@ cdef class Model: var1, var2 = v[0], v[1] PY_SCIP_CALL(SCIPaddBilinTermQuadratic(self._scip, scip_cons, var1.scip_var, var2.scip_var, c)) - PY_SCIP_CALL(SCIPaddCons(self._scip, scip_cons)) - PyCons = Constraint.create(scip_cons) - PY_SCIP_CALL(SCIPreleaseCons(self._scip, &scip_cons)) - return PyCons + return Constraint.create(scip_cons) - def _addNonlinearCons(self, ExprCons cons, **kwargs): + def _createNonlinearCons(self, ExprCons cons, **kwargs): cdef SCIP_EXPR* expr cdef SCIP_EXPR** varexprs cdef SCIP_EXPRDATA_MONOMIAL** monomials @@ -2110,16 +2106,13 @@ cdef class Model: kwargs['check'], kwargs['propagate'], kwargs['local'], kwargs['modifiable'], kwargs['dynamic'], kwargs['removable'], kwargs['stickingatnode']) ) - PY_SCIP_CALL(SCIPaddCons(self._scip, scip_cons)) - PyCons = Constraint.create(scip_cons) - PY_SCIP_CALL(SCIPreleaseCons(self._scip, &scip_cons)) PY_SCIP_CALL( SCIPexprtreeFree(&exprtree) ) free(vars) free(monomials) free(varexprs) - return PyCons + return Constraint.create(scip_cons) - def _addGenNonlinearCons(self, ExprCons cons, **kwargs): + def _createGenNonlinearCons(self, ExprCons cons, **kwargs): cdef SCIP_EXPR** childrenexpr cdef SCIP_EXPR** scipexprs cdef SCIP_EXPRTREE* exprtree @@ -2203,16 +2196,14 @@ cdef class Model: kwargs['check'], kwargs['propagate'], kwargs['local'], kwargs['modifiable'], kwargs['dynamic'], kwargs['removable'], kwargs['stickingatnode']) ) - PY_SCIP_CALL(SCIPaddCons(self._scip, scip_cons)) PyCons = Constraint.create(scip_cons) - PY_SCIP_CALL(SCIPreleaseCons(self._scip, &scip_cons)) PY_SCIP_CALL( SCIPexprtreeFree(&exprtree) ) # free more memory free(scipexprs) free(vars) - return PyCons + return Constraint.create(scip_cons) def addConsCoeff(self, Constraint cons, Variable var, coeff): """Add coefficient to the linear constraint (if non-zero). @@ -2224,68 +2215,41 @@ cdef class Model: """ PY_SCIP_CALL(SCIPaddCoefLinear(self._scip, cons.scip_cons, var.scip_var, coeff)) - def addConsNode(self, Node node, Constraint cons, Node validnode=None): + def addConsNode(self, Node node, cons, Node validnode=None, name="", initial=True, separate=True, + enforce=False, check=False, propagate=True, local=True, + modifiable=False, dynamic=False, removable=False, stickingatnode=True): """Add a constraint to the given node :param Node node: node to add the constraint to :param Constraint cons: constraint to add :param Node validnode: more global node where cons is also valid - + Note: Additional parameter for constraint settings get ignored if cons is not an ExprCons! """ - if isinstance(validnode, Node): - PY_SCIP_CALL(SCIPaddConsNode(self._scip, node.scip_node, cons.scip_cons, validnode.scip_node)) - else: - PY_SCIP_CALL(SCIPaddConsNode(self._scip, node.scip_node, cons.scip_cons, NULL)) - Py_INCREF(cons) - - def addExprConsNode(self, Node node, ExprCons cons, Node validnode=None, name="", initial=True, separate=True, - enforce=False, check=False, propagate=True, local=True, - modifiable=False, dynamic=False, removable=False, - stickingatnode=True): - """Add a linear constraint to the given node - - :param Node node: node to add the constraint to - :param Constraint cons: constraint to add - :param Node validnode: more global node where cons is also valid - - """ - kwargs = self._cons_kwargs(check, cons, dynamic, enforce, initial, local, modifiable, name, - propagate, removable, separate,stickingatnode) - deg = cons.expr.degree() - assert deg <= 1, f"Given constraint is not linear, but has degree {deg}" - - terms = cons.expr.terms - + cdef Constraint py_cons cdef SCIP_CONS* scip_cons - cdef int nvars = len(terms.items()) - - vars_array = malloc(nvars * sizeof(SCIP_VAR*)) - coeffs_array = malloc(nvars * sizeof(SCIP_Real)) - - for i, (key, coeff) in enumerate(terms.items()): - vars_array[i] = (key[0]).scip_var - coeffs_array[i] = coeff - - PY_SCIP_CALL(SCIPcreateConsLinear( - self._scip, &scip_cons, str_conversion(kwargs['name']), nvars, vars_array, coeffs_array, - kwargs['lhs'], kwargs['rhs'], kwargs['initial'], - kwargs['separate'], kwargs['enforce'], kwargs['check'], - kwargs['propagate'], kwargs['local'], kwargs['modifiable'], - kwargs['dynamic'], kwargs['removable'], kwargs['stickingatnode'])) - + if isinstance(cons, Constraint): + py_cons = cons + scip_cons = py_cons.scip_cons + # TODO: (Why) Is this INCREF necessary? + Py_INCREF(cons) + elif isinstance(cons, ExprCons): + kwargs = self._cons_kwargs(check, cons, dynamic, enforce, initial, local, modifiable, name, + propagate, removable, separate,stickingatnode) + py_cons = self._createCons(cons, **kwargs) + scip_cons = py_cons.scip_cons + PY_SCIP_CALL(SCIPreleaseCons(self._scip, &scip_cons)) + else: + raise Warning(f"Argument cons is of incorrect type ({type(cons)}).") if isinstance(validnode, Node): PY_SCIP_CALL(SCIPaddConsNode(self._scip, node.scip_node, scip_cons, validnode.scip_node)) - else: + elif validnode is None: PY_SCIP_CALL(SCIPaddConsNode(self._scip, node.scip_node, scip_cons, NULL)) + else: + raise Warning(f"Argument validnode is of incorrect type ({type(validnode)}).") - py_cons = Constraint.create(scip_cons) - PY_SCIP_CALL(SCIPreleaseCons(self._scip, &scip_cons)) - free(vars_array) - free(coeffs_array) - return py_cons def addConsLocal(self, Constraint cons, Node validnode=None): """Add a constraint to the current node From 744b2bb0b623b11e453b4df90b5608ba84ec84f5 Mon Sep 17 00:00:00 2001 From: Jonas Charfreitag Date: Tue, 25 Aug 2020 19:20:37 +0200 Subject: [PATCH 4/4] Fix segfault caused by addConsNode when called with ExprCons --- src/pyscipopt/scip.pyx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/pyscipopt/scip.pyx b/src/pyscipopt/scip.pyx index 57f0b859a..37937b0ae 100644 --- a/src/pyscipopt/scip.pyx +++ b/src/pyscipopt/scip.pyx @@ -2237,7 +2237,6 @@ cdef class Model: propagate, removable, separate,stickingatnode) py_cons = self._createCons(cons, **kwargs) scip_cons = py_cons.scip_cons - PY_SCIP_CALL(SCIPreleaseCons(self._scip, &scip_cons)) else: raise Warning(f"Argument cons is of incorrect type ({type(cons)}).") @@ -2247,8 +2246,8 @@ cdef class Model: PY_SCIP_CALL(SCIPaddConsNode(self._scip, node.scip_node, scip_cons, NULL)) else: raise Warning(f"Argument validnode is of incorrect type ({type(validnode)}).") - - + # TODO: This is needed for the ExprCons case. Is it a problem for the Constraint case? + PY_SCIP_CALL(SCIPreleaseCons(self._scip, &scip_cons)) def addConsLocal(self, Constraint cons, Node validnode=None):