Sage: Ticket #7311: Change min/max arguments of MixedIntegerLinearProgram
https://trac.sagemath.org/ticket/7311
<p>
The min and max arguments should accept variables too, and not just real values or None.
</p>
enus
Sage
https://trac.sagemath.org/chrome/site/logo_sagemath_trac.png
https://trac.sagemath.org/ticket/7311
Trac 1.1.6

mhansen
Mon, 26 Oct 2009 17:19:45 GMT
summary changed
https://trac.sagemath.org/ticket/7311#comment:1
https://trac.sagemath.org/ticket/7311#comment:1
<ul>
<li><strong>summary</strong>
changed from <em>Change min/max arguments</em> to <em>Change min/max arguments of MixedIntegerLinearProgram</em>
</li>
</ul>
Ticket

ncohen
Fri, 01 Jan 2010 14:41:06 GMT
description changed; upstream set
https://trac.sagemath.org/ticket/7311#comment:2
https://trac.sagemath.org/ticket/7311#comment:2
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/7311?action=diff&version=2">diff</a>)
</li>
<li><strong>upstream</strong>
set to <em>N/A</em>
</li>
</ul>
Ticket

ncohen
Fri, 15 Jan 2010 15:56:55 GMT
status, description, summary changed
https://trac.sagemath.org/ticket/7311#comment:3
https://trac.sagemath.org/ticket/7311#comment:3
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>needs_review</em>
</li>
<li><strong>description</strong>
modified (<a href="/ticket/7311?action=diff&version=3">diff</a>)
</li>
<li><strong>summary</strong>
changed from <em>Change min/max arguments of MixedIntegerLinearProgram</em> to <em>Improve the add_constraint method from MixedIntegerLinearProgram</em>
</li>
</ul>
Ticket

ncohen
Fri, 15 Jan 2010 15:58:22 GMT
description changed
https://trac.sagemath.org/ticket/7311#comment:4
https://trac.sagemath.org/ticket/7311#comment:4
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/7311?action=diff&version=4">diff</a>)
</li>
</ul>
Ticket

ncohen
Wed, 27 Jan 2010 08:47:31 GMT
https://trac.sagemath.org/ticket/7311#comment:5
https://trac.sagemath.org/ticket/7311#comment:5
<p>
Jeffrey Kantor smartly noticed it may not be a good idea to have > be an alias for >= when dealing with integer programs... Fixed :)
</p>
<p>
Nathann
</p>
Ticket

ncohen
Sat, 30 Jan 2010 20:01:18 GMT
priority changed
https://trac.sagemath.org/ticket/7311#comment:6
https://trac.sagemath.org/ticket/7311#comment:6
<ul>
<li><strong>priority</strong>
changed from <em>minor</em> to <em>critical</em>
</li>
</ul>
Ticket

abmasse
Fri, 26 Feb 2010 10:21:04 GMT
cc set
https://trac.sagemath.org/ticket/7311#comment:7
https://trac.sagemath.org/ticket/7311#comment:7
<ul>
<li><strong>cc</strong>
<em>abmasse</em> added
</li>
</ul>
Ticket

abmasse
Sun, 28 Feb 2010 10:53:31 GMT
https://trac.sagemath.org/ticket/7311#comment:8
https://trac.sagemath.org/ticket/7311#comment:8
<p>
I started looking at your patch. I think it shouldn't be too hard to review. All tests pass !
</p>
<p>
However, I have one question: is there a reason why you implement all operators <, >, >=, etc. ? You could only implement <code>__richcmp__</code> and would do the same job with only one function.
</p>
<p>
Another strange thing is that the tests pass only when I install GLPK. With CBC, nothing works. Is that normal ?
</p>
Ticket

ncohen
Sun, 28 Feb 2010 11:08:06 GMT
https://trac.sagemath.org/ticket/7311#comment:9
https://trac.sagemath.org/ticket/7311#comment:9
<p>
Hello !!!!
</p>
<p>
I remember having tried the two This patch has had many different versions At some point some of these classes were "cdef class", then got back to usual python... I remember having noticed at some point that if I were to define < > == ... using richcmp instead of the usual python operators, other lowlevel comparators would be used by default instead of mine. Well, I had many troubles with this class, so perhaps I should give it a try again now that it works again :)
</p>
<p>
It is not normal at all that nothing works with CBC :/
Could you tell me which kind of errors you get ?
</p>
<p>
Thank youuuuuuuuu !!
</p>
<p>
Nathann
</p>
Ticket

