Sage: Ticket #7794: PolynomialRing_integral_domain ignores Ctrl-C and segfaults
https://trac.sagemath.org/ticket/7794
<p>
The following was reported by Alex P on <a class="ext-link" href="http://groups.google.com/group/sage-support/browse_thread/thread/d92efb12f35e758a"><span class="icon"></span>sage-support</a>:
</p>
<pre class="wiki">sage: F.<a> = FiniteField(3)
sage: P.<T> = PolynomialRing(F)
sage: PP.<z> = PolynomialRing(P)
sage: PP
Univariate Polynomial Ring in z over Univariate Polynomial Ring in T over Finite Field of size 3
sage: type(PP)
<class 'sage.rings.polynomial.polynomial_ring.PolynomialRing_integral_domain'>
sage: (z^3 + T*z)^(81*3)
^CException KeyboardInterrupt: KeyboardInterrupt() in 'sage.rings.polynomial.polynomial_zmod_flint.get_cparent' ignored
^C^C
------------------------------------------------------------
Unhandled SIGFPE: An unhandled floating point exception occured in SAGE.
This probably occured because a *compiled* component
of SAGE has a bug in it (typically accessing invalid memory)
or is not properly wrapped with _sig_on, _sig_off.
You might want to run SAGE under gdb with 'sage -gdb' to debug this.
SAGE will now terminate (sorry).
------------------------------------------------------------
</pre><p>
So: First, Ctrl-C is ignored, which is bad IMO. Then, hitting Ctrl-C again (actually twice) results in a segfault, which is a desaster.
</p>
en-usSagehttps://trac.sagemath.org/chrome/site/logo_sagemath_trac.png
https://trac.sagemath.org/ticket/7794
Trac 1.1.6SimonKingThu, 19 May 2011 18:08:26 GMTstatus changed
https://trac.sagemath.org/ticket/7794#comment:1
https://trac.sagemath.org/ticket/7794#comment:1
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>needs_review</em>
</li>
</ul>
<p>
I just verified that the problem seems to be fixed in sage-4.6.2.
</p>
<p>
Firstly, the original example now works sufficiently quickly, so that there is no need to interrupt the computation:
</p>
<pre class="wiki">sage: F.<a> = FiniteField(3)
sage: P.<T> = PolynomialRing(F)
sage: PP.<z> = PolynomialRing(P)
sage: PP
Univariate Polynomial Ring in z over Univariate Polynomial Ring in T over Finite Field of size 3
sage: type(PP)
<class 'sage.rings.polynomial.polynomial_ring.PolynomialRing_integral_domain'>
sage: (z^3 + T*z)^(81*3)
z^729 + T^243*z^243
</pre><p>
Secondly, if one tries the same thing with a higher exponent, hitting Ctrl-c works (or at least it does after several attempts):
</p>
<pre class="wiki">sage: (z^3 + T*z)^(81^3)
^C^C^C---------------------------------------------------------------------------
KeyboardInterrupt Traceback (most recent call last)
/home/king/<ipython console> in <module>()
/mnt/local/king/SAGE/broken/local/lib/python2.6/site-packages/sage/rings/polynomial/polynomial_element.so in sage.rings.polynomial.polynomial_element.Polynomial.__pow__ (sage/rings/polynomial/polynomial_element.c:12738)()
/mnt/local/king/SAGE/broken/local/lib/python2.6/site-packages/sage/structure/element.so in sage.structure.element.generic_power (sage/structure/element.c:20326)()
...
<LONG TRACEBACK>
...
/mnt/local/king/SAGE/broken/local/lib/python2.6/site-packages/sage/interfaces/get_sigs.pyc in my_sigint(x, n)
7
8 def my_sigint(x, n):
----> 9 raise KeyboardInterrupt
10
11 def my_sigfpe(x, n):
KeyboardInterrupt:
</pre><p>
So, I believe this ticket can be closed! If I am not mistaken, I first need to put it as "needs review"...
</p>
TicketSimonKingThu, 19 May 2011 18:10:34 GMTstatus, milestone changed; reviewer set
https://trac.sagemath.org/ticket/7794#comment:2
https://trac.sagemath.org/ticket/7794#comment:2
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
<li><strong>reviewer</strong>
set to <em>Simon King</em>
</li>
<li><strong>milestone</strong>
changed from <em>sage-4.7</em> to <em>sage-duplicate/invalid/wontfix</em>
</li>
</ul>
<p>
I miss the possibility to change the ticket into the state "worksforme", but that's my review, in one word...
</p>
TicketjdemeyerThu, 19 May 2011 20:00:10 GMTstatus, milestone changed
https://trac.sagemath.org/ticket/7794#comment:3
https://trac.sagemath.org/ticket/7794#comment:3
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>needs_work</em>
</li>
<li><strong>milestone</strong>
changed from <em>sage-duplicate/invalid/wontfix</em> to <em>sage-4.7.1</em>
</li>
</ul>
<p>
I'm still able to reproduce the problem on sage-4.7.rc2 (note that interrupt handling was completely rewritten in <a class="closed ticket" href="https://trac.sagemath.org/ticket/9678" title="task: Rewrite interrupt handling (closed: fixed)">#9678</a>, merged in sage-4.7.alpha1, so testing this with sage-4.6.2 is pointless).
</p>
<pre class="wiki">sage: F.<a> = FiniteField(3)
sage: P.<T> = PolynomialRing(F)
sage: PP.<z> = PolynomialRing(P)
sage: (z^3 + T*z)^(81*3)
^C^CException KeyboardInterrupt in 'sage.rings.polynomial.polynomial_zmod_flint.get_cparent' ignored
z^729 + T^243*z^243
sage: (z^3 + T*z)^(81*3)
^C/usr/local/src/sage-4.7/local/lib/libcsage.so(print_backtrace+0x31)[0x7f3398533e02]
/usr/local/src/sage-4.7/local/lib/libcsage.so(sigdie+0x14)[0x7f3398533e34]
/usr/local/src/sage-4.7/local/lib/libcsage.so(sage_signal_handler+0x1e6)[0x7f3398533a5c]
/lib/libpthread.so.0(+0xf400)[0x7f339d78c400]
/usr/local/src/sage-4.7/local/lib/libgmp.so.3(__gmp_exception+0x1e)[0x7f33982c357e]
/usr/local/src/sage-4.7/local/lib/libgmp.so.3(+0xb5ae)[0x7f33982c35ae]
/usr/local/src/sage-4.7/local/lib/libgmp.so.3(__gmpz_fdiv_ui+0x0)[0x7f33982cfed0]
/usr/local/src/sage-4.7/local/lib/libflint.so(zn_mod_init+0x5c)[0x7f33902e109c]
/usr/local/src/sage-4.7/local/lib/python2.6/site-packages/sage/rings/polynomial/polynomial_zmod_flint.so(+0x19ce6)[0x7f336d5ddce6]
/usr/local/src/sage-4.7/local/lib/libpython2.6.so.1.0(+0x9b85c)[0x7f339da3485c]
/usr/local/src/sage-4.7/local/lib/libpython2.6.so.1.0(PyObject_Call+0x53)[0x7f339d9e3cf3]
/usr/local/src/sage-4.7/local/lib/libpython2.6.so.1.0(PyEval_CallObjectWithKeywords+0x47)[0x7f339da76617]
/usr/local/src/sage-4.7/local/lib/libpython2.6.so.1.0(+0x61747)[0x7f339d9fa747]
/usr/local/src/sage-4.7/local/lib/libpython2.6.so.1.0(PyObject_Call+0x53)[0x7f339d9e3cf3]
/usr/local/src/sage-4.7/local/lib/python2.6/site-packages/sage/rings/polynomial/polynomial_zmod_flint.so(+0xad24)[0x7f336d5ced24]
/usr/local/src/sage-4.7/local/lib/libpython2.6.so.1.0(+0xa1098)[0x7f339da3a098]
/usr/local/src/sage-4.7/local/lib/libpython2.6.so.1.0(PyObject_Call+0x53)[0x7f339d9e3cf3]
/usr/local/src/sage-4.7/local/lib/libpython2.6.so.1.0(PyEval_EvalFrameEx+0x3a79)[0x7f339da7a689]
/usr/local/src/sage-4.7/local/lib/libpython2.6.so.1.0(PyEval_EvalCodeEx+0x879)[0x7f339da7d819]
/usr/local/src/sage-4.7/local/lib/libpython2.6.so.1.0(+0x70b26)[0x7f339da09b26]
/usr/local/src/sage-4.7/local/lib/libpython2.6.so.1.0(PyObject_Call+0x53)[0x7f339d9e3cf3]
/usr/local/src/sage-4.7/local/lib/libpython2.6.so.1.0(+0x5793f)[0x7f339d9f093f]
/usr/local/src/sage-4.7/local/lib/libpython2.6.so.1.0(PyObject_Call+0x53)[0x7f339d9e3cf3]
/usr/local/src/sage-4.7/local/lib/python2.6/site-packages/sage/rings/polynomial/polynomial_element.so(+0x12ec1)[0x7f3388decec1]
/usr/local/src/sage-4.7/local/lib/python2.6/site-packages/sage/rings/polynomial/polynomial_element.so(+0x12beb)[0x7f3388decbeb]
/usr/local/src/sage-4.7/local/lib/python2.6/site-packages/sage/categories/map.so(+0x6be5)[0x7f3394501be5]
/usr/local/src/sage-4.7/local/lib/python2.6/site-packages/sage/structure/coerce.so(+0x1fb4d)[0x7f339473eb4d]
/usr/local/src/sage-4.7/local/lib/python2.6/site-packages/sage/structure/coerce.so(+0x179e0)[0x7f33947369e0]
/usr/local/src/sage-4.7/local/lib/python2.6/site-packages/sage/structure/element.so(+0x108d9)[0x7f339496b8d9]
/usr/local/src/sage-4.7/local/lib/python2.6/site-packages/sage/structure/element.so(+0x3a681)[0x7f3394995681]
/usr/local/src/sage-4.7/local/lib/libpython2.6.so.1.0(+0x4621c)[0x7f339d9df21c]
/usr/local/src/sage-4.7/local/lib/libpython2.6.so.1.0(PyNumber_Add+0x20)[0x7f339d9e1640]
/usr/local/src/sage-4.7/local/lib/python2.6/site-packages/sage/rings/polynomial/polynomial_element.so(+0x41f03)[0x7f3388e1bf03]
/usr/local/src/sage-4.7/local/lib/python2.6/site-packages/sage/rings/polynomial/polynomial_element.so(+0x6ff62)[0x7f3388e49f62]
/usr/local/src/sage-4.7/local/lib/python2.6/site-packages/sage/rings/polynomial/polynomial_element.so(+0x6ed99)[0x7f3388e48d99]
/usr/local/src/sage-4.7/local/lib/python2.6/site-packages/sage/rings/polynomial/polynomial_element.so(+0x6ef0c)[0x7f3388e48f0c]
/usr/local/src/sage-4.7/local/lib/python2.6/site-packages/sage/rings/polynomial/polynomial_element.so(+0x6edcf)[0x7f3388e48dcf]
------------------------------------------------------------------------
Unhandled SIGFPE: An unhandled floating point exception occurred in Sage.
This probably occurred because a *compiled* component of Sage has a bug
in it and is not properly wrapped with sig_on(), sig_off(). You might
want to run Sage under gdb with 'sage -gdb' to debug this.
Sage will now terminate.
------------------------------------------------------------------------
</pre>
TicketSimonKingFri, 20 May 2011 05:36:04 GMT
https://trac.sagemath.org/ticket/7794#comment:4
https://trac.sagemath.org/ticket/7794#comment:4
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/7794#comment:3" title="Comment 3">jdemeyer</a>:
</p>
<blockquote class="citation">
<p>
I'm still able to reproduce the problem on sage-4.7.rc2 (note that interrupt handling was completely rewritten in <a class="closed ticket" href="https://trac.sagemath.org/ticket/9678" title="task: Rewrite interrupt handling (closed: fixed)">#9678</a>, merged in sage-4.7.alpha1, so testing this with sage-4.6.2 is pointless).
</p>
</blockquote>
<p>
What a pity!
</p>
TicketSimonKingFri, 20 May 2011 06:25:39 GMT
https://trac.sagemath.org/ticket/7794#comment:5
https://trac.sagemath.org/ticket/7794#comment:5
<p>
Since the problem seems to be related with get_cparent, I was first looking into that. There was a bare "except:", where it should be catching an attribute error. But that was not the problem.
</p>
<p>
Next, I looked into polynomial_template.pxi, where get_cparent is frequently used. Too frequently, actually. In many methods it is called several times. So, it would be better to assign its output to a cdef'd variable -- that would actually boost the speed a lot!
</p>
<p>
Namely, working on other tickets, I know that the "harmless" method <code>modulus()</code> takes the most time when doing arithmetic with polynomials over finite fields. <code>modulus()</code> is called in get_cparent (actually, it is called twice!), and get_cparent is called several times in both _pow_ and _mul_ (as in polynomial_template.pxi).
</p>
<p>
So, one approach would be to remove the redundant calls - which makes sense anyway. If that does not help with the segfault, one could still try to wrap it into sig_on()/sig_off().
</p>
TicketSimonKingFri, 20 May 2011 06:39:22 GMT
https://trac.sagemath.org/ticket/7794#comment:6
https://trac.sagemath.org/ticket/7794#comment:6
<p>
It is really surprising how many redundant calls can be found in polynomial_template.pxi. I guess that's why get_cparent is defined as an inline cpdef function -- but that won't help if it internally calls pure Python functions!
</p>
TicketjdemeyerFri, 20 May 2011 08:41:25 GMTstatus changed
https://trac.sagemath.org/ticket/7794#comment:7
https://trac.sagemath.org/ticket/7794#comment:7
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
</ul>
<p>
The attached patch seems to fix the problem, but I haven't checked carefully whether it might break other stuff. I'm putting it to needs_review, but I haven't really thought about it much.
</p>
TicketSimonKingFri, 20 May 2011 12:13:35 GMT
https://trac.sagemath.org/ticket/7794#comment:8
https://trac.sagemath.org/ticket/7794#comment:8
<p>
Hi Jeroen,
</p>
<p>
Can you give me a pointer to the documentation of the <code>except? 0</code> syntax? Sounds useful.
</p>
<p>
I'll try your patch. I have prepared another patch, with the purpose of reducing the number of calls to get_cparent() (and thus to modulus()). But this will concern performance and should thus be on another ticket.
</p>
TicketSimonKingFri, 20 May 2011 12:18:01 GMT
https://trac.sagemath.org/ticket/7794#comment:9
https://trac.sagemath.org/ticket/7794#comment:9
<p>
I think a found a <a class="ext-link" href="http://docs.cython.org/src/userguide/language_basics.html#error-return-values"><span class="icon"></span>reference</a> to handling Cython errors.
</p>
TicketSimonKingFri, 20 May 2011 12:22:38 GMT
https://trac.sagemath.org/ticket/7794#comment:10
https://trac.sagemath.org/ticket/7794#comment:10
<p>
After reading the documentation, it seems to me that your approach makes very much sense. The original problem seems to be solved.
</p>
<p>
So, if the long doctests pass then I'll give it a positive review.
</p>
TicketSimonKingFri, 20 May 2011 12:33:12 GMT
https://trac.sagemath.org/ticket/7794#comment:11
https://trac.sagemath.org/ticket/7794#comment:11
<p>
Meanwhile I wonder:
</p>
<p>
In the original version of get_cparent, an attribute error was caught internally, and then zero was returned. Now, you return zero on error, and you do not catch the error internally. Couldn't that result in confusion, if the argument <code>parent</code> has no <code>modulus()</code> method?
</p>
<p>
Perhaps it is safer to keep modify the internal try-except clause (namely replace the bare <code>except:</code> by <code>except AttributeError:</code>) and declare the Cython funtion as <code>cdef inline cparent get_cparent(parent) except -1:</code>.
</p>
<p>
Namely, -1 can not result as a regular return value (the return value is the modulus of a ring, and thus a positive number), and so it would be safe to reserve -1 for errors (then you can also remove the question mark from <code>except?</code>).
</p>
<p>
Will you do this change (if you agree with my arguing), or shall I make the change in a separate patch?
</p>
TicketSimonKingFri, 20 May 2011 12:49:47 GMT
https://trac.sagemath.org/ticket/7794#comment:12
https://trac.sagemath.org/ticket/7794#comment:12
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/7794#comment:11" title="Comment 11">SimonKing</a>:
</p>
<blockquote class="citation">
<p>
Perhaps it is safer to keep modify the internal try-except clause (namely replace the bare <code>except:</code> by <code>except AttributeError:</code>) and declare the Cython funtion as <code>cdef inline cparent get_cparent(parent) except -1:</code>.
</p>
</blockquote>
<p>
On the other hand:
</p>
<ul><li>In polynomial_zmod_flint, get_cparent is supposed to return unsigned int. So, -1 does not make sense.
</li></ul><ul><li>The modulus is positive. So, actually 0 should be a clear sign that something went wrong. I wonder if the function is ever called with <code>parent=None</code> (because this is the only situation in which zero is returned).
</li></ul><p>
I will test whether <code>parent is None</code> ever occurs. If not, then we could keep 0 as return value on error, but could remove the question mark (which should result in a faster code).
</p>
TicketSimonKingFri, 20 May 2011 12:52:30 GMT
https://trac.sagemath.org/ticket/7794#comment:13
https://trac.sagemath.org/ticket/7794#comment:13
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/7794#comment:12" title="Comment 12">SimonKing</a>:
</p>
<blockquote class="citation">
<p>
I will test whether <code>parent is None</code> ever occurs. If not, then we could keep 0 as return value on error, but could remove the question mark (which should result in a faster code).
</p>
</blockquote>
<p>
Yes, it <em>is</em> called with <code>parent=None</code>, namely when unpickling.
</p>
TicketSimonKingFri, 20 May 2011 13:11:38 GMT
https://trac.sagemath.org/ticket/7794#comment:14
https://trac.sagemath.org/ticket/7794#comment:14
<p>
I considered the following version of get_cparent in polynomial_zmod_flint.pyx:
</p>
<pre class="wiki">cdef inline cparent get_cparent(parent) except 1:
if parent is None:
return 0
try:
return <unsigned long>(parent.modulus())
except AttributeError:
return 0
</pre><p>
Rationale:
</p>
<p>
The original code was catching an error that should probably be an attribute error - so, we do the same, and return the same value that was returned by the original version.
</p>
<p>
But the return value 1 can never occur (which I verified by the doctests of sage/rings/). So, it is safe to reserve 1 as the exceptional value, without question mark after <code>except</code>.
</p>
<p>
However, our two patch versions performed equally when I tried some timings. Hence, unless you say that I convinced you to use 1 as exceptional return value (without question mark), I will give your patch a positive review, provided the doc tests pass.
</p>
TicketSimonKingFri, 20 May 2011 13:49:33 GMTstatus changed; author set
https://trac.sagemath.org/ticket/7794#comment:15
https://trac.sagemath.org/ticket/7794#comment:15
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
<li><strong>author</strong>
set to <em>Jeroen Demeyer</em>
</li>
</ul>
<p>
Tests pass. So, let it be a positive review to your patch!
</p>
TicketjdemeyerFri, 20 May 2011 18:00:37 GMT
https://trac.sagemath.org/ticket/7794#comment:16
https://trac.sagemath.org/ticket/7794#comment:16
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/7794#comment:14" title="Comment 14">SimonKing</a>:
</p>
<blockquote class="citation">
<p>
But the return value 1 can never occur (which I verified by the doctests of sage/rings/). So, it is safe to reserve 1 as the exceptional value, without question mark after <code>except</code>.
</p>
</blockquote>
<p>
Are you <strong>very sure</strong> that 1 cannot occur? You might be right, but I would not count on it.
</p>
TicketjdemeyerFri, 20 May 2011 18:05:12 GMT
https://trac.sagemath.org/ticket/7794#comment:17
https://trac.sagemath.org/ticket/7794#comment:17
<p>
you are probably right about <code>except AttributeError</code>. How about
</p>
<pre class="wiki">cdef inline cparent get_cparent(parent) except? 0:
try:
return <unsigned long>(parent.modulus())
except AttributeError:
return 0
</pre><p>
This will also catch the case <code>parent == None</code>.
</p>
TicketSimonKingSat, 21 May 2011 06:17:15 GMTstatus changed; work_issues set
https://trac.sagemath.org/ticket/7794#comment:18
https://trac.sagemath.org/ticket/7794#comment:18
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>needs_work</em>
</li>
<li><strong>work_issues</strong>
set to <em>catch attribute error</em>
</li>
</ul>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/7794#comment:17" title="Comment 17">jdemeyer</a>:
</p>
<blockquote class="citation">
<p>
you are probably right about <code>except AttributeError</code>. How about
</p>
<pre class="wiki">cdef inline cparent get_cparent(parent) except? 0:
try:
return <unsigned long>(parent.modulus())
except AttributeError:
return 0
</pre><p>
This will also catch the case <code>parent == None</code>.
</p>
</blockquote>
<p>
Yes, that looks a bit better.
</p>
<p>
Concerning modulus 1:
</p>
<p>
It is possible to explicitly construct an instance of <code>sage.rings.polynomial.polynomial_zmod_flint.Polynomial_zmod_flint</code> whose parent has modulus 1:
</p>
<pre class="wiki">sage: from sage.rings.polynomial.polynomial_ring import PolynomialRing_dense_mod_n
sage: P = PolynomialRing_dense_mod_n(Zmod(1),'x')
sage: type(P.0)
<type 'sage.rings.polynomial.polynomial_zmod_flint.Polynomial_zmod_flint'>
</pre><p>
So, in order to be on the safe side, one should not have <code>get_cparent(parent) except 1:</code>.
</p>
<p>
One may argue, however, that it can not occur if one constructs a ring as one is supposed to, namely by calling the polynomial ring constructor:
</p>
<pre class="wiki">sage: P.<x> = Zmod(1)[]
sage: type(x)
<type 'sage.rings.polynomial.polynomial_element.Polynomial_generic_dense'>
</pre><p>
But better safe than sorry, in particular since your safe solution (<code>... except? 0:</code>) should have no serious time penalty.
</p>
<p>
Namely, as much as I understand, <code>cdef get_cparent(parent) except? 0:</code> will slow things down only if the return value is 0, because then it will be tested whether an error has actually occured.
</p>
<p>
But, to the best of my knowledge, it is <em>impossible</em> to have a parent with modulus zero, even if one works around the polynomial ring constructor:
</p>
<pre class="wiki">sage: P = PolynomialRing_dense_mod_n(ZZ,'x')
Traceback (most recent call last):
...
ValueError: invalid integer: +Infinity
</pre><p>
So, if one has return value zero then something exceptional must be happening: Either it is a real error, or it is during pickling, when <code>parent</code> is None. So, we can certainly afford to test for an error in that situation.
</p>
<p>
Can you update your patch, so that the attribute error is caught? Then I'll run the tests again, and will hoppefully be able to renew my positive review.
</p>
TicketjdemeyerSat, 21 May 2011 06:23:19 GMTattachment set
https://trac.sagemath.org/ticket/7794
https://trac.sagemath.org/ticket/7794
<ul>
<li><strong>attachment</strong>
set to <em>7794_get_cparent.patch</em>
</li>
</ul>
TicketjdemeyerSat, 21 May 2011 06:24:49 GMTstatus changed
https://trac.sagemath.org/ticket/7794#comment:19
https://trac.sagemath.org/ticket/7794#comment:19
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
</ul>
TicketjdemeyerSat, 21 May 2011 06:24:57 GMTwork_issues deleted
https://trac.sagemath.org/ticket/7794#comment:20
https://trac.sagemath.org/ticket/7794#comment:20
<ul>
<li><strong>work_issues</strong>
<em>catch attribute error</em> deleted
</li>
</ul>
TicketSimonKingSat, 21 May 2011 09:04:59 GMT
https://trac.sagemath.org/ticket/7794#comment:21
https://trac.sagemath.org/ticket/7794#comment:21
<p>
All long tests passed with your new patch.
</p>
<p>
I tried to add a doc test, as follows:
</p>
<pre class="wiki"> sage: def test():
... P = GF(3)['T']
... PP = P['z']
... T = P.gen()
... z = PP.gen()
... alarm(1)
... try:
... while(1):
... x = (z**3+T*z)**243
... except KeyboardInterrupt:
... print "First interrupt caught"
... alarm(1)
... try:
... while(1):
... x = (z**3+T*z)**243
... except KeyboardInterrupt:
... print "Second interrupt caught"
sage: test()
First interrupt caught
Second interrupt caught
</pre><p>
However, it did not work, since there seem to be other places in which errors are not properly caught. Namely, I occasionally got:
</p>
<pre class="wiki">sage: test()
First interrupt caught
Exception KeyboardInterrupt: KeyboardInterrupt('computation timed out because alarm was set for 1 seconds',) in T^79 + 2*T^78 + 2*T^77 + T^74 ignored
^CSecond interrupt caught
</pre><p>
So, the first alarm worked, but the second didn't, and only when I pressed <ctrl>-c then the computation was interrupted.
</p>
<p>
What shall we do now? Clearly, your patch is a progress. So, I tend to give it a positive review (because all tests pass), and deal with the remaining problem on a different ticket.
</p>
<p>
What do you think?
</p>
TicketjdemeyerMon, 23 May 2011 10:38:19 GMT
https://trac.sagemath.org/ticket/7794#comment:22
https://trac.sagemath.org/ticket/7794#comment:22
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/7794#comment:21" title="Comment 21">SimonKing</a>:
</p>
<blockquote class="citation">
<p>
What do you think?
</p>
</blockquote>
<p>
I'll see if I can identify the problem.
</p>
TicketjdemeyerMon, 23 May 2011 15:09:19 GMTstatus changed
https://trac.sagemath.org/ticket/7794#comment:23
https://trac.sagemath.org/ticket/7794#comment:23
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
</ul>
<p>
I cannot easily identify the problem, so let's put it to positive_review (even though I agree the isuue is not 100% solved).
</p>
TicketjdemeyerTue, 24 May 2011 07:52:00 GMT
https://trac.sagemath.org/ticket/7794#comment:24
https://trac.sagemath.org/ticket/7794#comment:24
<p>
I have tracked the <code>Exception KeyboardInterrupt... ignored</code> problem
down to the following Cython-generated code in <code>sage/rings/polynomial/polynomial_zmod_flint.c</code>:
</p>
<pre class="wiki">static void __pyx_tp_dealloc_4sage_5rings_10polynomial_21polynomial_zmod_flint_Polynomial_template(PyObject *o) {
{
PyObject *etype, *eval, *etb;
PyErr_Fetch(&etype, &eval, &etb);
++Py_REFCNT(o);
__pyx_pf_4sage_5rings_10polynomial_21polynomial_zmod_flint_19Polynomial_template_3__dealloc__(o);
if (PyErr_Occurred()) PyErr_WriteUnraisable(o);
--Py_REFCNT(o);
PyErr_Restore(etype, eval, etb);
}
__pyx_ptype_4sage_5rings_10polynomial_18polynomial_element_Polynomial->tp_dealloc(o);
}
</pre><p>
The problem is the line
</p>
<pre class="wiki">if (PyErr_Occurred()) PyErr_WriteUnraisable(o);
</pre><p>
Since this code is generated by Cython, perhaps it is not possible to solve this problem without patching Cython.
</p>
TicketjdemeyerTue, 24 May 2011 09:06:58 GMT
https://trac.sagemath.org/ticket/7794#comment:25
https://trac.sagemath.org/ticket/7794#comment:25
<p>
I believe the "Exception ignored" to be harmless, in the sense that it's not going to break anything or cause segmentation faults. If the exception is ignored, the computation simply continues and the user can press ctrl-C a second time and that should do it.
</p>
TicketSimonKingTue, 24 May 2011 09:23:16 GMT
https://trac.sagemath.org/ticket/7794#comment:26
https://trac.sagemath.org/ticket/7794#comment:26
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/7794#comment:25" title="Comment 25">jdemeyer</a>:
</p>
<blockquote class="citation">
<p>
I believe the "Exception ignored" to be harmless, in the sense that it's not going to break anything or cause segmentation faults. If the exception is ignored, the computation simply continues and the user can press ctrl-C a second time and that should do it.
</p>
</blockquote>
<p>
Yes, but it made it impossible for me to create a doctest for your fix!
</p>
TicketjdemeyerMon, 30 May 2011 14:52:02 GMTstatus changed; resolution, merged set
https://trac.sagemath.org/ticket/7794#comment:27
https://trac.sagemath.org/ticket/7794#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>sage-4.7.1.alpha2</em>
</li>
</ul>
Ticket