Opened 4 years ago

Last modified 2 years ago

#19523 new defect

Adding constraints for the wrong MILP crashes Sage — at Version 12

Reported by: jdemeyer Owned by:
Priority: major Milestone: sage-7.3
Component: linear programming Keywords:
Cc: Merged in:
Authors: Reviewers:
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: #19525, #20360 Stopgaps:

Description (last modified by mkoeppe)

sage: p = MixedIntegerLinearProgram(solver="glpk")
sage: q = MixedIntegerLinearProgram(solver="glpk")
sage: q.add_constraint(p[0] <= 1)
------------------------------------------------------------------------
Unhandled SIGABRT: An abort() occurred in Sage.
This probably occurred because a *compiled* component of Sage has a bug
in it and is not properly wrapped with sig_on(), sig_off().
Sage will now terminate.
------------------------------------------------------------------------

With #19525, this improves to not crashing Sage:

sage: sage: p = MixedIntegerLinearProgram(solver="glpk")
sage: sage: q = MixedIntegerLinearProgram(solver="glpk")
sage: sage: q.add_constraint(p[0] <= 1)
---------------------------------------------------------------------------
GLPKError                                 Traceback (most recent call last)
...
GLPKError: glp_set_mat_row: i = 1; len = 1; invalid row length 
Error detected in file glpapi01.c at line 760

This ticket is to actually fix the error completely or give a better error message.

Also a crash with the COIN backend (#20360).

And low-level error message with the PPL and InteractiveLP (#20296 ) backends.

The CVX backend silently adds a new variable that is accessible only to the backend:.

sage: sage: default_mip_solver("cvxopt")
sage: sage: p = MixedIntegerLinearProgram()
sage: sage: q = MixedIntegerLinearProgram()
sage: sage: q.add_constraint(p[0] <= 1)
sage: q.number_of_variables()
1

Change History (12)

comment:1 Changed 4 years ago by jdemeyer

  • Dependencies set to #19525
  • Description modified (diff)

comment:2 Changed 4 years ago by ncohen

  • Cc ncohen added

comment:3 Changed 4 years ago by jdemeyer

  • Description modified (diff)
  • Priority changed from critical to major

comment:4 Changed 4 years ago by mkoeppe

While MIPVariable objects know which MIP they belong to, their "components" gotten by p[0] etc. are elements of LinearFunctionsParent(base_ring), and do not remember their MIP (nor even their name). So with the current design it does not seem possible to catch the error of adding constraints to the wrong MIP. So unfortunately only lower-level error checking, catching out-of-bounds indices, is possible.

comment:5 Changed 4 years ago by mkoeppe

  • Description modified (diff)

comment:6 Changed 4 years ago by jdemeyer

Given that p[0] is just a linear function, it would make the most sense if q.add_constraint(p[0] <= 1) would simply work, not give an error message.

comment:7 follow-up: Changed 4 years ago by mkoeppe

No, q has no variables at this point.

comment:8 in reply to: ↑ 7 Changed 4 years ago by jdemeyer

Replying to mkoeppe:

No, q has no variables at this point.

That's exactly my point: the variables should be added by q.add_constraint().

comment:9 follow-up: Changed 4 years ago by mkoeppe

I'm not sure that this would be a good interface. It would allow to add variables, but only variables with default settings (continuous, lower bound 0, no upper bound, no name). I think it's better to raise an error, which could help spot programming mistakes -- at least when there's a dimension mismatch between the two problems. This bound checking should be done by the backends -- see #10232.

Last edited 4 years ago by mkoeppe (previous) (diff)

comment:10 in reply to: ↑ 9 Changed 4 years ago by jdemeyer

Replying to mkoeppe:

at least when there's a dimension mismatch between the two problems.

No. It should be always an error, or never an error. If it's only an error "when there's a dimension mismatch between the two problems", that will be more confusing than either extreme.

comment:11 Changed 4 years ago by mkoeppe

Then I strongly prefer to always raise an error in this situation. This requires changes to LinearFunction (a class that is I think only used in the context of MixedIntegerLinearProgram), so that it remembers the MIP that it relates to.

The mapping from MIP variables (and their indexed components) to integer indices (designating backend columns) is determined dynamically, adding a backend column when a MIP variable component is accessed. Because of this there's simply no good way to write correct code that interchanges MIP variables between two MixedIntegerLinearPrograms, or to use LinearFunctions directly somehow (without referring to the correct MIP's variables).

Last edited 4 years ago by mkoeppe (previous) (diff)

comment:12 Changed 4 years ago by mkoeppe

  • Dependencies changed from #19525 to #19525, #20360
  • Description modified (diff)
Note: See TracTickets for help on using tickets.