abmasse
Sun, 28 Feb 2010 11:58:04 GMT
https://trac.sagemath.org/ticket/7311#comment:10
https://trac.sagemath.org/ticket/7311#comment:10
<p>
Here is what I did:
</p>
<ol><li>I tested all sage with <code>sage testall</code> and all tests passed.
</li></ol><ol start="2"><li>I tried <code>sage t optional</code> on the folder sage/numerical. Of course, some tests didn't pass.
</li></ol><ol start="3"><li>I installed CBC. All tests passed with <code>sage t *</code> in the sage/numerical folder, but when I type <code>sage t optional *</code>, I get the following:
</li></ol><pre class="wiki">[~/Applications/sage/devel/sagelinear/sage/numerical]$ sage t optional *
sage t optional "devel/sagelinear/sage/numerical/__init__.py"
[0.2 s]
sage t optional "devel/sagelinear/sage/numerical/all.py"
[0.1 s]
sage t optional "devel/sagelinear/sage/numerical/knapsack.py"
[13.8 s]
sage t optional "devel/sagelinear/sage/numerical/mip.pyx"
**********************************************************************
File "/Users/alexandre/Applications/sage/devel/sagelinear/sage/numerical/mip.pyx", line 444:
sage: p.write_mps(SAGE_TMP+"/lp_problem.mps") # optional  requires GLPK
Exception raised:
Traceback (most recent call last):
File "/Users/alexandre/Applications/sage/local/bin/ncadoctest.py", line 1231, in run_one_test
self.run_one_example(test, example, filename, compileflags)
File "/Users/alexandre/Applications/sage/local/bin/sagedoctest.py", line 38, in run_one_example
OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags)
File "/Users/alexandre/Applications/sage/local/bin/ncadoctest.py", line 1172, in run_one_example
compileflags, 1) in test.globs
File "<doctest __main__.example_12[6]>", line 1, in <module>
p.write_mps(SAGE_TMP+"/lp_problem.mps") # optional  requires GLPK###line 444:
sage: p.write_mps(SAGE_TMP+"/lp_problem.mps") # optional  requires GLPK
File "mip.pyx", line 453, in sage.numerical.mip.MixedIntegerLinearProgram.write_mps (sage/numerical/mip.c:4108)
raise NotImplementedError("You need GLPK installed to use this function. To install it, you can type in Sage: install_package('glpk')")
NotImplementedError: You need GLPK installed to use this function. To install it, you can type in Sage: install_package('glpk')
**********************************************************************
File "/Users/alexandre/Applications/sage/devel/sagelinear/sage/numerical/mip.pyx", line 476:
sage: p.write_lp(SAGE_TMP+"/lp_problem.lp") # optional  requires GLPK
Exception raised:
Traceback (most recent call last):
File "/Users/alexandre/Applications/sage/local/bin/ncadoctest.py", line 1231, in run_one_test
self.run_one_example(test, example, filename, compileflags)
File "/Users/alexandre/Applications/sage/local/bin/sagedoctest.py", line 38, in run_one_example
OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags)
File "/Users/alexandre/Applications/sage/local/bin/ncadoctest.py", line 1172, in run_one_example
compileflags, 1) in test.globs
File "<doctest __main__.example_13[6]>", line 1, in <module>
p.write_lp(SAGE_TMP+"/lp_problem.lp") # optional  requires GLPK###line 476:
sage: p.write_lp(SAGE_TMP+"/lp_problem.lp") # optional  requires GLPK
File "mip.pyx", line 484, in sage.numerical.mip.MixedIntegerLinearProgram.write_lp (sage/numerical/mip.c:4329)
raise NotImplementedError("You need GLPK installed to use this function. To install it, you can type in Sage: install_package('glpk')")
NotImplementedError: You need GLPK installed to use this function. To install it, you can type in Sage: install_package('glpk')
**********************************************************************
File "/Users/alexandre/Applications/sage/devel/sagelinear/sage/numerical/mip.pyx", line 978:
sage: p.solve(solver='GLPK', objective_only=True) # optional  requires GLPK
Expected:
Traceback (most recent call last):
...
RuntimeError
Got:
Traceback (most recent call last):
File "/Users/alexandre/Applications/sage/local/bin/ncadoctest.py", line 1231, in run_one_test
self.run_one_example(test, example, filename, compileflags)
File "/Users/alexandre/Applications/sage/local/bin/sagedoctest.py", line 38, in run_one_example
OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags)
File "/Users/alexandre/Applications/sage/local/bin/ncadoctest.py", line 1172, in run_one_example
compileflags, 1) in test.globs
File "<doctest __main__.example_23[21]>", line 1, in <module>
p.solve(solver='GLPK', objective_only=True) # optional  requires GLPK###line 978:
sage: p.solve(solver='GLPK', objective_only=True) # optional  requires GLPK
File "mip.pyx", line 1004, in sage.numerical.mip.MixedIntegerLinearProgram.solve (sage/numerical/mip.c:7627)
raise NotImplementedError("GLPK is not installed and cannot be used to solve this MixedIntegerLinearProgram. To install it, you can type in Sage: install_package('glpk')")
NotImplementedError: GLPK is not installed and cannot be used to solve this MixedIntegerLinearProgram. To install it, you can type in Sage: install_package('glpk')
**********************************************************************
File "/Users/alexandre/Applications/sage/devel/sagelinear/sage/numerical/mip.pyx", line 1175:
sage: p.solve(solver="GLPK") # optional  requires GLPK
Expected:
Traceback (most recent call last):
...
MIPSolverException: 'GLPK : Solution is undefined'
Got:
Traceback (most recent call last):
File "/Users/alexandre/Applications/sage/local/bin/ncadoctest.py", line 1231, in run_one_test
self.run_one_example(test, example, filename, compileflags)
File "/Users/alexandre/Applications/sage/local/bin/sagedoctest.py", line 38, in run_one_example
OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags)
File "/Users/alexandre/Applications/sage/local/bin/ncadoctest.py", line 1172, in run_one_example
compileflags, 1) in test.globs
File "<doctest __main__.example_30[9]>", line 1, in <module>
p.solve(solver="GLPK") # optional  requires GLPK###line 1175:
sage: p.solve(solver="GLPK") # optional  requires GLPK
File "mip.pyx", line 1004, in sage.numerical.mip.MixedIntegerLinearProgram.solve (sage/numerical/mip.c:7627)
raise NotImplementedError("GLPK is not installed and cannot be used to solve this MixedIntegerLinearProgram. To install it, you can type in Sage: install_package('glpk')")
NotImplementedError: GLPK is not installed and cannot be used to solve this MixedIntegerLinearProgram. To install it, you can type in Sage: install_package('glpk')
**********************************************************************
File "/Users/alexandre/Applications/sage/devel/sagelinear/sage/numerical/mip.pyx", line 1191:
sage: p.solve(solver="GLPK") # optional  requires GLPK
Expected:
Traceback (most recent call last):
...
MIPSolverException: 'GLPK : Solution is undefined'
Got:
Traceback (most recent call last):
File "/Users/alexandre/Applications/sage/local/bin/ncadoctest.py", line 1231, in run_one_test
self.run_one_example(test, example, filename, compileflags)
File "/Users/alexandre/Applications/sage/local/bin/sagedoctest.py", line 38, in run_one_example
OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags)
File "/Users/alexandre/Applications/sage/local/bin/ncadoctest.py", line 1172, in run_one_example
compileflags, 1) in test.globs
File "<doctest __main__.example_30[16]>", line 1, in <module>
p.solve(solver="GLPK") # optional  requires GLPK###line 1191:
sage: p.solve(solver="GLPK") # optional  requires GLPK
File "mip.pyx", line 1004, in sage.numerical.mip.MixedIntegerLinearProgram.solve (sage/numerical/mip.c:7627)
raise NotImplementedError("GLPK is not installed and cannot be used to solve this MixedIntegerLinearProgram. To install it, you can type in Sage: install_package('glpk')")
NotImplementedError: GLPK is not installed and cannot be used to solve this MixedIntegerLinearProgram. To install it, you can type in Sage: install_package('glpk')
**********************************************************************
4 items had failures:
1 of 7 in __main__.example_12
1 of 7 in __main__.example_13
1 of 22 in __main__.example_23
2 of 17 in __main__.example_30
***Test Failed*** 5 failures.
For whitespace errors, see the file /Users/alexandre/.sage//tmp/.doctest_mip.py
[3.1 s]
sage t optional "devel/sagelinear/sage/numerical/mip_coin.pyx"
[2.6 s]
sage t optional "devel/sagelinear/sage/numerical/mip_glpk.pyx"
**********************************************************************
File "/Users/alexandre/Applications/sage/devel/sagelinear/sage/numerical/mip_glpk.pyx", line 41:
sage: from sage.numerical.mip_glpk import solve_glpk # optional  requires Glpk
Exception raised:
Traceback (most recent call last):
File "/Users/alexandre/Applications/sage/local/bin/ncadoctest.py", line 1231, in run_one_test
self.run_one_example(test, example, filename, compileflags)
File "/Users/alexandre/Applications/sage/local/bin/sagedoctest.py", line 38, in run_one_example
OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags)
File "/Users/alexandre/Applications/sage/local/bin/ncadoctest.py", line 1172, in run_one_example
compileflags, 1) in test.globs
File "<doctest __main__.example_1[2]>", line 1, in <module>
from sage.numerical.mip_glpk import solve_glpk # optional  requires Glpk###line 41:
sage: from sage.numerical.mip_glpk import solve_glpk # optional  requires Glpk
ImportError: No module named mip_glpk
**********************************************************************
File "/Users/alexandre/Applications/sage/devel/sagelinear/sage/numerical/mip_glpk.pyx", line 49:
sage: solve_glpk(p, objective_only=True) # optional  requires Glpk
Exception raised:
Traceback (most recent call last):
File "/Users/alexandre/Applications/sage/local/bin/ncadoctest.py", line 1231, in run_one_test
self.run_one_example(test, example, filename, compileflags)
File "/Users/alexandre/Applications/sage/local/bin/sagedoctest.py", line 38, in run_one_example
OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags)
File "/Users/alexandre/Applications/sage/local/bin/ncadoctest.py", line 1172, in run_one_example
compileflags, 1) in test.globs
File "<doctest __main__.example_1[9]>", line 1, in <module>
solve_glpk(p, objective_only=True) # optional  requires Glpk###line 49:
sage: solve_glpk(p, objective_only=True) # optional  requires Glpk
NameError: name 'solve_glpk' is not defined
**********************************************************************
File "/Users/alexandre/Applications/sage/devel/sagelinear/sage/numerical/mip_glpk.pyx", line 123:
sage: p.write_mps(SAGE_TMP+"/lp_problem.mps") # optional  requires GLPK
Exception raised:
Traceback (most recent call last):
File "/Users/alexandre/Applications/sage/local/bin/ncadoctest.py", line 1231, in run_one_test
self.run_one_example(test, example, filename, compileflags)
File "/Users/alexandre/Applications/sage/local/bin/sagedoctest.py", line 38, in run_one_example
OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags)
File "/Users/alexandre/Applications/sage/local/bin/ncadoctest.py", line 1172, in run_one_example
compileflags, 1) in test.globs
File "<doctest __main__.example_2[6]>", line 1, in <module>
p.write_mps(SAGE_TMP+"/lp_problem.mps") # optional  requires GLPK###line 123:
sage: p.write_mps(SAGE_TMP+"/lp_problem.mps") # optional  requires GLPK
File "mip.pyx", line 453, in sage.numerical.mip.MixedIntegerLinearProgram.write_mps (sage/numerical/mip.c:4108)
raise NotImplementedError("You need GLPK installed to use this function. To install it, you can type in Sage: install_package('glpk')")
NotImplementedError: You need GLPK installed to use this function. To install it, you can type in Sage: install_package('glpk')
**********************************************************************
File "/Users/alexandre/Applications/sage/devel/sagelinear/sage/numerical/mip_glpk.pyx", line 164:
sage: p.write_lp(SAGE_TMP+"/lp_problem.lp") # optional  requires GLPK
Exception raised:
Traceback (most recent call last):
File "/Users/alexandre/Applications/sage/local/bin/ncadoctest.py", line 1231, in run_one_test
self.run_one_example(test, example, filename, compileflags)
File "/Users/alexandre/Applications/sage/local/bin/sagedoctest.py", line 38, in run_one_example
OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags)
File "/Users/alexandre/Applications/sage/local/bin/ncadoctest.py", line 1172, in run_one_example
compileflags, 1) in test.globs
File "<doctest __main__.example_3[6]>", line 1, in <module>
p.write_lp(SAGE_TMP+"/lp_problem.lp") # optional  requires GLPK###line 164:
sage: p.write_lp(SAGE_TMP+"/lp_problem.lp") # optional  requires GLPK
File "mip.pyx", line 484, in sage.numerical.mip.MixedIntegerLinearProgram.write_lp (sage/numerical/mip.c:4329)
raise NotImplementedError("You need GLPK installed to use this function. To install it, you can type in Sage: install_package('glpk')")
NotImplementedError: You need GLPK installed to use this function. To install it, you can type in Sage: install_package('glpk')
**********************************************************************
3 items had failures:
2 of 10 in __main__.example_1
1 of 7 in __main__.example_2
1 of 7 in __main__.example_3
***Test Failed*** 4 failures.
For whitespace errors, see the file /Users/alexandre/.sage//tmp/.doctest_mip_glpk.py
[2.6 s]
sage t optional "devel/sagelinear/sage/numerical/optimize.py"
[5.7 s]
sage t optional "devel/sagelinear/sage/numerical/test.py"
[5.7 s]

