coin-or / python-mip

Python-MIP: collection of Python tools for the modeling and solution of Mixed-Integer Linear programs
Eclipse Public License 2.0
518 stars 92 forks source link

CBC: Unable to modify a constraint #158

Closed VJMCallewaert closed 2 years ago

VJMCallewaert commented 3 years ago

Describe the bug Using the cbc solver, there is seems to be no convenient way to modify a constraint. The available methods silently fail to do what you would expect.

To Reproduce

import mip

m = mip.Model()
a = m.add_var(var_type=mip.BINARY, name='a')
b = m.add_var(var_type=mip.BINARY, name='b')
m += a + b <= 1
constr_1 = m.constrs[0]
print(m.constrs[0]) # constr(0): +1.0 a +1.0 b <= 1.0

# Attempt 1: Fails because SolverCbc.constr_get_expr() returns a new `LinExpr` that is 
# modified, which gets modified with `add_term`. constr_1 is left unmodified.
constr_1.expr.add_term(a, 100) 
print(constr_1) # constr(0): +1.0 a +1.0 b <= 1.0

# Attempt 2: Fails as `SolverCbc.constr_set_expr` called by the `Model.expr` setter is 
# not implemented.
expr = constr_1.expr
expr.add_term(a, 100)
print(expr) # + 101.0a + b  <= 1.0
constr_1.expr = expr # No effect.
print(constr_1) # constr(0): +1.0 a +1.0 b <= 1.0

# Attempt 3: Only one that seems work. Add a new constraint and remove the old one.
constr_1 = m.constrs[0]
constr_2 = m.add_constr(constr_1.expr + 100 * a)
m.remove(constr_1)
for c in m.constrs: # constr(1): +101.0 A +1.0 B <= 1.0
    print(c)

When running the code in an IPython shell, the removal of constraints sometimes fails with error: Invalid row index (-1), valid range is [0,1). At /home/travis/build/coin-or/Cbc/src/Cbc_C_Interface.cpp:1598

Expected behavior

  1. Both of the above approaches 1 & 2 are expected to modify the constraint in the model.
  2. SolverCbc.constr_set_expr should maybe raise a NotImplementedError instead of silently doing notthing?
  3. It is unclear why the removal of constraints fails sometimes.

Desktop (please complete the following information):

h-g-s commented 3 years ago

Hi, one of the reasons for the efficiency of storing large Matrices in CBC is that they are stored compactly (one row adjacent to another); to add a term to one expression would involve a very expensive operation, possibly moving the entire matrix. The recommended way of doing it is:

  1. remove in batch (passing a list of rows) all rows that will be modified
  2. add the new version of the rows
peterlietz commented 3 years ago

Hi,

but wouldn't raising a NotImplementedError be preferable to silently ignoring the user's instructions? I just fell into the same trap.

Kind regards Peter

sebheger commented 2 years ago

Closed as a duplicate to #123