Opened 10 years ago
Closed 5 years ago
#10476 closed defect (fixed)
Problems interrupting Singular
Reported by:  jdemeyer  Owned by:  jdemeyer 

Priority:  critical  Milestone:  sage6.8 
Component:  interfaces  Keywords:  interrupt singular expect sd40.5 patchbot 
Cc:  malb, burcin, rbeezer  Merged in:  
Authors:  Jeroen Demeyer  Reviewers:  Volker Braun 
Report Upstream:  Reported upstream. Developers deny it's a bug.  Work issues:  
Branch:  02decd8 (Commits)  Commit:  02decd819ed4f6039b39a8b27592d51ed97b63ef 
Dependencies:  #18771  Stopgaps: 
Description (last modified by )
Type the following:
sage: singular._expect_expr("1")
Now type CTRLC
. It takes a very long time before something happens. This was the cause of #9163 (which was fixed to avoid this issue).
Upstream bug: http://www.singular.unikl.de:8002/trac/ticket/730
Attachments (2)
Change History (57)
comment:1 Changed 10 years ago by
comment:2 Changed 10 years ago by
I find that it takes about 5 seconds, not "very long". On a closer look, it actually takes 6 seconds = 20*0.3:
Definition: singular.interrupt(self, tries=20, timeout=0.29999999999999999, quit_on_fail=True) Source: def interrupt(self, tries=20, timeout=0.3, quit_on_fail=True): E = self._expect if E is None: return True success = False try: for i in range(tries): E.sendline(self._quit_string()) E.sendline(chr(3)) try: E.expect(self._prompt, timeout=timeout) success= True break except (pexpect.TIMEOUT, pexpect.EOF), msg: #print msg pass except Exception, msg: pass if success: pass elif quit_on_fail: self.quit() return success
 The inherited method
interrupt()
actually tries to quit Singular (self._quit_string()=="quit"
)  If quitting did not work the first time, is it going to work the next 19 times?
 From personal experience, I find that actually interrupting (using ctrlc and "continue") Singular often does not work. Restarting is very fast, though.
I propose that the Singular pexpect interface override interrupt()
to be a synonym for quit()
.
comment:3 Changed 9 years ago by
 Priority changed from major to blocker
comment:4 Changed 9 years ago by
 Keywords sd40.5 added
comment:5 Changed 9 years ago by
I'm having a hard time fixing this with pexpect, it's too unreliable. Next strategy is patching Singular not to show the stupid interrupt prompt...
comment:6 Changed 8 years ago by
 Description modified (diff)
 Status changed from new to needs_review
comment:7 Changed 8 years ago by
 Cc malb burcin added
Changed 8 years ago by
comment:8 Changed 8 years ago by
 Description modified (diff)
 Report Upstream changed from N/A to Reported upstream. No feedback yet.
comment:9 followup: ↓ 18 Changed 8 years ago by
 Reviewers set to Volker Braun
 Status changed from needs_review to needs_work
Running the doctest in a loop, I get occasional (maybe 1 in 20) failures
sage t force_lib "devel/sage/sage/interfaces/singular.py" ********************************************************************** File "/home/vbraun/opt/sage5.1.beta5/devel/sage/sage/interfaces/singular.py", line 480: sage: 2*a Exception raised: Traceback (most recent call last): File "/home/vbraun/opt/sage5.1.beta5/local/bin/ncadoctest.py", line 1231, in run_one_test self.run_one_example(test, example, filename, compileflags) File "/home/vbraun/opt/sage5.1.beta5/local/bin/sagedoctest.py", line 38, in run_one_example OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags) File "/home/vbraun/opt/sage5.1.beta5/local/bin/ncadoctest.py", line 1172, in run_one_example compileflags, 1) in test.globs File "<doctest __main__.example_9[6]>", line 1, in <module> Integer(2)*a###line 480: sage: 2*a File "element.pyx", line 1687, in sage.structure.element.RingElement.__mul__ (sage/structure/element.c:12755) File "coerce.pyx", line 749, in sage.structure.coerce.CoercionModel_cache_maps.bin_op (sage/structure/coerce.c:6778) File "coerce.pyx", line 745, in sage.structure.coerce.CoercionModel_cache_maps.bin_op (sage/structure/coerce.c:6718) File "element.pyx", line 1682, in sage.structure.element.RingElement.__mul__ (sage/structure/element.c:12681) File "element.pyx", line 1689, in sage.structure.element.RingElement._mul_ (sage/structure/element.c:12807) File "/home/vbraun/opt/sage5.1.beta5/local/lib/python/sitepackages/sage/interfaces/interface.py", line 1118, in _mul_ return self._operation('*', right) File "/home/vbraun/opt/sage5.1.beta5/local/lib/python/sitepackages/sage/interfaces/interface.py", line 1075, in _operation raise TypeError, msg TypeError: Singular error: ? `sage449` is not defined ? error occurred in or before STDIN line 11: `def sage458=sage450 * sage449;` ********************************************************************** 1 items had failures: 1 of 8 in __main__.example_9
comment:10 Changed 8 years ago by
 Milestone changed from sage5.1 to sage5.2
