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 ECL to 12.12.1 and let it build on (recent) Cygwins #13324

Closed
jpflori opened this issue Aug 1, 2012 · 208 comments
Closed

Upgrade ECL to 12.12.1 and let it build on (recent) Cygwins #13324

jpflori opened this issue Aug 1, 2012 · 208 comments

Comments

@jpflori
Copy link

jpflori commented Aug 1, 2012

On my 1.7.16 installation of Cygwin, the ecl spkg does not build because it defines unconditionaly _WINSOCKAPI_ in h/ecl-cmp.h which then prevents the definition of fd_set in /usr/include/sys/types.h.
I guess this is harmless on Linux, intended on Windows, but on Cygwin this breaks the build of ECL.

The proposed spkg patches h/ecl-cmp.h so that _WINSOCKAPI_ is not defined on CYGWIN.
This has been merged upstream and is in 12.12.1:
https://sourceforge.net/p/ecls/ecl/ci/07c4ab7db83eda2eed7fa4a5ddeec2e28c7eb58b/

Moreover comes patches from upstream to get correct signal handling and is in 12.12.1:

We build ECL single threaded to get hopefully more correct signal handling in library mode and avoid problems with the pexpect interface.

And a later patch to avoid infinite loops while compiling lisp to C:
http://sourceforge.net/p/ecls/ecl/ci/13459a98f0c0c58ccc4e9241c3bf0625e39f2383/

Older patches which are now upstream have been removed.

Be sure to use Sage 5.6.beta2 or higher, so that the Maxima spkg is ready for this.

Try spkg at
http://boxen.math.washington.edu/home/jpflori/ecl-12.12.1.p0.spkg

Apply:

Depends on #13860

Upstream: Fixed upstream, but not in a stable release.

CC: @kiwifb

Component: porting: Cygwin

Keywords: cygwin ecl spkg

Author: Jean-Pierre Flori

Reviewer: François Bissey, Karl-Dieter Crisman, Dmitrii Pasechnik

Merged: sage-5.7.beta0

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

@jpflori jpflori added this to the sage-5.3 milestone Aug 1, 2012
@nexttime
Copy link
Mannequin

nexttime mannequin commented Aug 1, 2012

Changed keywords from cygwin ecl to cygwin ecl spkg

@jpflori
Copy link
Author

jpflori commented Aug 2, 2012

Changed upstream from Not yet reported upstream; Will do shortly. to Workaround found; Bug reported upstream.

@jpflori
Copy link
Author

jpflori commented Aug 2, 2012

comment:4

Wrong ticket...

@dimpase
Copy link
Member

dimpase commented Aug 3, 2012

comment:5

Good, this also works on my Cygwin (and this change of spkg is needed, too).

@dimpase
Copy link
Member

dimpase commented Aug 4, 2012

comment:6

however, after the whole Sage build on my Cygwin is almost done, I get

