Sage: Ticket #19523: Raise an error when constraints are added to the wrong MILP
https://trac.sagemath.org/ticket/19523
<pre class="wiki">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
</pre><p>
Before <a class="closed ticket" href="https://trac.sagemath.org/ticket/19525" title="enhancement: Improve GLPK error handling (closed: fixed)">#19525</a>, this would instead crash Sage. A crash still happens with COIN (<a class="new ticket" href="https://trac.sagemath.org/ticket/20360" title="enhancement: Add sig_on/sig_off to COINBackend (new)">#20360</a>).
</p>
<p>
This ticket is to actually fix the error completely or give a better error message.
</p>
<p>
And low-level error message with the PPL and InteractiveLP (<a class="closed ticket" href="https://trac.sagemath.org/ticket/20296" title="enhancement: MixedIntegerLinearProgram: New backend using InteractiveLPProblem (closed: fixed)">#20296</a> ) backends.
</p>
<p>
The CVX backend silently adds a new variable that is accessible only to the backend:.
</p>
<pre class="wiki">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
</pre>en-usSagehttps://trac.sagemath.org/chrome/site/logo_sagemath_trac.png
https://trac.sagemath.org/ticket/19523
Trac 1.1.6jdemeyerWed, 04 Nov 2015 16:39:31 GMTdescription changed; dependencies set
https://trac.sagemath.org/ticket/19523#comment:1
https://trac.sagemath.org/ticket/19523#comment:1
<ul>
<li><strong>dependencies</strong>
set to <em>#19525</em>
</li>
<li><strong>description</strong>
modified (<a href="/ticket/19523?action=diff&version=1">diff</a>)
</li>
</ul>
TicketncohenWed, 04 Nov 2015 21:12:45 GMTcc set
https://trac.sagemath.org/ticket/19523#comment:2
https://trac.sagemath.org/ticket/19523#comment:2
<ul>
<li><strong>cc</strong>
<em>ncohen</em> added
</li>
</ul>
TicketjdemeyerThu, 05 Nov 2015 10:13:02 GMTpriority, description changed
https://trac.sagemath.org/ticket/19523#comment:3
https://trac.sagemath.org/ticket/19523#comment:3
<ul>
<li><strong>priority</strong>
changed from <em>critical</em> to <em>major</em>
</li>
<li><strong>description</strong>
modified (<a href="/ticket/19523?action=diff&version=3">diff</a>)
</li>
</ul>
TicketmkoeppeThu, 31 Mar 2016 02:08:09 GMT
https://trac.sagemath.org/ticket/19523#comment:4
https://trac.sagemath.org/ticket/19523#comment:4
<p>
While <code>MIPVariable</code> objects know which MIP they belong to,
their "components" gotten by <code>p[0]</code> etc. are elements of <code>LinearFunctionsParent(base_ring)</code>, 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.
</p>
TicketmkoeppeThu, 31 Mar 2016 04:58:38 GMTdescription changed
https://trac.sagemath.org/ticket/19523#comment:5
https://trac.sagemath.org/ticket/19523#comment:5
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/19523?action=diff&version=5">diff</a>)
</li>
</ul>
TicketjdemeyerThu, 31 Mar 2016 06:57:07 GMT
https://trac.sagemath.org/ticket/19523#comment:6
https://trac.sagemath.org/ticket/19523#comment:6
<p>
Given that <code>p[0]</code> is just a linear function, it would make the most sense if <code>q.add_constraint(p[0] <= 1)</code> would simply work, not give an error message.
</p>
TicketmkoeppeThu, 31 Mar 2016 06:58:12 GMT
https://trac.sagemath.org/ticket/19523#comment:7
https://trac.sagemath.org/ticket/19523#comment:7
<p>
No, q has no variables at this point.
</p>
TicketjdemeyerThu, 31 Mar 2016 07:35:45 GMT
https://trac.sagemath.org/ticket/19523#comment:8
https://trac.sagemath.org/ticket/19523#comment:8
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/19523#comment:7" title="Comment 7">mkoeppe</a>:
</p>
<blockquote class="citation">
<p>
No, q has no variables at this point.
</p>
</blockquote>
<p>
That's exactly my point: the variables should be added by <code>q.add_constraint()</code>.
</p>
TicketmkoeppeThu, 31 Mar 2016 20:37:20 GMT
https://trac.sagemath.org/ticket/19523#comment:9
https://trac.sagemath.org/ticket/19523#comment:9
<p>
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 <a class="closed ticket" href="https://trac.sagemath.org/ticket/10232" title="defect: check GLPK bound errors (closed: fixed)">#10232</a>.
</p>
TicketjdemeyerFri, 01 Apr 2016 19:36:19 GMT
https://trac.sagemath.org/ticket/19523#comment:10
https://trac.sagemath.org/ticket/19523#comment:10
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/19523#comment:9" title="Comment 9">mkoeppe</a>:
</p>
<blockquote class="citation">
<p>
at least when there's a dimension mismatch between the two problems.
</p>
</blockquote>
<p>
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.
</p>
TicketmkoeppeFri, 01 Apr 2016 22:57:37 GMT
https://trac.sagemath.org/ticket/19523#comment:11
https://trac.sagemath.org/ticket/19523#comment:11
<p>
Then I strongly prefer to always raise an error in this situation. This requires changes to <code>LinearFunction</code> (a class that is I think only used in the context of <code>MixedIntegerLinearProgram</code>), so that it remembers the MIP that it relates to.
</p>
<p>
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 <code>MixedIntegerLinearProgram</code>s, or to use <code>LinearFunction</code>s directly somehow (without referring to the correct MIP's variables).
</p>
TicketmkoeppeTue, 05 Apr 2016 16:59:48 GMTdependencies, description changed
https://trac.sagemath.org/ticket/19523#comment:12
https://trac.sagemath.org/ticket/19523#comment:12
<ul>
<li><strong>dependencies</strong>
changed from <em>#19525</em> to <em>#19525, #20360</em>
</li>
<li><strong>description</strong>
modified (<a href="/ticket/19523?action=diff&version=12">diff</a>)
</li>
</ul>
TicketmkoeppeTue, 05 Apr 2016 21:52:10 GMTsummary changed
https://trac.sagemath.org/ticket/19523#comment:13
https://trac.sagemath.org/ticket/19523#comment:13
<ul>
<li><strong>summary</strong>
changed from <em>Adding constraints for the wrong MILP crashes Sage</em> to <em>Raise an error when constraints are added to the wrong MILP</em>
</li>
</ul>
<p>
Since the crashes are now on separate tickets, I have changed the title of this ticket.
</p>
TicketmkoeppeSun, 17 Apr 2016 00:06:06 GMT
https://trac.sagemath.org/ticket/19523#comment:14
https://trac.sagemath.org/ticket/19523#comment:14
<p>
See also <a class="closed ticket" href="https://trac.sagemath.org/ticket/15159" title="defect: Segfault after deepcopy of MixedIntegerLinearProgram (closed: fixed)">#15159</a>, which raises a similar question when <code>q</code> started out as a copy of <code>p</code>.
</p>
TicketmkoeppeMon, 27 Jun 2016 03:16:03 GMT
https://trac.sagemath.org/ticket/19523#comment:15
https://trac.sagemath.org/ticket/19523#comment:15
<p>
A solution could be to change <code>LinearFunctionsParent</code> etc. so that it not only has remembers the <code>base_ring</code> but also the MIP.
</p>
TicketmkoeppeMon, 27 Jun 2016 03:16:16 GMTmilestone changed
https://trac.sagemath.org/ticket/19523#comment:16
https://trac.sagemath.org/ticket/19523#comment:16
<ul>
<li><strong>milestone</strong>
changed from <em>sage-6.10</em> to <em>sage-7.3</em>
</li>
</ul>
TicketjdemeyerWed, 23 Aug 2017 08:41:59 GMTdescription changed; cc, dependencies deleted
https://trac.sagemath.org/ticket/19523#comment:17
https://trac.sagemath.org/ticket/19523#comment:17
<ul>
<li><strong>cc</strong>
<em>ncohen</em> removed
</li>
<li><strong>dependencies</strong>
<em>#19525, #20360</em> deleted
</li>
<li><strong>description</strong>
modified (<a href="/ticket/19523?action=diff&version=17">diff</a>)
</li>
</ul>
TicketjdemeyerWed, 23 Aug 2017 08:42:14 GMTdescription changed
https://trac.sagemath.org/ticket/19523#comment:18
https://trac.sagemath.org/ticket/19523#comment:18
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/19523?action=diff&version=18">diff</a>)
</li>
</ul>
TicketyzhMon, 01 Feb 2021 04:11:54 GMTcc set
https://trac.sagemath.org/ticket/19523#comment:19
https://trac.sagemath.org/ticket/19523#comment:19
<ul>
<li><strong>cc</strong>
<em>yzh</em> added
</li>
</ul>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/19523#comment:15" title="Comment 15">mkoeppe</a>:
</p>
<blockquote class="citation">
<p>
A solution could be to change <code>LinearFunctionsParent</code> etc. so that it not only has remembers the <code>base_ring</code> but also the MIP.
</p>
</blockquote>
<p>
In contrast the example in the ticket description, no error is raised in the following examples, which is very confusing.
</p>
<pre class="wiki">sage: p = MixedIntegerLinearProgram(solver="glpk")
sage: q = MixedIntegerLinearProgram(solver="glpk")
sage: v = q.new_variable()
sage: q.add_constraint(v[0] <= 1)
</pre><p>
and
</p>
<pre class="wiki">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)
</pre><p>
It would be nice to have <code>LinearFunctionsParent</code> etc. remember its MIP.
</p>
Ticket