Well, improving the function from working less than 5% of the time to about 95% of the time isn't bad :)
But you're right that this needs work. Since I will have to make some more serious changes, let's not do it anymore in sage5.1.
comment:11 Changed 8 years ago by
 Report Upstream changed from Reported upstream. No feedback yet. to Fixed upstream, but not in a stable release.
comment:12 Changed 8 years ago by
 Owner changed from was to jdemeyer
 Report Upstream changed from Fixed upstream, but not in a stable release. to Fixed upstream, in a later stable release.
comment:13 Changed 8 years ago by
 Dependencies set to #13237
 Description modified (diff)
comment:14 Changed 8 years ago by
 Priority changed from blocker to critical
comment:15 Changed 7 years ago by
 Milestone changed from sage5.11 to sage5.12
comment:16 Changed 7 years ago by
 Milestone changed from sage6.1 to sage6.2
comment:17 Changed 7 years ago by
 Milestone changed from sage6.2 to sage6.3
comment:18 in reply to: ↑ 9 Changed 7 years ago by
 Keywords patchbot added
 Report Upstream changed from Fixed upstream, in a later stable release. to Not yet reported upstream; Will do shortly.
 Status changed from needs_work to needs_info
comment:19 Changed 7 years ago by
After 6.3beta2 the rate of failure under Ubuntu+patchbot (both SageMath
/Google
cloud) has gone up significantly to the order of 50%.
File "src/sage/interfaces/expect.py", line 790, in sage.interfaces.expect.Expect._eval_line Failed example: singular.interrupt(timeout=3) # sometimes very slow (up to 60s on sage.math, 2012) Expected: False Got: True
comment:20 followup: ↓ 21 Changed 7 years ago by
#15178 might improve things a bit (will be in beta3)
comment:21 in reply to: ↑ 20 Changed 6 years ago by
comment:22 Changed 6 years ago by
Since the cntrlc=a
patch is now in upstream we should at least make use of the command line option...
comment:23 Changed 6 years ago by
 Milestone changed from sage6.3 to sage6.4
comment:24 Changed 6 years ago by
Since the cntrlc=a patch is now in upstream we should at least make use of the command line option
That should be easy now, since the patch is in 3.1.7 and 3.1.7 is in sage! I just do not know where this change has to be done.
Following choices for cntrlc= are possible: a,r,b,c,q :
abort after this command(a), abort immediately(r), print backtrace(b), continue(c) or quit Singular(q) ?
comment:25 Changed 6 years ago by
 Report Upstream changed from Not yet reported upstream; Will do shortly. to Completely fixed; Fix reported upstream
comment:26 Changed 6 years ago by
 Dependencies changed from #13237 to #17704, #17718
 Status changed from needs_info to needs_work
comment:27 Changed 6 years ago by
 Dependencies changed from #17704, #17718 to #17704, #17718, #17719
comment:28 Changed 6 years ago by
 Branch set to public/ticket/10476
 Commit set to ac310c70781ee1d9eab425e5edc97c94e481825b
New commits:
ac310c7  Fix interrupting Singular

comment:29 Changed 6 years ago by
 Milestone changed from sage6.4 to sage6.7
comment:30 Changed 6 years ago by
 Commit changed from ac310c70781ee1d9eab425e5edc97c94e481825b to 79bce4d264e30010e714ebc6bfa1f120a1bdf958
