Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade and optimize pexpect #10295

Closed
simon-king-jena opened this issue Nov 20, 2010 · 218 comments
Closed

Upgrade and optimize pexpect #10295

simon-king-jena opened this issue Nov 20, 2010 · 218 comments

Comments

@simon-king-jena
Copy link
Member

We use pexpect version 2.0. Shouldn't we upgrade to the current version 4.0.1?

See also #10294.

New upstream tarballs:

Patches included and submitted upstream:

Note that this branch includes #19616.


Throughput timings (best out of 5):

pexpect 2.0 upstream:

sage: %time _ = str(gp("2^2^22"))
CPU times: user 184 ms, sys: 4 ms, total: 188 ms
Wall time: 384 ms

pexpect 4.0.1 upstream:

sage: %time _ = str(gp("2^2^22"))
CPU times: user 208 ms, sys: 4 ms, total: 212 ms
Wall time: 405 ms

pexpect 4.0.1 + Sage patches:

sage: %time _ = str(gp("2^2^22"))
CPU times: user 9 ms, sys: 7 ms, total: 16 ms
Wall time: 209 ms

Latency timings (best out of 5):

pexpect 2.0 upstream:

sage: gp(1);
sage: %time _ = [gp(i) for i in range(10^4)]
CPU times: user 2.96 s, sys: 855 ms, total: 3.82 s
Wall time: 4.52 s

pexpect 4.0.1 upstream:

sage: gp(1);
sage: %time _ = [gp(i) for i in range(10^4)]
CPU times: user 2.61 s, sys: 758 ms, total: 3.37 s
Wall time: 3.48 s

pexpect 4.0.1 + Sage patches:

sage: gp(1);
sage: %time _ = [gp(i) for i in range(10^4)]
CPU times: user 2.3 s, sys: 999 ms, total: 3.3 s
Wall time: 3.4 s

Depends on #19671

Upstream: Reported upstream. Developers acknowledge bug.

CC: @sagetrac-drkirkby @nexttime @sagetrac-Vincent-Neri

Component: interfaces

Keywords: pexpect upgrade

Author: François Bissey, Bill Page, Jeroen Demeyer

Branch/Commit: 9351ccb

Reviewer: Jeroen Demeyer, Bill Page

Issue created by migration from https://trac.sagemath.org/ticket/10295

@kiwifb
Copy link
Member

kiwifb commented Nov 20, 2010

comment:1

Try version 2.4 even. This has been discussed to death several times on sage-devel.
Version 2.1 and over are slower than what we currently have. Plus from experience
in sage-on-gentoo where at some stage we experimented with pexpect 2.4, some plotting broke down in the notebook.

@kiwifb
Copy link
Member

kiwifb commented Nov 20, 2010

comment:2

When I say version 2.4 you have to go there: http://pypi.python.org/pypi/pexpect
rather than here: http://pypi.python.org/pypi/pexpect

@simon-king-jena
Copy link
Member Author

comment:3

Replying to @kiwifb:

When I say version 2.4 you have to go there: http://pypi.python.org/pypi/pexpect
rather than here: http://pypi.python.org/pypi/pexpect

Aren't the two links the same?

So, you say that there might be a performance problem with more recent versions of pexpect. That's bad, because the idea was to get rid of a performance problem by upgrading...

@kiwifb
Copy link
Member

kiwifb commented Nov 20, 2010

comment:4

cut and paste didn't work as expected the second one was meant to be:
http://pexpect.sourceforge.net/

search the mailing list for pexpect, it is a proverbial can of worms.

@sagetrac-drkirkby
Copy link
Mannequin

sagetrac-drkirkby mannequin commented Apr 8, 2011

comment:5

2.4 is not released yet. Here's a package for 2.3 that I created.

http://boxen.math.washington.edu/home/kirkby/patches/pexpect-2.3.spkg

but on my OpenSolaris machine, this 2.3 package results in one doctest failure, which is:

sage -t  -long -force_lib devel/sage/sage/interfaces/expect.py

The interface must be semi-working, as the interface to R works - or at lease the R doctest does not fail. Here's the error message of the failed doctest.

sage -t  -long -force_lib devel/sage/sage/interfaces/sage0.py
         [14.8 s]
sage -t  -long -force_lib devel/sage/sage/interfaces/expect.py
Exception pexpect.ExceptionPexpect: ExceptionPexpect() in <bound method spawn.__del__ of <pexpect.spawn object at 0xc5988ac>> ignored

<snip out load more similar errors>

<pexpect.spawn object at 0xc386d6c>> ignored
Exception pexpect.ExceptionPexpect: ExceptionPexpect() in <bound method spawn.__del__ of <pexpect.spawn object at 0xc386d6c>> ignored
**********************************************************************
File "/export/home/drkirkby/sage-4.7.alpha3/devel/sage-main/sage/interfaces/expect.py", line 867:
    sage: r._expect.before
Expected:
    'abc;\r\n[1] '
Got:
    'abc <- 10 +15;\r\n__SAGE__R__PROMPT__> abc;\r\n[1] '
**********************************************************************
1 items had failures:
   1 of  11 in __main__.example_16
***Test Failed*** 1 failures.
For whitespace errors, see the file /export/home/drkirkby/.sage//tmp/.doctest_expect.py
         [18.4 s]
sage -t  -long -force_lib devel/sage/sage/interfaces/gap.py
         [20.4 s]

I think in some cases where we call external programs there are probably better ways of doing this. For example

  • R has libraries, I expect we can call R via the libraries, rather than on the command line.
  • Mathematica uses the Mathlink protocol to communicate between the kernel and front end. How to use Mathlink is documented, and has been used by at least open-source program (JMath) to work with Mathematica. But Sage calls Mathematica via the command line.

