Opened 9 years ago

Closed 4 years ago

#10476 closed defect (fixed)

Problems interrupting Singular

Reported by: jdemeyer Owned by: jdemeyer
Priority: critical Milestone: sage-6.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 jdemeyer)

Type the following:

sage: singular._expect_expr("1")

Now type CTRL-C. 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.uni-kl.de:8002/trac/ticket/730

Attachments (2)

singular-3-1-3-3.p7.diff (8.4 KB) - added by jdemeyer 7 years ago.
Diff for the singular spkg. For reference / review only.
10476_singular_interrupt.patch (3.9 KB) - added by jdemeyer 7 years ago.

Download all attachments as: .zip

Change History (57)

comment:1 Changed 9 years ago by jdemeyer

I suspect this is behind it, start Singular and press CTRL-C:

$ sage -singular
                     SINGULAR                             /  Development
 A Computer Algebra System for Polynomial Computations   /   version 3-1-1
                                                       0<
     by: G.-M. Greuel, G. Pfister, H. Schoenemann        \   Feb 2010
FB Mathematik der Universitaet, D-67653 Kaiserslautern    \
> ^C// ** Interrupt at cmd:`$INVALID$` in line:''
abort command(a), continue(c) or quit Singular(q) ?

So we get a stupid prompt instead.

comment:2 Changed 8 years ago by vbraun

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 ctrl-c 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 7 years ago by jdemeyer

  • Priority changed from major to blocker

comment:4 Changed 7 years ago by jdemeyer

  • Keywords sd40.5 added

comment:5 Changed 7 years ago by jdemeyer

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 7 years ago by jdemeyer

Diff for the singular spkg. For reference / review only.

comment:6 Changed 7 years ago by jdemeyer

  • Authors set to Jeroen Demeyer
  • Description modified (diff)
  • Status changed from new to needs_review

comment:7 Changed 7 years ago by jdemeyer

  • Cc malb burcin added

Changed 7 years ago by jdemeyer

comment:8 Changed 7 years ago by jdemeyer

  • Description modified (diff)
  • Report Upstream changed from N/A to Reported upstream. No feedback yet.

comment:9 follow-up: Changed 7 years ago by vbraun

  • 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/sage-5.1.beta5/devel/sage/sage/interfaces/singular.py", line 480:
    sage: 2*a
Exception raised:
    Traceback (most recent call last):
      File "/home/vbraun/opt/sage-5.1.beta5/local/bin/ncadoctest.py", line 1231, in run_one_test
        self.run_one_example(test, example, filename, compileflags)
      File "/home/vbraun/opt/sage-5.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/sage-5.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/sage-5.1.beta5/local/lib/python/site-packages/sage/interfaces/interface.py", line 1118, in _mul_
        return self._operation('*', right)
      File "/home/vbraun/opt/sage-5.1.beta5/local/lib/python/site-packages/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 7 years ago by jdemeyer

  • Milestone changed from sage-5.1 to sage-5.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 sage-5.1.

comment:11 Changed 7 years ago by jdemeyer

  • Report Upstream changed from Reported upstream. No feedback yet. to Fixed upstream, but not in a stable release.

comment:12 Changed 7 years ago by jdemeyer

  • 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 7 years ago by jdemeyer

  • Dependencies set to #13237
  • Description modified (diff)

comment:14 Changed 7 years ago by jdemeyer

  • Priority changed from blocker to critical

comment:15 Changed 6 years ago by jdemeyer

  • Milestone changed from sage-5.11 to sage-5.12

comment:16 Changed 5 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:17 Changed 5 years ago by vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:18 in reply to: ↑ 9 Changed 5 years ago by rws

  • 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

Replying to vbraun:

Running the doctest in a loop, I get occasional (maybe 1 in 20) failures

This is a good estimate of the rate when the issue leads to patchbot false test failures. But #13237 was fixed 21 months ago, so obviously they too didn't fix it completely.

comment:19 Changed 5 years ago by rws

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 follow-up: Changed 5 years ago by vbraun

#15178 might improve things a bit (will be in beta3)

comment:21 in reply to: ↑ 20 Changed 5 years ago by jdemeyer

Replying to vbraun:

#15178 might improve things a bit (will be in beta3)

I don't see how that could be related.

comment:22 Changed 5 years ago by vbraun

Since the cntrlc=a patch is now in upstream we should at least make use of the command line option...

comment:23 Changed 5 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4

comment:24 Changed 4 years ago by jakobkroeker

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 4 years ago by jdemeyer

  • Report Upstream changed from Not yet reported upstream; Will do shortly. to Completely fixed; Fix reported upstream