The following tests failed:
sage t optional "devel/sagelinear/sage/numerical/mip.pyx"
sage t optional "devel/sagelinear/sage/numerical/mip_glpk.pyx"
Total time for all tests: 33.9 seconds
</pre><ol start="4"><li>I then installed GLPK, and then all tests passed, with the command <code>sage t *</code> as well as the command <code>sage t optional *</code>.
</li></ol><p>
If I'm not mistaken and if my installation is not broken, it means that the examples you included in the docstring are based on GLPK and not CBC.
</p>
Ticket

ncohen
Sun, 28 Feb 2010 12:03:26 GMT
https://trac.sagemath.org/ticket/7311#comment:11
https://trac.sagemath.org/ticket/7311#comment:11
<p>
Then it is not a problem at all :)
</p>
<p>
The errors you see are in no way related to the present patch. You can see in all of your error messages that the line at fault if followed by : optional  requires GLPK. Note that there is no mention of CBC in those.
</p>
<p>
These lines are GLPKspecific as they are those who define the functions exporting a LP to a .mps or a .lp file, which make use of the GLPK library.
</p>
<p>
You had me worried for a while ! :)
</p>
<p>
Nathann
</p>
Ticket

abmasse
Sun, 28 Feb 2010 12:15:47 GMT
https://trac.sagemath.org/ticket/7311#comment:12
https://trac.sagemath.org/ticket/7311#comment:12
<p>
Great ! Sorry for my comments, I'm not familiar enough with these two optional packages. I'll resume reviewing the patch later today.
</p>
Ticket