comment:31 Changed 6 years ago by
There is one failing doctest in the singular interface.
comment:32 Changed 6 years ago by
I would rather have #17686 in Sage first before continuing on this ticket.
comment:33 Changed 5 years ago by
This seems to be much more frequent now with #17686
sage t long src/sage/interfaces/expect.py ********************************************************************** File "src/sage/interfaces/expect.py", line 823, in sage.interfaces.expect.Expect._eval_line Failed example: singular.interrupt(timeout=3) # sometimes very slow (up to 60s on sage.math, 2012) Expected: False Got: True ********************************************************************** 1 item had failures: 1 of 15 in sage.interfaces.expect.Expect._eval_line [89 tests, 1 failure, 4.74 s]
comment:34 Changed 5 years ago by
Good, so we're making progress!
The correct result of that doctest is True
.
comment:35 Changed 5 years ago by
 Cc rbeezer added
comment:36 Changed 5 years ago by
 Dependencies #17704, #17718, #17719 deleted
 Description modified (diff)
comment:37 Changed 5 years ago by
 Commit changed from 79bce4d264e30010e714ebc6bfa1f120a1bdf958 to 42197f66f8fbbfd882630504ec101095287a706a
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
42197f6  Fix interrupting Singular

comment:38 Changed 5 years ago by
 Description modified (diff)
 Report Upstream changed from Completely fixed; Fix reported upstream to Reported upstream. No feedback yet.
comment:39 Changed 5 years ago by
 Commit changed from 42197f66f8fbbfd882630504ec101095287a706a to aa4a15654ddc1c3f999428d3b0d9ca334c384f6a
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
aa4a156  Fix interrupting Singular

comment:40 Changed 5 years ago by
 Dependencies set to #18771
 Description modified (diff)
comment:41 Changed 5 years ago by
 Commit changed from aa4a15654ddc1c3f999428d3b0d9ca334c384f6a to a4f2d83238a2c25400648a794a94830f2fa49ad4
comment:42 Changed 5 years ago by
 Description modified (diff)
comment:43 Changed 5 years ago by
 Commit changed from a4f2d83238a2c25400648a794a94830f2fa49ad4 to 17d23e9f81355875d60944359c67815c6acddaad
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
17d23e9  Fix interrupting Singular

comment:44 Changed 5 years ago by
 Status changed from needs_work to needs_review
comment:45 Changed 5 years ago by
Minor doc error:
OSError: [interface] docstring of sage.interfaces.sagespawn.SageSpawn.expect_peek:12: WARNING: Block quote ends without a blank line; unexpected unindent.
comment:46 Changed 5 years ago by
 Commit changed from 17d23e9f81355875d60944359c67815c6acddaad to 02decd819ed4f6039b39a8b27592d51ed97b63ef
Branch pushed to git repo; I updated commit sha1. New commits:
02decd8  Fix documentation

comment:47 Changed 5 years ago by
The documentation builds now.
Not a thorough review of the code, but when repeatedly doctesting the file ./src/sage/interfaces/expect.py
on 6.8beta5, I can get singular.interrupt(timeout=3)
to give an error every hundred or so times. With this patch applied, I've all had all tests pass a few thousand times.
comment:48 Changed 5 years ago by
 Milestone changed from sage6.7 to sage6.8
comment:49 Changed 5 years ago by
I tried ./sage t src/sage/interfaces/expect.py
on sage6.8.beta7 and Ubuntu 15.04 a dozen times: it failed in about 50% of the runs (seemingly at random).
comment:50 Changed 5 years ago by
Upstream bug ticket has been updated.
comment:51 Changed 5 years ago by
 Description modified (diff)
comment:52 Changed 5 years ago by
 Report Upstream changed from Reported upstream. No feedback yet. to Reported upstream. Developers deny it's a bug.
comment:53 followup: ↓ 54 Changed 5 years ago by
 Status changed from needs_review to positive_review
Well they don't really deny its a bug, just difficult to fix in the current architecture...
comment:54 in reply to: ↑ 53 Changed 5 years ago by
Replying to vbraun:
Well they don't really deny its a bug, just difficult to fix in the current architecture...
I sort of agree, but they did say it's considered a "proposed feature".
comment:55 Changed 5 years ago by
 Branch changed from public/ticket/10476 to 02decd819ed4f6039b39a8b27592d51ed97b63ef
 Resolution set to fixed
 Status changed from positive_review to closed
I suspect this is behind it, start Singular and press
CTRLC
:So we get a stupid prompt instead.