Dave

@sagetrac-drkirkby
Copy link
Mannequin

sagetrac-drkirkby mannequin commented Apr 8, 2011

comment:6

Oops, I see this doctest is related to R. But I'm sure there are other doctests which make use of pexpect, which are passing

Dave

@kiwifb
Copy link
Member

kiwifb commented Apr 8, 2011

comment:7

While 2.4 is not on sourceforge but on pypi.python.org http://pypi.python.org/pypi/pexpect I think we can call it released unless you have other infos (from author/mailing list).
As far as I remember, last time we tried pexpect-2.4 on sage-on-gentoo it broke plotting in the notebook. Can you try to do some plotting in the notebook with this version of pexpect? Digged my email archive, more precisely:

g=sin(x); plot(g,(x,-pi,3*pi/2))

in the notebook. It produced the following for us (at the time, notice the python time stamp):

import os;os.chdir("/tmp/tmpo2KomY");
execfile("_sage_input_1.py")Python 2.6.4 (r264:75706, Mar  4 2010,
21:15:13)
[GCC 4.3.4] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> 
>>> import os;os.chdir("/tmp/tmpo2KomY");
>>> execfile("_sage_input_1.py")
START1

import os;os.chdir("/tmp/tmpY61yjM");
execfile("_sage_input_2.py")

import os;os.chdir("/tmp/tmpXjmsQK");
execfile("_sage_input_3.py")
__SAGE__
__SAGE__import os;os.chdir("/tmp/tmpY61yjM");
__SAGE__execfile("_sage_input_2.py")
START2
__SAGE__
__SAGE__import os;os.chdir("/tmp/tmpXjmsQK");

@kiwifb
Copy link
Member

kiwifb commented Apr 12, 2011

comment:8

Dave, have you tried plotting in the notebook?

@jdemeyer jdemeyer modified the milestones: sage-5.11, sage-5.12 Aug 13, 2013
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.1, sage-6.2 Jan 30, 2014
@kiwifb
Copy link
Member

kiwifb commented Mar 30, 2014

comment:12

Bump this. There is a pexpect 3.1 now at http://pexpect.readthedocs.org/en/latest/ (note: the sourceforge address will redirect you there). For a long time I thought pexpect was done and there wouldn't be anymore release.

I think if the project is alive they may take request from us.

@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.2, sage-6.3 May 6, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.3, sage-6.4 Aug 10, 2014
@rwst

This comment has been minimized.

@kiwifb

This comment has been minimized.

@kiwifb
Copy link
Member

kiwifb commented May 8, 2015

Commit: 9b1e098

@kiwifb
Copy link
Member

kiwifb commented May 8, 2015

Branch: u/fbissey/pexpect3.3

@kiwifb kiwifb modified the milestones: sage-6.4, sage-6.7 May 8, 2015
@kiwifb
Copy link
Member

kiwifb commented May 8, 2015

comment:17

Hum wrong stuff in that branch sorry will update shortly.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 8, 2015

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

679e33aUpdate pexpect to 3.3

@jdemeyer jdemeyer modified the milestones: sage-6.10, sage-7.0 Dec 10, 2015
@kcrisman
Copy link
Member

comment:177

Now that this has positive review otherwise, I'll also be sure to include the relevant piece in #19616. François, if you want to make the same PR as before I can do that; I'm not sure how to revert reverting a changeset on Github, I just want to make sure your name is still attached to that change.

@kiwifb
Copy link
Member

kiwifb commented Dec 10, 2015

comment:178

Replying to @kcrisman:

Now that this has positive review otherwise, I'll also be sure to include the relevant piece in #19616. François, if you want to make the same PR as before I can do that; I'm not sure how to revert reverting a changeset on Github, I just want to make sure your name is still attached to that change.

On it.

@jdemeyer
Copy link

comment:179

I have made one additional pull request to pexpect: https://github.com/pexpect/pexpect/pull/308/files

It is really quite trivial and it provided yet another easy optimization. If some reviewer agrees, I will add that patch to the branch here.

@kiwifb
Copy link
Member

kiwifb commented Dec 10, 2015

comment:180

Replying to @kiwifb:

Replying to @kcrisman:

Now that this has positive review otherwise, I'll also be sure to include the relevant piece in #19616. François, if you want to make the same PR as before I can do that; I'm not sure how to revert reverting a changeset on Github, I just want to make sure your name is still attached to that change.

On it.

This is now sagemath/sagenb#356

@jdemeyer

This comment has been minimized.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 14, 2015

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

fd1ec53Upgrade sagenb to version 0.11.5
0bb1df2Upgrade sagenb to version 0.11.6
1255a5aMerge remote-tracking branch 'trac/u/kcrisman/ticket/19616' into t/10295/pexpect3.3

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 14, 2015

Changed commit from 1e75fbd to 1255a5a

@jdemeyer
Copy link

comment:183

Merged the latest version of #19616.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 16, 2015

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

bed31b2Upgrade sagenb to version 0.11.6.1
9351ccbMerge remote-tracking branch 'trac/u/kcrisman/ticket/19616' into t/10295/pexpect3.3

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 16, 2015

Changed commit from 1255a5a to 9351ccb

@jdemeyer

This comment has been minimized.

@jdemeyer
Copy link

Changed dependencies from #19671, #19616 to #19671

@jdemeyer
Copy link

comment:187

Follow-up at #19736.

@jdemeyer
Copy link

Changed reviewer from Jeroen Demeyer to Jeroen Demeyer, Bill Page

@vbraun
Copy link
Member

vbraun commented Dec 22, 2015

Changed branch from u/jdemeyer/pexpect3.3 to 9351ccb

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants