Opened 11 years ago

Closed 11 years ago

#9959 closed defect (fixed)

Get PARI/GP to stop starting automatically

Reported by: kcrisman Owned by: tbd
Priority: blocker Milestone: sage-4.6
Component: packages: standard Keywords:
Cc: mpatel, jdemeyer, leif Merged in: sage-4.6.rc0
Authors: Jeroen Demeyer Reviewers: John Cremona
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by jdemeyer)

I still am getting the known behavior below when exiting Sage.

Exiting Sage (CPU time 0m0.74s, Wall time 11m11.95s). 
Exiting spawned GP/PARI interpreter process. 

It's not exactly a bug, but it's also annoying and potentially confusing. We should stop it.

Apply 9959_combined.patch below and install the new spkg: http://sage.math.washington.edu/home/jdemeyer/spkg/pari-2.4.3.svn-12577.p9.spkg (note that this spkg is not based on the earlier p8)

Attachments (8)

9959_rename_pari_gp.patch (9.4 KB) - added by jdemeyer 11 years ago.
Renames "GP/PARI" to "PARI/GP"
9959_remove_breakloop.patch (1.1 KB) - added by jdemeyer 11 years ago.
Don't set the 'breakloop' default in Sage
9959_gp_error_doctest.patch (843 bytes) - added by jdemeyer 11 years ago.
Add doctest for error recovery
pari.p8.patch (2.5 KB) - added by jdemeyer 11 years ago.
Patch for the PARI spkg .p7 to .p8 (for review)
9959_gprc.patch (1.7 KB) - added by jdemeyer 11 years ago.
Use $SAGE_ROOT/local/etc/gprc.expect as .gprc file
gprc.expect (1.7 KB) - added by jdemeyer 11 years ago.
.gprc file for the Gp() interface
pari.p9.patch (4.5 KB) - added by jdemeyer 11 years ago.
Patch for the PARI spkg .p7 to .p9 (for review)
9959_combined.patch (11.9 KB) - added by jdemeyer 11 years ago.
All 4 sagelib patches combined

Download all attachments as: .zip

Change History (47)

comment:1 follow-up: Changed 11 years ago by leif

Oh, I thought you just meant "GP/PARI" should read "PARI/GP"...

comment:2 Changed 11 years ago by leif

So how do we manage this?

Rewrite the class sage.interfaces.gp.Gp to lazily start the interpreter, or initialize sage.interfaces.gp.gp to None and explicitly do gp = Gp() everywhere it is used unless gp != None?

comment:3 follow-up: Changed 11 years ago by jdemeyer

I believe this in Gp.__init__ is the culprit:

# gp starts up with this set to 1, which makes pexpect hang: 
self.set_default('breakloop',0)

If this is the case, an easy solution is to add a patch to the pari spkg to set breakloop to 0 by default.

comment:4 in reply to: ↑ 1 Changed 11 years ago by jdemeyer

Replying to leif:

Oh, I thought you just meant "GP/PARI" should read "PARI/GP"...

This should also be changed.

comment:5 in reply to: ↑ 3 Changed 11 years ago by leif

Replying to jdemeyer:

I believe this in Gp.__init__ is the culprit:

# gp starts up with this set to 1, which makes pexpect hang: 
self.set_default('breakloop',0)

Good catch. I read this a dozen times without noticing that this actually calls gp... 8/

If this is the case, an easy solution is to add a patch to the pari spkg to set breakloop to 0 by default.

Try it...

Changed 11 years ago by jdemeyer

Renames "GP/PARI" to "PARI/GP"

comment:7 Changed 11 years ago by jdemeyer

  • Authors set to Jeroen Demeyer
  • Description modified (diff)
  • Priority changed from major to minor
  • Status changed from new to needs_review
  • Summary changed from Get Pari to stop starting automatically to Get PARI/GP to stop starting automatically

comment:8 Changed 11 years ago by leif

Wow, a lot of places where "GP/PARI" had to be changed... (well done)

We have two similar instances in the scripts repo:

local/bin/sage-sage:    echo "  -gp <..>      -- run Sage's GP (Pari) with given arguments"
local/bin/sage-sage.py:                     help="run Sage's GP (Pari), passing it all remaining arguments")

But I think we don't need to change these on this ticket.

Patches look fine, will test them later (with the new spkg of course).

comment:9 Changed 11 years ago by leif

The spkg's changelog entry (and patches/README.txt) could describe the reason for patching default.c a bit more. (The ticket number is only in the commit message.)

comment:10 Changed 11 years ago by leif

s|and|and/or|

comment:11 follow-ups: Changed 11 years ago by cremona

OK, so I was the one who put in those two breakloop calls, which I only did since otherwise things did not work properly -- but in the testing I just did I found it only necessary to delete the brealoop call in __init__, while keeping the one in _start. Are you both sure that it is safe to delete the one in _start as well? Does that not cause gp to enter the breakloop on encountering an error, which is not what you want?

If you are right (and I assumed that you did test this!) then I am wondering what else has changed, since when we started the pari upgrade it was definitely necessary to deal with this.

And by the way, the *only* reason I created a _start function for Gp different from the default one in PExpect was to add this line in _start; so if that line is not needed, you can probably delete the while _start function for class Gp.

comment:12 follow-up: Changed 11 years ago by leif

I haven't come to testing yet... (including a closer look at the relevant PARI sources).

It should at least not hurt to keep the second set_default('breakloop',0) in Gp._start().

comment:13 in reply to: ↑ 12 Changed 11 years ago by leif

Replying to leif:

It should at least not hurt to keep the second set_default('breakloop',0) in Gp._start().

As far as I can see, we don't need it there, since gp will of course (re)start with the hard-coded defaults whenever _start() is called.

(This is different to setting it "manually" from the Expect interface each time gp is [re]started.)

So we can IMHO drop Gp._start() completely, as John noted.

comment:14 in reply to: ↑ 11 Changed 11 years ago by jdemeyer

Replying to cremona:

Are you both sure that it is safe to delete the one in _start as well? Does that not cause gp to enter the breakloop on encountering an error, which is not what you want?

The new spkg patches the gp source code to disable the breakloop by default, so we can remove all references to breakloop from Sage. Note also that the patch 9959_gp_error_doctest.patch actually acts a doctest to check this.

Changed 11 years ago by jdemeyer

Don't set the 'breakloop' default in Sage

Changed 11 years ago by jdemeyer

Add doctest for error recovery

comment:15 in reply to: ↑ 11 Changed 11 years ago by jdemeyer

Replying to cremona:

And by the way, the *only* reason I created a _start function for Gp different from the default one in PExpect was to add this line in _start; so if that line is not needed, you can probably delete the while _start function for class Gp.

Done

comment:16 follow-up: Changed 11 years ago by cremona

It's a pity that we had to patch the gp source code -- clearly that was an option right from the start, and what I did was to avoid that. Best (in my opinion) would be if gp had a command-line option to turn off the breakloop default, since Sage could easily use that. It might be worth suggesting that possibility upstream.

Changed 11 years ago by jdemeyer

Patch for the PARI spkg .p7 to .p8 (for review)

comment:17 in reply to: ↑ 16 ; follow-up: Changed 11 years ago by jdemeyer

Replying to cremona:

It's a pity that we had to patch the gp source code -- clearly that was an option right from the start, and what I did was to avoid that. Best (in my opinion) would be if gp had a command-line option to turn off the breakloop default, since Sage could easily use that. It might be worth suggesting that possibility upstream.

If one would add an option to gp, add an option for non-interactive (script) use which would also disable breakloop by default. I remember I proposed this (a long time ago) to the PARI/GP developers without much success.

comment:18 in reply to: ↑ 17 ; follow-up: Changed 11 years ago by cremona

Replying to jdemeyer:

Replying to cremona:

It's a pity that we had to patch the gp source code -- clearly that was an option right from the start, and what I did was to avoid that. Best (in my opinion) would be if gp had a command-line option to turn off the breakloop default, since Sage could easily use that. It might be worth suggesting that possibility upstream.

If one would add an option to gp, add an option for non-interactive (script) use which would also disable breakloop by default. I remember I proposed this (a long time ago) to the PARI/GP developers without much success.

I suppose that from their point of view it makes no sense to use gp non-interactively, since gp is the interactive interface to the pari library!

comment:19 follow-up: Changed 11 years ago by jdemeyer

Alternative solution:

When gp starts, it reads a configuration file (by default, $HOME/.gprc). If the environment variable $GPRC is set, it uses that as a filename for the .gprc file. We could create a file $HOME/.sage/gp/gprc similarly to $HOME/.sage/ipyhton/ipythonrc and have Sage set $GPRC to that location.

Then, the file $HOME/.sage/gp/gprc should contain a line:

breakloop =  0

comment:20 in reply to: ↑ 18 Changed 11 years ago by jdemeyer

Replying to cremona:

I suppose that from their point of view it makes no sense to use gp non-interactively, since gp is the interactive interface to the pari library!

Good point :-)

comment:21 in reply to: ↑ 19 ; follow-up: Changed 11 years ago by leif

Replying to jdemeyer:

Alternative solution:

When gp starts, it reads a configuration file (by default, $HOME/.gprc). If the environment variable $GPRC is set, it uses that as a filename for the .gprc file. We could create a file $HOME/.sage/gp/gprc similarly to $HOME/.sage/ipyhton/ipythonrc and have Sage set $GPRC to that location.

Then, the file $HOME/.sage/gp/gprc should contain a line:

breakloop =  0

I was just going to suggest that, too.

Note that we should have two of them, one for the interactive gp provided by Sage, and one for the pexpect interface.