abmasse
Mon, 01 Mar 2010 21:56:13 GMT
https://trac.sagemath.org/ticket/7311#comment:13
https://trac.sagemath.org/ticket/7311#comment:13
<p>
I looked once again at your patch. It seems correct, but I have a few comments:
</p>
<ol><li>Around line 711, you write
</li></ol><pre class="wiki">f = linear_function.f
</pre><p>
Woudn't be better to access this value by a function that <a class="missing wiki">LinearFunction?</a> provides ? More precisely, <a class="missing wiki">LinearFunction?</a> objects should have a method <code>f()</code> or <code>function()</code> and a private attribute <code>_f</code>. This is not a big problem, but seems cleaner. Same remark with <code>linear_function.constraints</code> of line 733 and <code>linear_function.equality</code> of line 735.
</p>
<ol start="2"><li>This is more a style remark. You tend to put a lot of space in your code, but I feel there is too much, which causes the body of the functions to span more lines than terminals can display.
</li></ol><p>
Short of that, the code seems ok, and all tests even optional pass. There are some typos in the doc, but I can fix it while reviewing. As soon as you answer/correct item 1, I should be able to finish reviewing it.
</p>
Ticket

ncohen
Tue, 02 Mar 2010 09:35:50 GMT
https://trac.sagemath.org/ticket/7311#comment:14
https://trac.sagemath.org/ticket/7311#comment:14
<p>
Here it is ! A new .dict() method for <a class="missing wiki">LinearFunction?</a> :)
</p>
<p>
I looked for long lines in the code, but I did not know how to fold them without making it much harder to understand... :/
</p>
<p>
If you find one you think can be safely cut, do not hesitate though :)
</p>
<p>
Thank you again for your help !
</p>
<p>
Nathann
</p>
Ticket

