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
513 stars 88 forks source link

`add_constr` returns broken object when adding constraint without variables #364

Open mjdv opened 5 months ago

mjdv commented 5 months ago

Describe the bug You can pass a trivial constraint, like xsum([]) == 0 or xsum([]) == 1, to a Model object's add_constr function. This returns an object which is broken in some way (using it causes errors or even segmentation faults).

To Reproduce The simplest code that produces the bug is

from mip import *

model = Model()
print(model.add_constr(xsum([])) == 0)

This produces an error:

Traceback (most recent call last):
  File "/home/mees/minimal_example.py", line 4, in <module>
    print(model.add_constr(xsum([])) == 0)
  File "/home/mees/.local/lib/python3.10/site-packages/mip/model.py", line 339, in add_constr
    return self.constrs.add(lin_expr, name, priority)
  File "/home/mees/.local/lib/python3.10/site-packages/mip/lists.py", line 153, in add
    self.__model.solver.add_constr(lin_expr, name)
  File "/home/mees/.local/lib/python3.10/site-packages/mip/cbc.py", line 1429, in add_constr
    cbclib.Cbc_addRow(mp, namestr, numnz, self.iidx, self.dvec, sense, rhs)
TypeError: initializer for ctype 'char' must be a bytes of length 1, not bytes

Other actions on the object also cause errors. For instance, the following code also fails:

from mip import *

model = Model()
constraint = model.add_constr(xsum([]) == 0)
model.remove(constraint)

In this case, the error is a segmentation fault, although in context of a larger program I have also seen other errors.

Expected behavior Ideally, these dummy constraints should just be accepted and added to the model, be printable and removable, etc.

If this is not possible due to the nature of the underlying solver(s), the add_constr should give a useful error when such a constraint is added to it, rather than return a broken object.

Desktop (please complete the following information):

Additional context Of course it is unusual to add dummy constraints like the one I did above. In my use case, I am modelling a dynamic logical problem, and regularly want to check for consistency of the full model. In particular, I have lists of binary variables that may grow or shrink during the computation, and regularly want to check if it is still consistent for there to be a variable in some list with value 1. Occasionally these lists are empty, which results in a constraint like xsum([]) == 1. It would be convenient if that did not present a special case.

rschwarz commented 5 months ago

This is a known problem, see, eg, #184.

The problem is that the solvers (here, CBC?) don't actually add a row to their internal representation, which means that python-mip and the solver no longer have a consistent understanding of the number of constraints, leading to out-of-bounds indexing.

It can be even worse, when you add constraints that are empty, not only after simple preprocessing is applied (eg 0*x == 0), because it means that the constraint is dropped only during the actual solve call.

flipz357 commented 5 months ago

@rschwarz But then, wouldn't this suggest a solution of python-mip simply issuing a warning that there's an empty constraint and it's gonna be removed, then it removes these constraints, and then there will be no inconsistency any more between the CBC(?) and python-mip?

rschwarz commented 5 months ago

Yes, I think this is done in the PR https://github.com/coin-or/python-mip/pull/237.

One could go further and also check for expressions that have some terms in them, but with coefficients 0.0, which might be removed later by the solver. We have sometimes checked for this manually before adding constraints, but it turns out that this can take significant time, so it's not always a good default behaviour.