comment:22 in reply to: ↑ 21 ; follow-up: Changed 11 years ago by jdemeyer

  • Status changed from needs_review to needs_work

Replying to leif:

Note that we should have two of them, one for the interactive gp provided by Sage, and one for the pexpect interface.

I think we need only one, for the pexpect interface. I would prefer sage -gp to use my $HOME/.gprc.

Setting this to needs_work to implement the alternative solution.

comment:23 Changed 11 years ago by leif

Replying to jdemeyer:

If one would add an option to gp, add an option for non-interactive (script) use which would also disable breakloop by default. I remember I proposed this (a long time ago) to the PARI/GP developers without much success.

Perhaps they meanwhile chnaged their minds.. ;-)

There's some stuff in it for using gp from Emacs or TeXmacs?, but only as a compile time option IIRC. Also some kind of "non-interactive" gp use.

comment:24 in reply to: ↑ 22 Changed 11 years ago by leif

Replying to jdemeyer:

Replying to leif:

Note that we should have two of them, one for the interactive gp provided by Sage, and one for the pexpect interface.

I think we need only one, for the pexpect interface. I would prefer sage -gp to use my $HOME/.gprc.

Right, but we should take care not to use the other one (i.e. by setting GPRC and therefore overriding $HOME/.gprc) if we start the interactive gp.

comment:25 Changed 11 years ago by jdemeyer

Anybody happens to know where is the code to deal with $DOT_SAGE, to populate it with files?

Changed 11 years ago by jdemeyer

Use $SAGE_ROOT/local/etc/gprc.expect as .gprc file

comment:26 Changed 11 years ago by jdemeyer

  • Description modified (diff)

New spkg: http://sage.math.washington.edu/home/jdemeyer/spkg/pari-2.4.3.svn-12577.p9.spkg. This is based on p7 (not p8) and adds the gprc.expect file. So installing the new spkg and applying all 4 patches should do it.

comment:27 Changed 11 years ago by leif

  • Status changed from needs_work to needs_review

comment:28 Changed 11 years ago by jdemeyer

  • Status changed from needs_review to needs_work

Doesn't work yet, stupid mistake.

comment:29 Changed 11 years ago by jdemeyer

sage.math.washington.edu seems much slower than usual today...

comment:30 follow-up: Changed 11 years ago by leif

Does gp expand $SAGE_ROOT in the gprc.expect file, or does Sage do that before it starts gp?

comment:31 in reply to: ↑ 30 Changed 11 years ago by jdemeyer

Replying to leif:

Does gp expand $SAGE_ROOT in the gprc.expect file, or does Sage do that before it starts gp?

gp does that.

Changed 11 years ago by jdemeyer

.gprc file for the Gp() interface

Changed 11 years ago by jdemeyer

Patch for the PARI spkg .p7 to .p9 (for review)

comment:32 Changed 11 years ago by jdemeyer

  • Status changed from needs_work to needs_review

Changed 11 years ago by jdemeyer

All 4 sagelib patches combined

comment:33 Changed 11 years ago by jdemeyer

  • Cc leif added
  • Description modified (diff)

comment:34 Changed 11 years ago by mpatel

  • Priority changed from minor to blocker

comment:35 follow-up: Changed 11 years ago by cremona

The sagelib patch looks good to me.

For review, please could you give instructions for the simple-minded as to how to apply the patch which converts .p7 to .p9? And then how to rebuild? Thanks.

Of course, someone else who knows how to do this reliably is welcome to get in first with a review.

comment:36 in reply to: ↑ 35 Changed 11 years ago by jdemeyer

  • Reviewers set to John Cremona

Replying to cremona:

The sagelib patch looks good to me.

For review, please could you give instructions for the simple-minded as to how to apply the patch which converts .p7 to .p9?

Well, the easiest is to install the new spkg:

sage -i http://sage.math.washington.edu/home/jdemeyer/spkg/pari-2.4.3.svn-12577.p9.spkg

And then how to rebuild?

I personally do not know the most reliable way to rebuild a complete Sage installation. One thing which works for sure is the following:

  • download sage-4.6.alpha3.tar and extract it so we have a clean, uncompiled sage-4.6.alpha3.
  • rm spkg/standard/pari-*.spkg
  • download [ttp://sage.math.washington.edu/home/jdemeyer/spkg/pari-2.4.3.svn-12577.p9.spkg] and and put it in spkg/standard
  • now make Sage as usual

comment:37 Changed 11 years ago by cremona

THanks -- I am doing that, will do a complete new build (with the sagelib patch too of course) and report back.

comment:38 Changed 11 years ago by cremona

  • Status changed from needs_review to positive_review

I made a freshly unpacked 4.6.alpha3, and replaced its pari spkg with the p9 version; then built all Sage; then applied the sagelib patch on this ticket; then tested the whole library.

All passed, so here's a positive review.

comment:39 Changed 11 years ago by mpatel

  • Merged in set to sage-4.6.rc0
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.