abmasse
Tue, 02 Mar 2010 22:23:24 GMT
https://trac.sagemath.org/ticket/7311#comment:15
https://trac.sagemath.org/ticket/7311#comment:15
<p>
Hello again Nathann !
</p>
<p>
I looked once again at your patch. You'll see that I added one on top of yours: it only modifies the formatting in the code and in the documentation, nothing major. In particular, I removed empty lines where they didn't seem necessary to me. I should give a positive review to your ticket very soon, but before that, there are some questions I would like you to answer.
</p>
<ol><li>There is a parameter <code>name</code> to the <code>add_constraint</code> function of <code>MixedIntegerLinearProgram</code>. Why do you need such a parameter ? I'm sure there is a good reason, but it would be nice to illustrate by an example why this is helpful.
</li></ol><ol start="2"><li>Around line 715, you have <code>constant_coefficient = f.pop(1,0)</code>. Isn't it dangerous ? If I'm not mistaken, this modifies the parameter <code>linear_function</code> and could have unsuspected side effects ?
</li></ol><ol start="3"><li>Since functions such as <code>__init__</code>, <code>__eq__</code>, etc. do not appear in the documentation, it would be better to have more detailed documentation just under the declaration of the class. Instead of <code>A class to represent Linear Constraints</code>, you could describe in detail what it is used for and give some examples.
</li></ol><p>
Sorry again for bothering you ! Don't forget to apply my patch if you want to add some modifications to yours. Next time should be a positive review !
</p>
Ticket

abmasse
Wed, 03 Mar 2010 07:53:12 GMT
https://trac.sagemath.org/ticket/7311#comment:16
https://trac.sagemath.org/ticket/7311#comment:16
<ol start="4"><li>There are still lines of code of the form
<pre class="wiki">functions = linear_function.constraints
if linear_function.equality:
</pre></li></ol><p>
It's true that you do not modify the <code>linear_function</code> object (you seem only to access the <code>constraints</code> and <code>equality</code> objects, but this would be cleaner to have methods that give you access to these.
</p>
Ticket

