fix: fix bug in scip freeConstraints and gurobi setConstraints crash (#…

…44)

* fix: fix scip freeConstraints

* fix gurobi error

* remove cov

* change test

* update syntax

ilpy/impl/solvers/GurobiBackend.cpp

@@ -12,7 +12,6 @@
 
 GurobiBackend::GurobiBackend() :
 	_numVariables(0),
-	_numConstraints(0),
 	_env(0),
 	_model(0),
 	_timeout(0),
@@ -49,7 +48,6 @@ GurobiBackend::initialize(
 
 	if (_model) {
 		GRBfreemodel(_model);
-		_numConstraints = 0;
 	}
 	GRB_CHECK(GRBnewmodel(_env, &_model, NULL, 0, NULL, NULL, NULL, NULL, NULL));
 
@@ -132,25 +130,30 @@ GurobiBackend::setObjective(const Objective& objective) {
 }
 
 void
-GurobiBackend::setConstraints(const Constraints& constraints) {
-
-	// delete all previous constraints
-
-	if (_numConstraints > 0) {
-
-		int* constraintIndicies = new int[_numConstraints];
-		for (unsigned int i = 0; i < _numConstraints; i++)
+GurobiBackend::setConstraints(const Constraints &constraints)
+{
+	int numConstrs;
+
+	// Get the number of constraints
+	GRB_CHECK(GRBgetintattr(_model, GRB_INT_ATTR_NUMCONSTRS, &numConstrs));
+
+	// Remove all constraints if there are any
+	if (numConstrs > 0)
+	{
+		int *constraintIndicies = new int[numConstrs];
+		for (unsigned int i = 0; i < numConstrs; i++)
 			constraintIndicies[i] = i;
-		GRB_CHECK(GRBdelconstrs(_model, _numConstraints, constraintIndicies));
+
+		GRB_CHECK(GRBdelconstrs(_model, numConstrs, constraintIndicies));
+		delete[] constraintIndicies;
 
 		GRB_CHECK(GRBupdatemodel(_model));
 	}
 
-	_numConstraints = constraints.size();
-	for (const Constraint& constraint : constraints) {
+	for (const Constraint &constraint : constraints)
 		addConstraint(constraint);
-	}
 
+	// Update the model to include new constraints
 	GRB_CHECK(GRBupdatemodel(_model));
 }
 

ilpy/impl/solvers/GurobiBackend.h

@@ -95,9 +95,6 @@ class GurobiBackend : public SolverBackend {
 	// size of a and x
 	unsigned int _numVariables;
 
-	// number of rows in A and C
-	unsigned int _numConstraints;
-
 	// the GRB environment
 	GRBenv* _env;
 

ilpy/impl/solvers/ScipBackend.cpp

@@ -183,10 +183,12 @@ ScipBackend::addConstraint(const Constraint& constraint) {
 			lhs,
 			rhs));
 
-	_constraints.push_back(c);
 
 	SCIP_CALL_ABORT(SCIPaddCons(_scip, c));
-	SCIP_CALL_ABORT(SCIPreleaseCons(_scip, &c));
+	_constraints.push_back(c);
+	// we do not release the constraint here
+	// so that we can remove the constraints later in freeConstraints()
+	// SCIP_CALL_ABORT(SCIPreleaseCons(_scip, &c));
 }
 
 void
@@ -290,9 +292,21 @@ ScipBackend::freeVariables() {
 }
 
 void
-ScipBackend::freeConstraints() {
+ScipBackend::freeConstraints()
+{
+	// Iterate over all constraints and remove them from the SCIP model
+	for (SCIP_CONS *cons : _constraints)
+	{
+		if (cons != nullptr)
+		{
+			// Remove the constraint from the model
+			SCIP_CALL_ABORT(SCIPdelCons(_scip, cons));
+			// Release the constraint
+			SCIP_CALL_ABORT(SCIPreleaseCons(_scip, &cons));
+		}
+	}
 
-	// SCIPfree should free the constraints for us
+	// Clear the vector of constraint pointers
 	_constraints.clear();
 }
 

tests/test_solvers.py

@@ -199,3 +199,32 @@ def test_solution_indexing() -> None:
     _ = solution[x]  # smoke test
     solution[x] = 2  # can be used to set too
     assert solution[x] == 2
+
+
+@pytest.mark.parametrize("preference", PREFS)
+def test_solve_twice(preference: ilpy.Preference) -> None:
+    solver = ilpy.Solver(2, ilpy.VariableType.Integer, preference=preference)
+    x1 = ilpy.Variable("x1", index=0)
+    x2 = ilpy.Variable("x2", index=1)
+
+    solver.set_objective((x1 + x2).as_objective(ilpy.Maximize))
+
+    c0 = ilpy.Constraints()
+    c0.add(x1 + x2 <= 10)
+    c0.add(2 * x1 + 3 * x2 >= 12)
+    c0.add(x1 - x2 <= 5)
+
+    c1 = ilpy.Constraints()
+    c1.add(x1 + x2 <= 10)
+    c1.add(2 * x1 + 3 * x2 >= 12)
+    c1.add(x1 - x2 >= 5)
+
+    # initial solve
+    solver.set_constraints(c0)
+    solution = solver.solve()
+    assert list(solution) == [7, 3] and solution.get_value() == 10
+
+    # add a constraint and check that the solution has changed
+    solver.set_constraints(c1)
+    solution = solver.solve()
+    assert list(solution) != [7, 3]