...
building 'sage.ext.interpreters.wrapper_py' extension
building 'sage.ext.interpreters.wrapper_el' extension
Executing 241 commands (using 1 thread)
gcc -I/usr/include/ncurses -fno-strict-aliasing -fwrapv -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes -I/home/Dima/sage-5.2.rc1/local/include/ecl/ -I/home/Dima/sage-5.2.rc1/local/include -I/home/Dima/sage-5.2.rc1/local/include/csage -I/home/Dima/sage-5.2.rc1/devel/sage/sage/ext -I/home/Dima/sage-5.2.rc1/local/include/python2.7 -c sage/libs/ecl.c -o build/temp.cygwin-1.7.16-i686-2.7/sage/libs/ecl.o -w
gcc -shared -Wl,--enable-auto-image-base -L/home/Dima/sage-5.2.rc1/local/lib build/temp.cygwin-1.7.16-i686-2.7/sage/libs/ecl.o -L/home/Dima/sage-5.2.rc1/local/lib -L/home/Dima/sage-5.2.rc1/local/lib/python2.7/config -lcsage -lecl -lstdc++ -lntl -lpython2.7 -o build/lib.cygwin-1.7.16-i686-2.7/sage/libs/ecl.dll
build/temp.cygwin-1.7.16-i686-2.7/sage/libs/ecl.o: In function `ecl_bignum_from_mpz':
/home/Dima/sage-5.2.rc1/devel/sage/sage/libs/eclsig.c:52: undefined reference to `__imp____gmpz_set'
/home/Dima/sage-5.2.rc1/devel/sage/sage/libs/eclsig.c:52: undefined reference to `__imp____gmpz_set'
collect2: ld returned 1 exit status
gcc -I/usr/include/ncurses -fno-strict-aliasing -fwrapv -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes -I/home/Dima/sage-5.2.rc1/local/include/FLINT/ -I/home/Dima/sage-5.2.rc1/local/include -I/home/Dima/sage-5.2.rc1/local/include/csage -I/home/Dima/sage-5.2.rc1/devel/sage/sage/ext -I/home/Dima/sage-5.2.rc1/local/include/python2.7 -c sage/libs/flint/flint.c -o build/temp.cygwin-1.7.16-i686-2.7/sage/libs/flint/flint.o -std=c99 -D_XPG6 -w
error: command 'gcc' failed with exit status 1
Error installing modified sage library code.

A missing library somewhere?

@jpflori
Copy link
Author

jpflori commented Aug 4, 2012

comment:7

That was expected.
In fact I guess the build is not nearing the end at all.

You can rerun "./sage -b" and you'll see that a lot of cpp files produced from pyx files still have to be compiled.
The first that should refail then is ecl.cpp.
See #13334 for that one.

For the following ones I've not opened tockets yet, but there should be instructions enough on the CygwinPort page (although you'll have to implement the ideas evoked there yourself, no code provided).

@jpflori
Copy link
Author

jpflori commented Aug 7, 2012

Changed upstream from Workaround found; Bug reported upstream. to Fixed upstream, but not in a stable release.

@jpflori
Copy link
Author

jpflori commented Aug 7, 2012

comment:9

The fix has been integrated upstream
https://sourceforge.net/p/ecls/ecl/ci/07c4ab7db83eda2eed7fa4a5ddeec2e28c7eb58b/

@jpflori

This comment has been minimized.

@jpflori
Copy link
Author

jpflori commented Aug 7, 2012

comment:10

Wrong ticket...

@jpflori
Copy link
Author

jpflori commented Aug 20, 2012

comment:11

I've upped an updated ECL spkg which includes upstream version 12.7.1 plus the git commit corresponding to the fix originally proposed here.
It's at
http://perso.telecom-paristech.fr/~flori/sage/ecl-12.7.1.p0.spkg

I've updated the spkg-make file and homogenized the indentation in the spkg-install file as well.

Two trivial doctests had to be changed, and it passes make ptestlong on Ubuntu 12.04 64 bits.
No time to retest Cygwin yet.

@jpflori

This comment has been minimized.

@jpflori jpflori changed the title Ecl does not build on (recent) Cygwins Upgrade ECL to 12.7.1 and let it build on (recent) Cygwins Aug 20, 2012
@dimpase
Copy link
Member

dimpase commented Aug 20, 2012

comment:12

Replying to @jpflori:

I've upped an updated ECL spkg which includes upstream version 12.7.1 plus the git commit corresponding to the fix originally proposed here.

I think you are cloning ECL from an old repo, no longer updated (they recently switched).
In the right repo 12.7.1 does have your commit already applied.

@jpflori
Copy link
Author

jpflori commented Aug 20, 2012

comment:13

My point was to use a released version, not a git clone, and add only the needed commit on top of that.
We could rather use a fresh git clone with the needed commit and further bug fixes, but I'm less inclined to do so, as it might be confusing.
When upstream release ECL 12.7.2 or whatever the next stable release will be, let's get rid of the patch to ecl-cmp.h in the patches/directory.

@jpflori
Copy link
Author

jpflori commented Aug 20, 2012

comment:14

For example, such an approach was used for Singular in #13237 IIRC.

@jpflori
Copy link
Author

jpflori commented Aug 20, 2012

comment:15

The spkg I uploaded has some problems I'll upload a better one in a few moments.

@jpflori
Copy link
Author

jpflori commented Aug 20, 2012

comment:16

Should be ok now.

@kiwifb
Copy link
Member

kiwifb commented Aug 20, 2012

comment:18

I see you disabled unicode outright. Is this to avoid the problems mentioned in #12985? I haven't seen the problem you fix in gsl/integration.pyx last time I tried ecl 12.7.1 so it may be slightly platform dependent. As a general rule I don't think adding decimal places to a result is the right way to fix numerical noise in a doctest.
The other patch to ecl.pyx is absolutely necessary.
The spkg looks ok otherwise.

@jpflori
Copy link
Author

jpflori commented Aug 20, 2012

comment:19

I did not change the unicode behavior, Im aware there's a ticket open about that, but I definitely did not modify the spkg-install for that purpose, it was already as such (something about Maxima IIRC).

I agree about the numerical noise, I planned to put ... there but it ended up as 44.

@kiwifb
Copy link
Member

kiwifb commented Aug 20, 2012

comment:20

Looking at the diff you provided it was indeed already there. I may even have put it there one of the last time I touched the spkg on knowledge from sage-on-gentoo that it was causing problems.
Haven't tested the whole thing yet you didn't see any problems with sage/calculus/calculus.py?
That would mean it was entirely caused by enabling unicode when I saw it in #12985.

@jpflori
Copy link
Author

jpflori commented Aug 20, 2012

comment:21

It passed make ptestlong on 64 bits Ubuntu 12.04.
(Now it's stuck on Cygwin rebuilding Maxima, but that may be Cygwin voodoo...).

@jpflori
Copy link
Author

jpflori commented Aug 21, 2012

comment:22

I've committed the changes in the spkg and puts dots to take care of the numerical noise in the Sage library patch.

@kiwifb
Copy link
Member

kiwifb commented Aug 21, 2012

comment:23

I am happy to give this a positive review but I am putting this on sage-5.4 as 5.3 is in the rc stage it won't make the cut unless Jeroen changes his mind about the release schedule.

@kiwifb
Copy link
Member

kiwifb commented Jan 11, 2013

comment:162

I'll admit to a fair amount of testing as part of sage-on-gentoo but I don't do that kind of torture test. And I wouldn't even pretend to reach the number of tests you run across the sagemath cluster... For the record I also see it on my sage-on-gentoo install on x86_64.

leif: As far as I can see neither (time out in progress)

USER       PID %CPU %MEM    VSZ   RSS TTY      STAT START   TIME COMMAND
fbissey   5088  0.0  0.0  11672  1464 pts/0    S    22:34   0:00 bash thousand.sh
fbissey   5096  0.0  0.0  51120  6248 pts/0    S    22:34   0:00 /usr//bin//python2.7 /usr//bin/sage-cleaner
fbissey   9699  0.0  0.0  14096  4388 pts/2    S+   22:49   0:00 nano -w /scratch/portage/dev-python/sympy-0.7.2/work/sympy-0.7.2/sympy/utilities/runtests.py
fbissey  11689  0.0  0.0  11800  1572 pts/0    S    22:53   0:00 bash /usr/bin/sage -t -verbose /usr/share/sage/devel/sage-main/sage/interfaces/lisp.py
fbissey  11690  0.0  0.0  34372  4972 pts/0    S    22:53   0:00 /usr/bin/python2.7 /usr/bin/sage -test -verbose /usr/share/sage/devel/sage-main/sage/interfaces/lisp.py
fbissey  11692  0.0  0.0  57624  6668 pts/0    S    22:53   0:00 /usr/bin/python2.7 /usr/bin/sage -doctest -verbose /usr/share/sage/devel/sage-main/sage/interfaces/lisp.py
fbissey  11693  0.4  0.6 969368 151608 pts/0   S    22:53   0:00 /usr/bin/python2.7 /home/fbissey/.sage/tmp/lisp_11692.py
fbissey  11694  0.0  0.0  32460  7396 pts/3    Ss+  22:53   0:00 /usr//bin/ecl
fbissey  11705  0.0  0.0  15144  1128 pts/0    R+   22:57   0:00 ps ux
fbissey  11706  0.0  0.0   6644   800 pts/0    S+   22:57   0:00 more
fbissey  25062  0.0  0.0  41056  1656 ?        S    21:42   0:02 sshd: fbissey@pts/0
fbissey  25063  0.0  0.0  20488  2424 pts/0    Ss   21:42   0:00 -bash
fbissey  29298  0.0  0.0  41056  1656 ?        S    21:53   0:00 sshd: fbissey@pts/2
fbissey  29300  0.0  0.0  20488  2436 pts/2    Ss   21:53   0:00 -bash

@nexttime
Copy link
Mannequin

nexttime mannequin commented Jan 11, 2013

comment:163

Btw, you can also do

env SAGE_TEST_ITER=1000 SAGE_TIMEOUT=60 ./sage -tp 1 devel/sage/sage/interfaces/lisp.py &> foo.log

(Add --verbose, change the number of threads, or the timeout as you like.)

Afterwards grep the log...

@nexttime
Copy link
Mannequin

nexttime mannequin commented Jan 11, 2013

comment:164

Replying to @nexttime:

Btw, you can also do

env SAGE_TEST_ITER=1000 SAGE_TIMEOUT=60 ./sage -tp 1 devel/sage/sage/interfaces/lisp.py &> foo.log

I tried this with the old ECL (on Linux x86_64) and didn't get a single failure / timeout, so the timeouts are definitely a regression. (Although I must admit I haven't tried with the new ECL [on the same machine].)

@jpflori
Copy link
Author

jpflori commented Jan 11, 2013

comment:165

Ok, I think I got a little more info on this one by enabling the pexpect logs in our Sage classes:

  • we throw (+ 1 random) to ECL for synchronization,
  • it seems the output is somehow split on multiple lines randomly, e.g. on the last failing example I got we threw (+ 1 937780183) and here is what is in the log
...
> (+ 1 937780183)

(+ 1 937780183)^M
^M
93^M
7780184^M
> 

So indeed we will never read 937780184 back.
(Not sure what ^M exactly is)

By the way, the lisp.py file needs a big cleanup, especially all the :: for the examples and tests.

@jpflori
Copy link
Author

jpflori commented Jan 11, 2013

comment:166

All our troubles may be caused by the fact that the sync command we send is

"(+ 1 s)\n"

Note the "\n" which is superfulous and might get intersparsed with the output of "(+ 1 s)" if ECL does not handle it gracefully so leading to our problem.

@jpflori
Copy link
Author

jpflori commented Jan 11, 2013

comment:167

Could you test lisp.py after removing that extra \n in _synchronize in lisp.py?
That may not be a real fix, but at least a workaround.

@jdemeyer
Copy link

comment:168

^M is a carriage return, it's normal that this appears at the end of every line.

@nexttime
Copy link
Mannequin

nexttime mannequin commented Jan 11, 2013

comment:169

Replying to @jdemeyer:

^M is a carriage return, it's normal that this appears at the end of every line.

Yes, a carriage return (rather than a newline, \n), which is quite DOSsy (\r\n).

@jpflori
Copy link
Author

jpflori commented Jan 11, 2013

Cleanup lisp.py and remove problematic \n from synchronize

@jpflori
Copy link
Author

jpflori commented Jan 11, 2013

comment:170

Attachment: trac_13324.2.patch.gz

@jpflori

This comment has been minimized.

@jpflori
Copy link
Author

jpflori commented Jan 11, 2013

comment:171

I went through one thousand tests without problem with the last patch.

@kiwifb
Copy link
Member

kiwifb commented Jan 12, 2013

comment:172

It's gone here too. I think that did the trick. In fact I am wondering if that has got rid of the problems I was seeing in #12985 but that's a discusion for that other ticket. I am putting this back to positive review.

@jdemeyer
Copy link

Merged: sage-5.7.beta0

@jdemeyer
Copy link

jdemeyer commented Feb 5, 2013

comment:174

This is a massive regression due to this ECL upgrade:

Execute

time print(len(maxima._commands(verbose=False)))
  • Before: about 2.5s
  • After: about 20s

It makes we wonder whether we should unmerge this upgrade, what if more ECL-related functionality has slowed down?

@dimpase
Copy link
Member

dimpase commented Feb 5, 2013

comment:175

Replying to @jdemeyer:

This is a massive regression due to this ECL upgrade:

Execute

time print(len(maxima._commands(verbose=False)))
  • Before: about 2.5s
  • After: about 20s

It makes we wonder whether we should unmerge this upgrade, what if more ECL-related functionality has slowed down?

what time do you measure?

sage: time print(len(maxima._commands(verbose=False)))
1926
Time: CPU 0.91 s, Wall: 28.65 s

and do you get 1926 with the older ECL, too?

@jdemeyer
Copy link

jdemeyer commented Feb 5, 2013

comment:176

Replying to @dimpase:

and do you get 1926 with the older ECL, too?

No, I get 1871 which is close enough.

@kcrisman
Copy link
Member

kcrisman commented Feb 5, 2013

comment:177

Also,

/Users/.../sage-5.7.beta2/local/lib/python2.7/site-packages/sage/misc/sage_extension.py:359: DeprecationWarning: Use %time instead of time.
See http://trac.sagemath.org/12719 for details.

Anyway, it's only the very first time that this is really slow, presumably due to Maxima starting up. However, there is a brief slowdown.

sage: time print(len(maxima._commands(verbose=False)))
1926
CPU times: user 0.32 s, sys: 0.07 s, total: 0.39 s
Wall time: 16.06 s
sage: time print(len(maxima._commands(verbose=False)))
1926
CPU times: user 0.00 s, sys: 0.00 s, total: 0.00 s
Wall time: 0.00 s
sage: %timeit print(len(maxima._commands(verbose=False)))
<many 1926 later>
100000 loops, best of 3: 5.09 us per loop

With 5.6:

sage: time print(len(maxima._commands(verbose=False)))
1871
Time: CPU 0.37 s, Wall: 16.32 s
sage: time print(len(maxima._commands(verbose=False)))
1871
Time: CPU 0.00 s, Wall: 0.00 s
sage: %timeit print(len(maxima._commands(verbose=False)))
625 loops, best of 3: 2.16 µs per loop

Testing another command. Old:

sage: %timeit integrate(cos(x^2),x)
25 loops, best of 3: 21.4 ms per loop

New:

sage: %timeit integrate(cos(x^2),x)
1 loops, best of 3: 21.2 ms per loop

I think this is a non-issue. How often are people going to need really fast listing of all Maxima commands?

@jdemeyer
Copy link

jdemeyer commented Feb 5, 2013

comment:178

Replying to @kcrisman:

How often are people going to need really fast listing of all Maxima commands?

I was going to say "TAB completion", but that's apparently cached in $DOT_SAGE.

@jdemeyer
Copy link

jdemeyer commented Feb 5, 2013

comment:179

Replying to @kcrisman:

presumably due to Maxima starting up.

That can't be the explanation, as other maxima commands don't take 20 seconds...

@kcrisman
Copy link
Member

kcrisman commented Feb 5, 2013

comment:180

presumably due to Maxima starting up.

That can't be the explanation, as other maxima commands don't take 20 seconds...

Hmm, true. On startup, it only seems to add an extra 1.5 seconds for Maxima to start, so the list seems to take 15+ seconds.

Okay, it seems to be creating the whole list for tab completion, actually. I'm used to seeing this slowness, but on my 10-year-old machine with a 1/2 G of memory.

My guess? This line from maxima.completions??:

cmd_list = self._eval_line('apropos("%s")'%s, error_check=False).replace('\\ - ','-')

Recall that we also upgraded Maxima in 5.7.beta0, #13364. Perhaps "apropos" slowed down.

Or maybe the IPython upgrade in #12719 made

sys.stdout.flush()

slower, if this only shows up in 5.7.beta1. I'm going to check this momentarily.

@kcrisman
Copy link
Member

kcrisman commented Feb 5, 2013

comment:181

Or maybe the IPython upgrade in #12719 made

sys.stdout.flush()

slower, if this only shows up in 5.7.beta1. I'm going to check this momentarily.

Okay, it also is in beta0, and in retrospect that was a silly comment by me, because that is only called with verbose=True.

So I wonder about "apropos".

How often are people going to need really fast listing of all Maxima commands?

I was going to say "TAB completion", but that's apparently cached in $DOT_SAGE.

And

        try:
            return self.__commands
        except AttributeError:

So it's cached there as well.

Well, I would say open a ticket, and let's see if we can find a way to find a Maxima command to mimic this slowdown in this listing command to report upstream.

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

6 participants