Opened 12 years ago
Last modified 7 years ago
#10476 closed defect
Problems interrupting Singular — at Version 51
Reported by:  Jeroen Demeyer  Owned by:  Jeroen Demeyer 

Priority:  critical  Milestone:  sage6.8 
Component:  interfaces  Keywords:  interrupt singular expect sd40.5 patchbot 
Cc:  Martin Albrecht, Burcin Erocal, Rob Beezer  Merged in:  
Authors:  Jeroen Demeyer  Reviewers:  Volker Braun 
Report Upstream:  Reported upstream. Developers deny it's a bug.  Work issues:  
Branch:  public/ticket/10476 (Commits, GitHub, GitLab)  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
Change History (53)
comment:1 Changed 12 years ago by
comment:2 Changed 12 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 10 years ago by
Priority:  major → blocker 

comment:4 Changed 10 years ago by
Keywords:  sd40.5 added 

comment:5 Changed 10 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...
Changed 10 years ago by
Attachment:  singular3133.p7.diff added 

Diff for the singular spkg. For reference / review only.
comment:6 Changed 10 years ago by
Authors:  → Jeroen Demeyer 

Description:  modified (diff) 
Status:  new → needs_review 
comment:7 Changed 10 years ago by
Cc:  Martin Albrecht Burcin Erocal added 

Changed 10 years ago by
Attachment:  10476_singular_interrupt.patch added 

comment:8 Changed 10 years ago by
Description:  modified (diff) 

Report Upstream:  N/A → Reported upstream. No feedback yet. 
comment:9 followup: 18 Changed 10 years ago by
Reviewers:  → Volker Braun 

Status:  needs_review → 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 10 years ago by
Milestone:  sage5.1 → 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 10 years ago by
Report Upstream:  Reported upstream. No feedback yet. → Fixed upstream, but not in a stable release. 

comment:12 Changed 10 years ago by
Owner:  changed from William Stein to Jeroen Demeyer 

Report Upstream:  Fixed upstream, but not in a stable release. → Fixed upstream, in a later stable release. 
comment:13 Changed 10 years ago by
Dependencies:  → #13237 

Description:  modified (diff) 
comment:14 Changed 10 years ago by
Priority:  blocker → critical 

comment:15 Changed 9 years ago by
Milestone:  sage5.11 → sage5.12 

comment:16 Changed 9 years ago by
Milestone:  sage6.1 → sage6.2 

comment:17 Changed 8 years ago by
Milestone:  sage6.2 → sage6.3 

comment:18 Changed 8 years ago by
Keywords:  patchbot added 

Report Upstream:  Fixed upstream, in a later stable release. → Not yet reported upstream; Will do shortly. 
Status:  needs_work → needs_info 
comment:19 Changed 8 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 8 years ago by
#15178 might improve things a bit (will be in beta3)
comment:21 Changed 8 years ago by
comment:22 Changed 8 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 8 years ago by
Milestone:  sage6.3 → sage6.4 

comment:24 Changed 8 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 8 years ago by
Report Upstream:  Not yet reported upstream; Will do shortly. → Completely fixed; Fix reported upstream 

comment:26 Changed 8 years ago by
Dependencies:  #13237 → #17704, #17718 

Status:  needs_info → needs_work 
comment:27 Changed 8 years ago by
Dependencies:  #17704, #17718 → #17704, #17718, #17719 

comment:28 Changed 7 years ago by
Branch:  → public/ticket/10476 

Commit:  → ac310c70781ee1d9eab425e5edc97c94e481825b 
New commits:
ac310c7  Fix interrupting Singular

comment:29 Changed 7 years ago by
Milestone:  sage6.4 → sage6.7 

comment:30 Changed 7 years ago by
Commit:  ac310c70781ee1d9eab425e5edc97c94e481825b → 79bce4d264e30010e714ebc6bfa1f120a1bdf958 

comment:32 Changed 7 years ago by
I would rather have #17686 in Sage first before continuing on this ticket.
comment:33 Changed 7 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 7 years ago by
Good, so we're making progress!
The correct result of that doctest is True
.
comment:35 Changed 7 years ago by
Cc:  Rob Beezer added 

comment:36 Changed 7 years ago by
Dependencies:  #17704, #17718, #17719 

Description:  modified (diff) 
comment:37 Changed 7 years ago by
Commit:  79bce4d264e30010e714ebc6bfa1f120a1bdf958 → 42197f66f8fbbfd882630504ec101095287a706a 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
42197f6  Fix interrupting Singular

comment:38 Changed 7 years ago by
Description:  modified (diff) 

Report Upstream:  Completely fixed; Fix reported upstream → Reported upstream. No feedback yet. 
comment:39 Changed 7 years ago by
Commit:  42197f66f8fbbfd882630504ec101095287a706a → aa4a15654ddc1c3f999428d3b0d9ca334c384f6a 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
aa4a156  Fix interrupting Singular

comment:40 Changed 7 years ago by
Dependencies:  → #18771 

Description:  modified (diff) 
comment:41 Changed 7 years ago by
Commit:  aa4a15654ddc1c3f999428d3b0d9ca334c384f6a → a4f2d83238a2c25400648a794a94830f2fa49ad4 

comment:42 Changed 7 years ago by
Description:  modified (diff) 

comment:43 Changed 7 years ago by
Commit:  a4f2d83238a2c25400648a794a94830f2fa49ad4 → 17d23e9f81355875d60944359c67815c6acddaad 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
17d23e9  Fix interrupting Singular

comment:44 Changed 7 years ago by
Status:  needs_work → needs_review 

comment:45 Changed 7 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 7 years ago by
Commit:  17d23e9f81355875d60944359c67815c6acddaad → 02decd819ed4f6039b39a8b27592d51ed97b63ef 

Branch pushed to git repo; I updated commit sha1. New commits:
02decd8  Fix documentation

comment:47 Changed 7 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 7 years ago by
Milestone:  sage6.7 → sage6.8 

comment:49 Changed 7 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:51 Changed 7 years ago by
Description:  modified (diff) 

I suspect this is behind it, start Singular and press
CTRLC
:So we get a stupid prompt instead.