ncohen
Wed, 03 Mar 2010 09:49:14 GMT
https://trac.sagemath.org/ticket/7311#comment:17
https://trac.sagemath.org/ticket/7311#comment:17
<p>
Hello !! I will be answering your questions today :)
</p>
<p>
Meanhile, I understand that I access many fields without calling methods, but in the end as this class is very short (and as I don't directly access fields without calling methods when outside of the definition of this class), I feel like adding many other methods would just create more calls to function, thus slowing it a bit, without really improving the readability.
</p>
<p>
Well, I don't mind changing it anyway, but ... What do you think ?
</p>
<p>
Nathann
</p>
Ticket

abmasse
Wed, 03 Mar 2010 09:56:34 GMT
https://trac.sagemath.org/ticket/7311#comment:18
https://trac.sagemath.org/ticket/7311#comment:18
<p>
I agree with you, since speed is very important in MILPs, it is reasonable... Forget about what I said. However, I still think there is a reasonable way to extract the constant coefficient without having to modify the object returned by the <code>dict()</code> function. As for item 4, maybe this could be addressed in another ticket.
</p>
Ticket

ncohen
Wed, 03 Mar 2010 13:16:20 GMT
https://trac.sagemath.org/ticket/7311#comment:19
https://trac.sagemath.org/ticket/7311#comment:19
<p>
Hello !!!!
</p>
<p>
First, this new patch contains first my file trac_7311.patch, along with your corrections from trac_7311_reviewabm.patch (and my thanks for your time!).
</p>
<blockquote>
<p>
# (we talked about this together, but for the completeness of this ticket...). The names for Variables and Constraints are totally "useless" at the moment. Well, they can be useful only when one is trying to export a <a class="missing wiki">MixedIntegerLinearProgram?</a> object to a .mps or a .lp file using the methods write_mps and write_lp. This way, anyone can export the program and try to solve it using a different solver, as these files are standard types to encode a LP, and clearly having names in such a file instead of having variables like x1 x2 x3, .... and constraints like c1 c2 c3, ... can help at some point :)
</p>
</blockquote>
<blockquote>
<blockquote>
<p>
Besides, I have not lost hope of touching the .show() method so that it would use the names of these variables/constraints when the user wants to see his LP.
</p>
</blockquote>
</blockquote>
<blockquote>
<p>
# It is dangerous. And it is a mistake :)
</p>
</blockquote>
<blockquote>
<p>
# I thought the documentation from <span class="underline">init</span> appeared when typing <a class="missing wiki">MixedIntegerLinearProgram?</a>? .... A bit silly not to have noticed it until now.... You're perfectly right ! I copied the documentation from <span class="underline">init</span> several lines above and added a few lines to explain what the class does. It is likely no one but developpers will ever get interested in this class, as it does not appear when using LP.
</p>
</blockquote>
<p>
And here is the new version of the patch :)
</p>
<p>
Nathann
</p>
Ticket

ncohen
Wed, 03 Mar 2010 13:24:14 GMT
attachment set
https://trac.sagemath.org/ticket/7311
https://trac.sagemath.org/ticket/7311
<ul>
<li><strong>attachment</strong>
set to <em>trac_7311.patch</em>
</li>
</ul>
Ticket

abmasse
Thu, 04 Mar 2010 13:43:28 GMT
attachment set
https://trac.sagemath.org/ticket/7311
https://trac.sagemath.org/ticket/7311
<ul>
<li><strong>attachment</strong>
set to <em>trac_7311_reviewabm.patch</em>
</li>
</ul>
<p>
Minor doc and code formatting fixes  apply on top of Nathann's patch
</p>
Ticket

abmasse
Thu, 04 Mar 2010 13:51:35 GMT
reviewer, author set
https://trac.sagemath.org/ticket/7311#comment:20
https://trac.sagemath.org/ticket/7311#comment:20
<ul>
<li><strong>reviewer</strong>
set to <em>Alexandre Blondin MassÃ©</em>
</li>
<li><strong>author</strong>
set to <em>Nathann Cohen</em>
</li>
</ul>
<p>
I tested Nathann's patch on sage4.3.3. All tests passed and the documentation builds fine, except for the signature of many functions since the main involved file is a Cython file (this issue seems to have been solved in <a class="closed ticket" href="https://trac.sagemath.org/ticket/8244" title="defect: Annoying warnings when building the HTML reference manual (closed: fixed)">#8244</a>).
</p>
<p>
Note that all non optional tests passed with neither GLPK nor CBC installed, and all tests including optional ones pass when I installed both GLPK and CBC.
</p>
<p>
I made some minor changes on code formatting and documentation. If Nathann agrees with my changes, I allow him to set this ticket to a positive review.
</p>
Ticket