I'm in pexpect mood anyway (#17686, #17704), so I might as well have a look at this.

comment:26 Changed 4 years ago by jdemeyer

  • Dependencies changed from #13237 to #17704, #17718
  • Status changed from needs_info to needs_work

comment:27 Changed 4 years ago by jdemeyer

  • Dependencies changed from #17704, #17718 to #17704, #17718, #17719

comment:28 Changed 4 years ago by chapoton

  • Branch set to public/ticket/10476
  • Commit set to ac310c70781ee1d9eab425e5edc97c94e481825b

New commits:

ac310c7Fix interrupting Singular

comment:29 Changed 4 years ago by chapoton

  • Milestone changed from sage-6.4 to sage-6.7

comment:30 Changed 4 years ago by git

  • Commit changed from ac310c70781ee1d9eab425e5edc97c94e481825b to 79bce4d264e30010e714ebc6bfa1f120a1bdf958

Branch pushed to git repo; I updated commit sha1. New commits:

1d3289cMerge branch 'public/ticket/10476' into 6.7
79bce4dtrac #10476 doc formatting

comment:31 Changed 4 years ago by chapoton

There is one failing doctest in the singular interface.

comment:32 Changed 4 years ago by jdemeyer

I would rather have #17686 in Sage first before continuing on this ticket.

comment:33 Changed 4 years ago by vbraun

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 4 years ago by jdemeyer

Good, so we're making progress!

The correct result of that doctest is True.

comment:35 Changed 4 years ago by rbeezer

  • Cc rbeezer added

comment:36 Changed 4 years ago by jdemeyer

  • Dependencies #17704, #17718, #17719 deleted
  • Description modified (diff)

comment:37 Changed 4 years ago by git

  • Commit changed from 79bce4d264e30010e714ebc6bfa1f120a1bdf958 to 42197f66f8fbbfd882630504ec101095287a706a

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

42197f6Fix interrupting Singular

comment:38 Changed 4 years ago by jdemeyer

  • Description modified (diff)
  • Report Upstream changed from Completely fixed; Fix reported upstream to Reported upstream. No feedback yet.

comment:39 Changed 4 years ago by git

  • Commit changed from 42197f66f8fbbfd882630504ec101095287a706a to aa4a15654ddc1c3f999428d3b0d9ca334c384f6a

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

aa4a156Fix interrupting Singular

comment:40 Changed 4 years ago by jdemeyer

  • Dependencies set to #18771
  • Description modified (diff)

comment:41 Changed 4 years ago by git

  • Commit changed from aa4a15654ddc1c3f999428d3b0d9ca334c384f6a to a4f2d83238a2c25400648a794a94830f2fa49ad4

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

45969a6Clean up interrupt()
a4f2d83Fix interrupting Singular

comment:42 Changed 4 years ago by jdemeyer

  • Description modified (diff)

comment:43 Changed 4 years ago by git

  • Commit changed from a4f2d83238a2c25400648a794a94830f2fa49ad4 to 17d23e9f81355875d60944359c67815c6acddaad

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

17d23e9Fix interrupting Singular

comment:44 Changed 4 years ago by jdemeyer

  • Status changed from needs_work to needs_review

comment:45 Changed 4 years ago by kdilks

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 4 years ago by git

  • Commit changed from 17d23e9f81355875d60944359c67815c6acddaad to 02decd819ed4f6039b39a8b27592d51ed97b63ef

Branch pushed to git repo; I updated commit sha1. New commits:

02decd8Fix documentation

comment:47 Changed 4 years ago by kdilks

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 4 years ago by chapoton

  • Milestone changed from sage-6.7 to sage-6.8

comment:49 Changed 4 years ago by wluebbe

I tried ./sage -t src/sage/interfaces/expect.py on sage-6.8.beta7 and Ubuntu 15.04 a dozen times: it failed in about 50% of the runs (seemingly at random).

comment:50 Changed 4 years ago by kdilks

Upstream bug ticket has been updated.

comment:51 Changed 4 years ago by jdemeyer

  • Description modified (diff)

comment:52 Changed 4 years ago by jdemeyer

  • Report Upstream changed from Reported upstream. No feedback yet. to Reported upstream. Developers deny it's a bug.

comment:53 follow-up: Changed 4 years ago by vbraun

  • 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 4 years ago by jdemeyer

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 4 years ago by vbraun

  • Branch changed from public/ticket/10476 to 02decd819ed4f6039b39a8b27592d51ed97b63ef
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.