Sage: Ticket #23892: Various doctest failures if pynormaliz is installed
https://trac.sagemath.org/ticket/23892
<pre class="wiki">sage -t --long --warn-long 63.7 src/sage/graphs/generic_graph.py # Killed due to segmentation fault
sage -t --long --warn-long 63.7 src/sage/crypto/mq/sr.py # Killed due to abort
sage -t --long --warn-long 63.7 src/sage/combinat/rigged_configurations/tensor_product_kr_tableaux.py # Bad exit: 1
sage -t --long --warn-long 63.7 src/sage/combinat/rigged_configurations/rigged_configurations.py # Bad exit: 127
sage -t --long --warn-long 63.7 src/sage/rings/integer.pyx # 1 doctest failed
sage -t --long --warn-long 63.7 src/sage/combinat/crystals/kirillov_reshetikhin.py # Bad exit: 1
sage -t --long --warn-long 63.7 src/sage/structure/sage_object.pyx # Killed due to abort
sage -t --long --warn-long 63.7 src/sage/rings/polynomial/multi_polynomial_sequence.py # Killed due to abort
sage -t --long --warn-long 63.7 src/sage/rings/polynomial/pbori.pyx # Killed due to segmentation fault
sage -t --long --warn-long 63.7 src/sage/combinat/rigged_configurations/rigged_configuration_element.py # Bad exit: 1
sage -t --long --warn-long 63.7 src/sage/crypto/sbox.py # Killed due to abort
sage -t --long --warn-long 63.7 src/sage/graphs/base/graph_backends.pyx # Killed due to segmentation fault
sage -t --long --warn-long 63.7 src/sage/rings/polynomial/multi_polynomial_libsingular.pyx # Killed due to segmentation fault
sage -t --long --warn-long 63.7 src/sage/combinat/rigged_configurations/bij_type_B.py # Bad exit: 1
sage -t --long --warn-long 63.7 src/sage/combinat/rigged_configurations/tensor_product_kr_tableaux_element.py # Bad exit: 2
sage -t --long --warn-long 63.7 src/sage/crypto/boolean_function.pyx # Killed due to abort
sage -t --long --warn-long 63.7 src/sage/combinat/rigged_configurations/kleber_tree.py # Bad exit: 127
sage -t --long --warn-long 63.7 src/sage/combinat/crystals/affinization.py # Bad exit: 1
sage -t --long --warn-long 63.7 src/sage/structure/category_object.pyx # Killed due to abort
sage -t --long --warn-long 63.7 src/sage/combinat/crystals/tensor_product_element.pyx # Bad exit: 127
sage -t --long --warn-long 63.7 src/sage/combinat/rigged_configurations/bij_type_E67.py # Bad exit: 1
sage -t --long --warn-long 63.7 src/sage/geometry/polyhedron/backend_normaliz.py # Bad exit: 127
sage -t --long --warn-long 63.7 src/sage/sat/solvers/dimacs.py # Killed due to abort
sage -t --long --warn-long 63.7 src/doc/en/reference/sat/index.rst # Killed due to abort
sage -t --long --warn-long 63.7 src/sage/sat/boolean_polynomials.py # Killed due to abort
sage -t --long --warn-long 63.7 src/sage/rings/polynomial/polynomial_ring_constructor.py # Killed due to abort
sage -t --long --warn-long 63.7 src/sage/sat/converters/polybori.py # Killed due to abort
sage -t --long --warn-long 63.7 src/sage/ext/memory.pyx # 1 doctest failed
</pre><p>
The cause seems to be the same as <a class="closed ticket" href="https://trac.sagemath.org/ticket/23879" title="defect: Various doctest failures if cbc is installed (closed: duplicate)">#23879</a>: <code>pynormaliz</code> requires a very large amount of memory (more than what <a class="closed ticket" href="https://trac.sagemath.org/ticket/23748" title="enhancement: Run doctests with limited memory (closed: fixed)">#23748</a> allows).
</p>
en-usSagehttps://trac.sagemath.org/chrome/site/logo_sagemath_trac.png
https://trac.sagemath.org/ticket/23892
Trac 1.1.6mderickxTue, 19 Sep 2017 11:20:19 GMT
https://trac.sagemath.org/ticket/23892#comment:1
https://trac.sagemath.org/ticket/23892#comment:1
<p>
Again??? :(
</p>
TicketmderickxTue, 19 Sep 2017 11:31:03 GMTcc set
https://trac.sagemath.org/ticket/23892#comment:2
https://trac.sagemath.org/ticket/23892#comment:2
<ul>
<li><strong>cc</strong>
<em>mderickx</em> added
</li>
</ul>
TicketjdemeyerWed, 20 Sep 2017 07:07:05 GMTdescription changed
https://trac.sagemath.org/ticket/23892#comment:3
https://trac.sagemath.org/ticket/23892#comment:3
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/23892?action=diff&version=3">diff</a>)
</li>
</ul>
TicketjdemeyerWed, 20 Sep 2017 09:03:03 GMTdescription changed
https://trac.sagemath.org/ticket/23892#comment:4
https://trac.sagemath.org/ticket/23892#comment:4
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/23892?action=diff&version=4">diff</a>)
</li>
</ul>
TicketjdemeyerWed, 20 Sep 2017 11:56:32 GMTdescription, summary changed; author set
https://trac.sagemath.org/ticket/23892#comment:5
https://trac.sagemath.org/ticket/23892#comment:5
<ul>
<li><strong>author</strong>
set to <em>Jeroen Demeyer</em>
</li>
<li><strong>description</strong>
modified (<a href="/ticket/23892?action=diff&version=5">diff</a>)
</li>
<li><strong>summary</strong>
changed from <em>Various doctest failures if pynormaliz is installed</em> to <em>Run doctests with OMP_NUM_THREADS=2</em>
</li>
</ul>
TicketjdemeyerWed, 20 Sep 2017 12:01:47 GMTbranch set
https://trac.sagemath.org/ticket/23892#comment:6
https://trac.sagemath.org/ticket/23892#comment:6
<ul>
<li><strong>branch</strong>
set to <em>u/jdemeyer/run_doctests_with_omp_num_threads_2</em>
</li>
</ul>
TicketjdemeyerWed, 20 Sep 2017 12:02:03 GMTstatus changed; commit set
https://trac.sagemath.org/ticket/23892#comment:7
https://trac.sagemath.org/ticket/23892#comment:7
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>needs_review</em>
</li>
<li><strong>commit</strong>
set to <em>9f9f7b71f56c1c7ac20ed8601355ef495bf42fa3</em>
</li>
</ul>
<p>
New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit?id=9f9f7b71f56c1c7ac20ed8601355ef495bf42fa3"><span class="icon"></span>9f9f7b7</a></td><td><code>Run doctests with OMP_NUM_THREADS=2</code>
</td></tr></table>
TicketmderickxWed, 20 Sep 2017 12:22:07 GMT
https://trac.sagemath.org/ticket/23892#comment:8
https://trac.sagemath.org/ticket/23892#comment:8
<p>
Hi Jeroen,
</p>
<p>
I want to review this, but before doing so I run into trouble, because on my machine all doctests pass in sage 8.1.beta5 with pynormaliz. Could you give pointers to which doctests failed for you and maybe help reproduce the failure so I can better understand wether this solution works. Also is there any particular reason for the integer 2? Why not 3 or 4? I agree it should not be 1 because certain bugs might go undetected in that way.
</p>
TicketjdemeyerWed, 20 Sep 2017 13:21:09 GMT
https://trac.sagemath.org/ticket/23892#comment:9
https://trac.sagemath.org/ticket/23892#comment:9
<p>
In particular, many tests in <code>src/sage/combinat/rigged_configurations</code> fail for me with <code>pynormaliz</code>.
</p>
<p>
Since the problem depends on the number of threads, which is the number of cores by default, it could very well be that this problem only occurs on systems with many cores. The system where I saw the failure has 24 cores. Maybe you could get the failure with <code>OMP_NUM_THREADS=24</code>?
</p>
TicketjdemeyerWed, 20 Sep 2017 13:22:34 GMT
https://trac.sagemath.org/ticket/23892#comment:10
https://trac.sagemath.org/ticket/23892#comment:10
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/23892#comment:8" title="Comment 8">mderickx</a>:
</p>
<blockquote class="citation">
<p>
Also is there any particular reason for the integer 2?
</p>
</blockquote>
<p>
Yes, it is the smallest integer strictly larger than 1.
</p>
<p>
1 thread is too few, because it doesn't really test threading. With 2 threads, you do test threading. On the other hand, the system load will at most be a factor 200% too large, which is not too bad.
</p>
TicketmderickxWed, 20 Sep 2017 15:02:29 GMT
https://trac.sagemath.org/ticket/23892#comment:11
https://trac.sagemath.org/ticket/23892#comment:11
<p>
Without the patch I indeed get 4 files with failing doctests if I do:
</p>
<pre class="wiki">export OMP_NUM_THREADS=24
sage -t long src/sage/combinat/rigged_configurations
</pre><p>
and it does not fail anymore with the patch. So it seems to be the right thing to do in order to fix it.
</p>
<p>
One thing that I don't like about the current patch is that it overwrites <code>OMP_NUM_THREADS</code> even if it is already explicitly set before running sage tests. This means that if someone for some reason wants to run the doctests with a different number of <code>OMP_NUM_THREADS</code> for debugging purposes that involve problems with threading then one has to modify the source code. So I think it would be better to only set <code>OMP_NUM_THREADS=2</code> if nothing was set before, providing a sane default value, but still allowing a less sane default value if one really insists. What are your thoughts on this?
</p>
TicketjdemeyerThu, 21 Sep 2017 07:53:52 GMT
https://trac.sagemath.org/ticket/23892#comment:12
https://trac.sagemath.org/ticket/23892#comment:12
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/23892#comment:11" title="Comment 11">mderickx</a>:
</p>
<blockquote class="citation">
<p>
One thing that I don't like about the current patch is that it overwrites <code>OMP_NUM_THREADS</code> even if it is already explicitly set before running sage tests.
</p>
</blockquote>
<p>
I consider that a feature. The point is that doctests should be reproducible and not depend too much on the external environment. If somebody has set an environment variable <code>OMP_NUM_THREADS</code>, the most likely reason is that he wants to use that number of threads for actual computations. It does not mean that he wants to use that many threads for doctests too.
</p>
<blockquote class="citation">
<p>
This means that if someone for some reason wants to run the doctests with a different number of <code>OMP_NUM_THREADS</code> for debugging purposes that involve problems with threading then one has to modify the source code.
</p>
</blockquote>
<p>
Alternatively, you can set <code>os.environ['OMP_NUM_THREADS']</code> in a doctest too. That's easy to do and would fix the testing problem.
</p>
TicketmderickxMon, 25 Sep 2017 13:15:40 GMT
https://trac.sagemath.org/ticket/23892#comment:13
https://trac.sagemath.org/ticket/23892#comment:13
<p>
Ok, I am now running all the doctest after <code>export OMP_NUM_THREADS=100</code> since I think standard patchbot testing is not good enough. If this passes then I will give positive review.
</p>
<p>
Does your remark mean that it would also be better to fix <a class="closed ticket" href="https://trac.sagemath.org/ticket/23612" title="defect: Include !python3 in default value for SAGE_CHECK_PACKAGES (closed: fixed)">#23612</a> (edit: sorry I meant <a class="needs_work ticket" href="https://trac.sagemath.org/ticket/23613" title="defect: Doctest failure if $PYTHONPATH is set (needs_work)">#23613</a>) by unsetting <code>PYTHONPATH</code> instead of making the doctest more admissible?
</p>
TicketjdemeyerMon, 25 Sep 2017 13:24:52 GMT
https://trac.sagemath.org/ticket/23892#comment:14
https://trac.sagemath.org/ticket/23892#comment:14
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/23892#comment:13" title="Comment 13">mderickx</a>:
</p>
<blockquote class="citation">
<p>
Does your remark mean that it would also be better to fix <a class="closed ticket" href="https://trac.sagemath.org/ticket/23612" title="defect: Include !python3 in default value for SAGE_CHECK_PACKAGES (closed: fixed)">#23612</a> by unsetting <code>PYTHONPATH</code> instead of making the doctest more admissible?
</p>
</blockquote>
<p>
Are you sure you mean <a class="closed ticket" href="https://trac.sagemath.org/ticket/23612" title="defect: Include !python3 in default value for SAGE_CHECK_PACKAGES (closed: fixed)">#23612</a>? It seems unrelated to <code>PYTHONPATH</code> or doctests.
</p>
TicketmderickxMon, 25 Sep 2017 13:35:09 GMT
https://trac.sagemath.org/ticket/23892#comment:15
https://trac.sagemath.org/ticket/23892#comment:15
<p>
Sorry, I meant <a class="needs_work ticket" href="https://trac.sagemath.org/ticket/23613" title="defect: Doctest failure if $PYTHONPATH is set (needs_work)">#23613</a>.
</p>
TicketmderickxMon, 25 Sep 2017 15:43:05 GMTstatus changed
https://trac.sagemath.org/ticket/23892#comment:16
https://trac.sagemath.org/ticket/23892#comment:16
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
</ul>
<p>
Ok looks good to me.
</p>
TicketvbraunMon, 25 Sep 2017 22:42:06 GMTstatus changed
https://trac.sagemath.org/ticket/23892#comment:17
https://trac.sagemath.org/ticket/23892#comment:17
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>needs_work</em>
</li>
</ul>
<p>
Reviewer name
</p>
TicketmderickxTue, 26 Sep 2017 01:59:30 GMTstatus changed; reviewer set
https://trac.sagemath.org/ticket/23892#comment:18
https://trac.sagemath.org/ticket/23892#comment:18
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>positive_review</em>
</li>
<li><strong>reviewer</strong>
set to <em>Maarten Derickx</em>
</li>
</ul>
TicketvbraunSun, 01 Oct 2017 00:19:15 GMTstatus, branch changed; resolution set
https://trac.sagemath.org/ticket/23892#comment:19
https://trac.sagemath.org/ticket/23892#comment:19
<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>branch</strong>
changed from <em>u/jdemeyer/run_doctests_with_omp_num_threads_2</em> to <em>9f9f7b71f56c1c7ac20ed8601355ef495bf42fa3</em>
</li>
</ul>
TicketembrayThu, 05 Oct 2017 16:54:20 GMTcommit deleted
https://trac.sagemath.org/ticket/23892#comment:20
https://trac.sagemath.org/ticket/23892#comment:20
<ul>
<li><strong>commit</strong>
<em>9f9f7b71f56c1c7ac20ed8601355ef495bf42fa3</em> deleted
</li>
</ul>
<p>
I wonder if this and/or <a class="closed ticket" href="https://trac.sagemath.org/ticket/23748" title="enhancement: Run doctests with limited memory (closed: fixed)">#23748</a> will fix the doc build problems I was having on Windows for the past several weeks (which caused me to have to take down the Window patchbot >_<). Fingers crossed...
</p>
TicketembrayThu, 05 Oct 2017 16:55:51 GMT
https://trac.sagemath.org/ticket/23892#comment:21
https://trac.sagemath.org/ticket/23892#comment:21
<p>
Oh wait, this was just for the doctests, I misread. Maybe not then...
</p>
Ticket