ncohen
Thu, 04 Mar 2010 14:34:04 GMT
https://trac.sagemath.org/ticket/7311#comment:21
https://trac.sagemath.org/ticket/7311#comment:21
<p>
Thank you very much !! But what do you mean about problems with the signatures of functions ? O_o
</p>
<p>
Nathann
</p>
Ticket

abmasse
Thu, 04 Mar 2010 14:43:25 GMT
https://trac.sagemath.org/ticket/7311#comment:22
https://trac.sagemath.org/ticket/7311#comment:22
<p>
When you run <code></code>sage docbuild reference html<code></code>, you get the following warnings:
</p>
<pre class="wiki">/Users/alexandre/Applications/sage/devel/sage/doc/en/reference/sage/numerical/mip.rst:6: (WARNING/2) error while formatting signature for sage.numerical.mip.LinearFunction.dict: arg is not a Python function
/Users/alexandre/Applications/sage/devel/sage/doc/en/reference/sage/numerical/mip.rst:6: (WARNING/2) error while formatting signature for sage.numerical.mip.MIPVariable.depth: arg is not a Python function
/Users/alexandre/Applications/sage/devel/sage/doc/en/reference/sage/numerical/mip.rst:6: (WARNING/2) error while formatting signature for sage.numerical.mip.MIPVariable.items: arg is not a Python function
/Users/alexandre/Applications/sage/devel/sage/doc/en/reference/sage/numerical/mip.rst:6: (WARNING/2) error while formatting signature for sage.numerical.mip.MIPVariable.keys: arg is not a Python function
/Users/alexandre/Applications/sage/devel/sage/doc/en/reference/sage/numerical/mip.rst:6: (WARNING/2) error while formatting signature for sage.numerical.mip.MIPVariable.values: arg is not a Python function
/Users/alexandre/Applications/sage/devel/sage/doc/en/reference/sage/numerical/mip.rst:6: (WARNING/2) error while formatting signature for sage.numerical.mip.MixedIntegerLinearProgram.add_constraint: arg is not a Python function
/Users/alexandre/Applications/sage/devel/sage/doc/en/reference/sage/numerical/mip.rst:6: (WARNING/2) error while formatting signature for sage.numerical.mip.MixedIntegerLinearProgram.constraints: arg is not a Python function
/Users/alexandre/Applications/sage/devel/sage/doc/en/reference/sage/numerical/mip.rst:6: (WARNING/2) error while formatting signature for sage.numerical.mip.MixedIntegerLinearProgram.get_max: arg is not a Python function
/Users/alexandre/Applications/sage/devel/sage/doc/en/reference/sage/numerical/mip.rst:6: (WARNING/2) error while formatting signature for sage.numerical.mip.MixedIntegerLinearProgram.get_min: arg is not a Python function
/Users/alexandre/Applications/sage/devel/sage/doc/en/reference/sage/numerical/mip.rst:6: (WARNING/2) error while formatting signature for sage.numerical.mip.MixedIntegerLinearProgram.get_values: arg is not a Python function
/Users/alexandre/Applications/sage/devel/sage/doc/en/reference/sage/numerical/mip.rst:6: (WARNING/2) error while formatting signature for sage.numerical.mip.MixedIntegerLinearProgram.is_binary: arg is not a Python function
/Users/alexandre/Applications/sage/devel/sage/doc/en/reference/sage/numerical/mip.rst:6: (WARNING/2) error while formatting signature for sage.numerical.mip.MixedIntegerLinearProgram.is_integer: arg is not a Python function
/Users/alexandre/Applications/sage/devel/sage/doc/en/reference/sage/numerical/mip.rst:6: (WARNING/2) error while formatting signature for sage.numerical.mip.MixedIntegerLinearProgram.is_real: arg is not a Python function
/Users/alexandre/Applications/sage/devel/sage/doc/en/reference/sage/numerical/mip.rst:6: (WARNING/2) error while formatting signature for sage.numerical.mip.MixedIntegerLinearProgram.new_variable: arg is not a Python function
/Users/alexandre/Applications/sage/devel/sage/doc/en/reference/sage/numerical/mip.rst:6: (WARNING/2) error while formatting signature for sage.numerical.mip.MixedIntegerLinearProgram.set_binary: arg is not a Python function
/Users/alexandre/Applications/sage/devel/sage/doc/en/reference/sage/numerical/mip.rst:6: (WARNING/2) error while formatting signature for sage.numerical.mip.MixedIntegerLinearProgram.set_integer: arg is not a Python function
/Users/alexandre/Applications/sage/devel/sage/doc/en/reference/sage/numerical/mip.rst:6: (WARNING/2) error while formatting signature for sage.numerical.mip.MixedIntegerLinearProgram.set_max: arg is not a Python function
/Users/alexandre/Applications/sage/devel/sage/doc/en/reference/sage/numerical/mip.rst:6: (WARNING/2) error while formatting signature for sage.numerical.mip.MixedIntegerLinearProgram.set_min: arg is not a Python function
/Users/alexandre/Applications/sage/devel/sage/doc/en/reference/sage/numerical/mip.rst:6: (WARNING/2) error while formatting signature for sage.numerical.mip.MixedIntegerLinearProgram.set_objective: arg is not a Python function
/Users/alexandre/Applications/sage/devel/sage/doc/en/reference/sage/numerical/mip.rst:6: (WARNING/2) error while formatting signature for sage.numerical.mip.MixedIntegerLinearProgram.set_objective_name: arg is not a Python function
/Users/alexandre/Applications/sage/devel/sage/doc/en/reference/sage/numerical/mip.rst:6: (WARNING/2) error while formatting signature for sage.numerical.mip.MixedIntegerLinearProgram.set_problem_name: arg is not a Python function
/Users/alexandre/Applications/sage/devel/sage/doc/en/reference/sage/numerical/mip.rst:6: (WARNING/2) error while formatting signature for sage.numerical.mip.MixedIntegerLinearProgram.set_real: arg is not a Python function
/Users/alexandre/Applications/sage/devel/sage/doc/en/reference/sage/numerical/mip.rst:6: (WARNING/2) error while formatting signature for sage.numerical.mip.MixedIntegerLinearProgram.show: arg is not a Python function
/Users/alexandre/Applications/sage/devel/sage/doc/en/reference/sage/numerical/mip.rst:6: (WARNING/2) error while formatting signature for sage.numerical.mip.MixedIntegerLinearProgram.solve: arg is not a Python function
/Users/alexandre/Applications/sage/devel/sage/doc/en/reference/sage/numerical/mip.rst:6: (WARNING/2) error while formatting signature for sage.numerical.mip.MixedIntegerLinearProgram.write_lp: arg is not a Python function
/Users/alexandre/Applications/sage/devel/sage/doc/en/reference/sage/numerical/mip.rst:6: (WARNING/2) error while formatting signature for sage.numerical.mip.MixedIntegerLinearProgram.write_mps: arg is not a Python function
</pre><p>
Sphinx has some problem to deal with Cython files when generating the documentation and this issue was discussed in <a class="closed ticket" href="https://trac.sagemath.org/ticket/8244" title="defect: Annoying warnings when building the HTML reference manual (closed: fixed)">#8244</a>.
</p>
Ticket

