Sage: Ticket #9108: Mark long doctests in rings/polynomial/symmetric_ideal
https://trac.sagemath.org/ticket/9108
<p>
Two doctests/examples in <code>sage/rings/polynomial/symmetric_ideal.py</code> tend to time out on older/slower machines (and take a large amount of the overall test time of that module).
</p>
<p>
Can you say which one? Then I'll either try shorter tests (which might actually be quite instructive, like a principal symmetric ideal whose minimal symmetric Groebner basis is formed by quite many polynomials) or mark the offensive tests as long.
</p>
en-usSagehttps://trac.sagemath.org/chrome/site/logo_sagemath_trac.png
https://trac.sagemath.org/ticket/9108
Trac 1.1.6SimonKingTue, 01 Jun 2010 15:19:47 GMTdescription changed
https://trac.sagemath.org/ticket/9108#comment:1
https://trac.sagemath.org/ticket/9108#comment:1
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/9108?action=diff&version=1">diff</a>)
</li>
</ul>
<p>
Replying to <a class="closed ticket" href="https://trac.sagemath.org/ticket/9108" title="defect: Mark long doctests in rings/polynomial/symmetric_ideal (closed: fixed)">leif</a>:
</p>
<blockquote class="citation">
<p>
Two doctests/examples in <code>sage/rings/polynomial/symmetric_ideal.py</code> tend to time out on older/slower machines (and take a large amount of the overall test time of that module).
</p>
</blockquote>
TicketleifTue, 01 Jun 2010 15:20:03 GMTattachment set
https://trac.sagemath.org/ticket/9108
https://trac.sagemath.org/ticket/9108
<ul>
<li><strong>attachment</strong>
set to <em>trac_9108-mark_long_doctests_in_symmetric_ideal.patch</em>
</li>
</ul>
<p>
Marks the offending lines with <code># long time</code>. Based on 4.4.3.alpha0.
</p>
TicketleifTue, 01 Jun 2010 15:23:32 GMTstatus changed
https://trac.sagemath.org/ticket/9108#comment:2
https://trac.sagemath.org/ticket/9108#comment:2
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>needs_review</em>
</li>
</ul>
<p>
Simon, you're too fast... ;-)
</p>
<p>
I've just uploaded a patch that reduces the module test time on a Pentium 4 (Prescott, 3.2 GHz) from 238.7 seconds to 33.1 seconds.
</p>
TicketleifTue, 01 Jun 2010 15:28:54 GMT
https://trac.sagemath.org/ticket/9108#comment:3
https://trac.sagemath.org/ticket/9108#comment:3
<p>
Perhaps you could add less demanding tests. ;-)
</p>
<p>
Also, some docstring lines are "too long". (I personally don't mind source code that exceeds 80 columns, but the help output should perhaps be limited to 80 characters in width.)
</p>
TicketleifTue, 01 Jun 2010 15:42:38 GMTcc changed
https://trac.sagemath.org/ticket/9108#comment:4
https://trac.sagemath.org/ticket/9108#comment:4
<ul>
<li><strong>cc</strong>
<em>cremona</em> added
</li>
</ul>
TicketcremonaTue, 01 Jun 2010 16:22:36 GMTstatus changed
https://trac.sagemath.org/ticket/9108#comment:5
https://trac.sagemath.org/ticket/9108#comment:5
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
</ul>
<p>
Before applying the patch to 4.4.3.alpha0:
</p>
<pre class="wiki">jec@selmer%sage -t sage/rings/polynomial/symmetric_ideal.py
sage -t "sage/rings/polynomial/symmetric_ideal.py"
[110.8 s]
----------------------------------------------------------------------
All tests passed!
Total time for all tests: 110.8 seconds
jec@selmer%sage -t -long sage/rings/polynomial/symmetric_ideal.py
sage -t -long "sage/rings/polynomial/symmetric_ideal.py"
[109.8 s]
----------------------------------------------------------------------
All tests passed!
Total time for all tests: 109.8 seconds
</pre><p>
and after:
</p>
<pre class="wiki">jec@selmer%sage -t sage/rings/polynomial/symmetric_ideal.py sage -t "sage/rings/polynomial/symmetric_ideal.py"
[16.5 s]
----------------------------------------------------------------------
All tests passed!
Total time for all tests: 16.5 seconds
jec@selmer%sage -t -long sage/rings/polynomial/symmetric_ideal.py sage -t -long "sage/rings/polynomial/symmetric_ideal.py"
[108.2 s]
----------------------------------------------------------------------
All tests passed!
Total time for all tests: 108.2 seconds
</pre><p>
Interesting to note that it is essentially just one test which takes the time!
</p>
TicketSimonKingTue, 01 Jun 2010 16:41:11 GMT
https://trac.sagemath.org/ticket/9108#comment:6
https://trac.sagemath.org/ticket/9108#comment:6
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/9108#comment:5" title="Comment 5">cremona</a>:
</p>
<blockquote class="citation">
<p>
Interesting to note that it is essentially just one test which takes the time!
</p>
</blockquote>
<p>
Off list, Leif just sent me some timings:
</p>
<p>
There is one symmetric Groebner basis computation that takes 73 seconds, but most of the time is actually spent for testing whether all variable permutations of all basis elements do indeed have symmetric reduction zero modulo the symmetric Groebner basis: 130 s.
</p>
<p>
I see two ways to proceed, depending on how soon the next release is due:
</p>
<ol><li>Leif's patch could go in, as John gave it a positive review, and it is certainly harmless and solves the problem.
</li><li>I could try to find a solution for the one offending doc test. For example, the long Groebner basis computation could be replaced by something else, such us the following, of course without the timings that I just inserted for demonstration:
<pre class="wiki">sage: R.<x,y> = InfinitePolynomialRing(GF(5),order='degrevlex')
sage: I = [2*x[4]*x[3]*y[4] - 2*y[0]^3]*R
sage: %time G = I.groebner_basis()
CPU times: user 1.70 s, sys: 0.01 s, total: 1.71 s
Wall time: 1.71 s
sage: G
[x_2*x_1*y_1 - y_0^3, x_2*x_1*y_2 - y_0^3, y_2*y_0^3 - y_1*y_0^3]
sage: %time [[(p^P).reduce(G) for p in G] for P in Permutations(Integer(3))]
CPU times: user 1.38 s, sys: 0.00 s, total: 1.38 s
Wall time: 1.38 s
[[0, 0, 0], [0, 0, 0], [0, 0, 0], [0, 0, 0], [0, 0, 0], [0, 0, 0]]
</pre></li></ol><p>
I think this example would actually be a good one, as it shows:
</p>
<ul><li>Even a "principal" symmetric ideal may have a reduced symmetric Groebner basis formed by more than one element.
</li><li>The test whether the elements still reduce to zero after variable permutation is easier, since the maximal variable index can be smaller (3 instead of 4; it should be bigger than the maximal index 2 that occurs in the symmetric Groebner basis).
</li></ul><p>
So, if the next release will be soon, I suggest to take Leif's patch as it is. But I think in the long run, a new example (like the one above) is needed.
</p>
<p>
Concerning line lengths: Does this only concern the first line of the doc strings? I know that my first lines tend to be rather lengthy, as I learnt that the basic description of the functionality should be given in the first line of the doc string (this is why I don't do a line wrap).
</p>
TicketleifTue, 01 Jun 2010 16:43:19 GMT
https://trac.sagemath.org/ticket/9108#comment:7
https://trac.sagemath.org/ticket/9108#comment:7
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/9108#comment:5" title="Comment 5">cremona</a>:
</p>
<blockquote class="citation">
<p>
[...]
Interesting to note that it is essentially just one test which takes the time!
</p>
</blockquote>
<p>
Actually two lines/tests take very long:
</p>
<pre class="wiki">line# walltime statement (preparsed)
[0116 72.980s] J=I.groebner_basis()
[0135 130.070s] [[(p**P).reduce(J) for p in J] for P in Permutations(Integer(4))]
</pre><p>
(of a total of ~240s on that system)
</p>
<p>
Note that line numbers slightly change after applying the patch.
</p>
TicketSimonKingTue, 01 Jun 2010 16:47:01 GMT
https://trac.sagemath.org/ticket/9108#comment:8
https://trac.sagemath.org/ticket/9108#comment:8
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/9108#comment:6" title="Comment 6">SimonKing</a>:
</p>
<blockquote class="citation">
<p>
I think this example would actually be a good one, as it shows:
</p>
</blockquote>
<p>
Or perhaps not <em>that</em> good...
</p>
<p>
The generator is not minimally chosen in its orbit, and I don't like that the second summand has index zero. But I recently did a series of random examples in order to test how large a symmetric Groebner basis of a symmetric ideal generated by a single small polynomial can actually be, so, it is likely that I'll find a better one.
</p>
TicketleifTue, 01 Jun 2010 16:56:23 GMT
https://trac.sagemath.org/ticket/9108#comment:9
https://trac.sagemath.org/ticket/9108#comment:9
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/9108#comment:6" title="Comment 6">SimonKing</a>:
</p>
<blockquote class="citation">
<p>
So, if the next release will be soon, I suggest to take Leif's patch as it is. But I think in the long run, a new example (like the one above) is needed.
</p>
</blockquote>
<p>
Feel free to add additional (short) tests... ;-)
</p>
<p>
Perhaps on another ticket?
</p>
<blockquote class="citation">
<p>
Concerning line lengths: Does this only concern the first line of the doc strings? I know that my first lines tend to be rather lengthy, as I learnt that the basic description of the functionality should be given in the first line of the doc string (this is why I don't do a line wrap).
</p>
</blockquote>
<p>
I just noticed that e.g. some parameter description lines are wider (net width).
</p>
<p>
Also, some are "marked" <code>(optional)</code>; the current practice seems to be repeating the default value from the function definition, too, i.e.
</p>
<pre class="wiki"> ``param`` -- (type, default: some_value) further description...
</pre>
TicketSimonKingTue, 01 Jun 2010 17:00:34 GMT
https://trac.sagemath.org/ticket/9108#comment:10
https://trac.sagemath.org/ticket/9108#comment:10
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/9108#comment:9" title="Comment 9">leif</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/9108#comment:6" title="Comment 6">SimonKing</a>:
</p>
<blockquote class="citation">
<p>
So, if the next release will be soon, I suggest to take Leif's patch as it is. But I think in the long run, a new example (like the one above) is needed.
</p>
</blockquote>
<p>
Feel free to add additional (short) tests... ;-)
</p>
<p>
Perhaps on another ticket?
</p>
</blockquote>
<p>
Seems reasonable. So, for now, the solution is to skip the long test unless it is wanted, and on a different ticket, I'll try to replace the offensive example (not <em>add</em> an example) and will also deal with the line length etc.
</p>
<p>
Thank you for your patch!
</p>
TicketcremonaTue, 01 Jun 2010 19:12:12 GMT
https://trac.sagemath.org/ticket/9108#comment:11
https://trac.sagemath.org/ticket/9108#comment:11
<p>
I am quite happy with the the conclusion to this discussion! John
</p>
TicketSimonKingWed, 02 Jun 2010 11:06:18 GMT
https://trac.sagemath.org/ticket/9108#comment:12
https://trac.sagemath.org/ticket/9108#comment:12
<p>
I created a new ticket <a class="closed ticket" href="https://trac.sagemath.org/ticket/9114" title="defect: Improve documentation of infinite polynomial rings (closed: fixed)">#9114</a> (ready for review) that replaces the offensive test by something better, and also improves the formatting of the documentation of "infinite polynomial rings and friends".
</p>
<p>
Since Leif's patch already has a positive review, I based <a class="closed ticket" href="https://trac.sagemath.org/ticket/9114" title="defect: Improve documentation of infinite polynomial rings (closed: fixed)">#9114</a> on it.
</p>
TicketmhansenSun, 06 Jun 2010 00:51:51 GMTreviewer, merged, author set
https://trac.sagemath.org/ticket/9108#comment:13
https://trac.sagemath.org/ticket/9108#comment:13
<ul>
<li><strong>reviewer</strong>
set to <em>John Cremona, Simon King</em>
</li>
<li><strong>merged</strong>
set to <em>sage-4.4.4.alpha0</em>
</li>
<li><strong>author</strong>
set to <em>Leif Leonhardy</em>
</li>
</ul>
TicketmhansenSun, 06 Jun 2010 08:26:59 GMTstatus changed; resolution set
https://trac.sagemath.org/ticket/9108#comment:14
https://trac.sagemath.org/ticket/9108#comment:14
<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>
</ul>
Ticket