Opened 7 years ago
Last modified 20 months ago
#19523 new defect
Raise an error when constraints are added to the wrong MILP
Reported by: | Jeroen Demeyer | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-7.3 |
Component: | linear programming | Keywords: | |
Cc: | Yuan Zhou | Merged in: | |
Authors: | Reviewers: | ||
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
sage: p = MixedIntegerLinearProgram(solver="glpk") sage: q = MixedIntegerLinearProgram(solver="glpk") 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
Before #19525, this would instead crash Sage. A crash still happens with COIN (#20360).
This ticket is to actually fix the error completely or give a better error message.
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 (19)
comment:1 Changed 7 years ago by
Dependencies: | → #19525 |
---|---|
Description: | modified (diff) |
comment:2 Changed 7 years ago by
Cc: | Nathann Cohen added |
---|
comment:3 Changed 7 years ago by
Description: | modified (diff) |
---|---|
Priority: | critical → major |
comment:4 Changed 7 years ago by
comment:5 Changed 7 years ago by
Description: | modified (diff) |
---|
comment:6 Changed 7 years ago by
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:8 Changed 7 years ago by
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: 10 Changed 7 years ago by
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.
comment:10 Changed 7 years ago by
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 7 years ago by
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 MixedIntegerLinearProgram
s, or to use LinearFunction
s directly somehow (without referring to the correct MIP's variables).
comment:12 Changed 6 years ago by
Dependencies: | #19525 → #19525, #20360 |
---|---|
Description: | modified (diff) |
comment:13 Changed 6 years ago by
Summary: | Adding constraints for the wrong MILP crashes Sage → Raise an error when constraints are added to the wrong MILP |
---|
Since the crashes are now on separate tickets, I have changed the title of this ticket.
comment:14 Changed 6 years ago by
See also #15159, which raises a similar question when q
started out as a copy of p
.
comment:15 follow-up: 19 Changed 6 years ago by
A solution could be to change LinearFunctionsParent
etc. so that it not only has remembers the base_ring
but also the MIP.
comment:16 Changed 6 years ago by
Milestone: | sage-6.10 → sage-7.3 |
---|
comment:17 Changed 5 years ago by
Cc: | Nathann Cohen removed |
---|---|
Dependencies: | #19525, #20360 |
Description: | modified (diff) |
comment:18 Changed 5 years ago by
Description: | modified (diff) |
---|
comment:19 Changed 20 months ago by
Cc: | Yuan Zhou added |
---|
Replying to mkoeppe:
A solution could be to change
LinearFunctionsParent
etc. so that it not only has remembers thebase_ring
but also the MIP.
In contrast the example in the ticket description, no error is raised in the following examples, which is very confusing.
sage: p = MixedIntegerLinearProgram(solver="glpk") sage: q = MixedIntegerLinearProgram(solver="glpk") sage: v = q.new_variable() sage: q.add_constraint(v[0] <= 1)
and
sage: p = MixedIntegerLinearProgram(solver='InteractiveLP') sage: v = p.new_variable() sage: x,y = v[0], v[1] sage: pp = MixedIntegerLinearProgram(solver='InteractiveLP') sage: vv = pp.new_variable() sage: xx,yy = vv[0], vv[1] sage: p.add_constraint(x + y, max = 10) sage: p.add_constraint(xx + 3*yy, max = 15) sage: p.set_objective(x + 3*y) sage: p.solve() 15 sage: p.show() Maximization: x1 + 3 x2 Constraints: x1 + x2 <= 10 x1 + 3 x2 <= 15 Variables: x1 = x_0 is a continuous variable (min=-oo, max=+oo) x2 = x_1 is a continuous variable (min=-oo, max=+oo)
It would be nice to have LinearFunctionsParent
etc. remember its MIP.
While
MIPVariable
objects know which MIP they belong to, their "components" gotten byp[0]
etc. are elements ofLinearFunctionsParent(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.