ncohen
Thu, 04 Mar 2010 16:20:46 GMT
status changed
https://trac.sagemath.org/ticket/7311#comment:23
https://trac.sagemath.org/ticket/7311#comment:23
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
</ul>
<p>
OOps, ok, I see.... Then I accept your positive review and your fixes :)
</p>
<p>
Thank you very much !!!!
</p>
<p>
Nathann
</p>
Ticket

abmasse
Wed, 10 Mar 2010 21:33:33 GMT
status changed
https://trac.sagemath.org/ticket/7311#comment:24
https://trac.sagemath.org/ticket/7311#comment:24
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>needs_work</em>
</li>
</ul>
<p>
While retesting the patch, I noticed that some tests didn't pass when one doesn't have CBC or GLPK installed.
</p>
<p>
This means that some doctest wasn't tagged "optional" properly. Sorry about that, Nathann, but there is still some work to do. I'll try to identify the bugs tonight.
</p>
Ticket

abmasse
Wed, 10 Mar 2010 21:40:10 GMT
status changed
https://trac.sagemath.org/ticket/7311#comment:25
https://trac.sagemath.org/ticket/7311#comment:25
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
</ul>
<p>
Sorry about my last comment, everything seems fine. This is very complicated to test optional packages properly... I have too many copies of sage on my computer. Positive review.
</p>
Ticket

abmasse
Wed, 10 Mar 2010 21:40:29 GMT
status changed
https://trac.sagemath.org/ticket/7311#comment:26
https://trac.sagemath.org/ticket/7311#comment:26
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
</ul>
Ticket

jhpalmieri
Thu, 15 Apr 2010 23:41:28 GMT
status changed; resolution, merged set
https://trac.sagemath.org/ticket/7311#comment:27
https://trac.sagemath.org/ticket/7311#comment:27
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>closed</em>
</li>
<li><strong>resolution</strong>
set to <em>fixed</em>
</li>
<li><strong>merged</strong>
set to <em>sage4.4.alpha0</em>
</li>
</ul>
<p>
Merged into 4.4.alpha0:
</p>
<ul><li>trac_7311.patch
</li><li>trac_7311_reviewabm.patch
</li></ul>
Ticket