Opened 8 years ago
Closed 6 years ago
#12661 closed enhancement (fixed)
Include GMP 5.1.3 as an optional package
Reported by:  SimonKing  Owned by:  tbd 

Priority:  major  Milestone:  sage6.2 
Component:  packages: optional  Keywords:  gmp 
Cc:  leif, rws  Merged in:  
Authors:  Simon King, JeanPierre Flori  Reviewers:  François Bissey 
Report Upstream:  N/A  Work issues:  
Branch:  a14aaf2 (Commits)  Commit:  a14aaf2fd94252c0e263c8b02003f8c640d70e0e 
Dependencies:  Stopgaps: 
Description (last modified by )
Change History (49)
comment:1 Changed 8 years ago by
comment:2 Changed 8 years ago by
Yeah, I made some progress!!
I found that I needed to edit src/gmph.in
, and I needed to #include <cstdio>
, but only #if defined (__cplusplus)
.
Then, building the package went beyond the point where it failed previously. However, it turns out that the gmp package requires yacc. I am insalling it now.
TODO (for the moment):
 Turn my changes to
src/gmph.in
into a patch, to be put into thepatches
directory.  spkginstall should check for the presence of yacc.
comment:3 Changed 8 years ago by
Hooray, the modified package builds on my openSuse laptop. However, spkgcheck fails, ending with
Making check in cxx make[3]: Entering directory `/home/simon/SAGE/sage5.0.beta7/spkg/optional/gmp4.2.1.p11.copy/src/tests/cxx' make tassign tbinary tcast tconstr theaders tistream tlocale tmisc tops tostream tprec trand tternary tunary make[4]: Entering directory `/home/simon/SAGE/sage5.0.beta7/spkg/optional/gmp4.2.1.p11.copy/src/tests/cxx' g++ DHAVE_CONFIG_H I. I. I../.. I../.. I../../tests O2 m64 mtune=k8 c o tassign.o tassign.cc /bin/sh ../../libtool mode=link g++ O2 m64 mtune=k8 o tassign tassign.o L../../.libs ../../tests/libtests.la ../../libgmpxx.la ../../libgmp.la mkdir .libs g++ O2 m64 mtune=k8 o .libs/tassign tassign.o L/home/simon/SAGE/sage5.0.beta7/spkg/optional/gmp4.2.1.p11.copy/src/.libs ../../tests/.libs/libtests.a ../../.libs/libgmpxx.so /home/simon/SAGE/sage5.0.beta7/spkg/optional/gmp4.2.1.p11.copy/src/.libs/libgmp.so ../../.libs/libgmp.so Wl,rpath Wl,/home/simon/SAGE/sage5.0.beta7/local/lib creating tassign g++ DHAVE_CONFIG_H I. I. I../.. I../.. I../../tests O2 m64 mtune=k8 c o tbinary.o tbinary.cc /bin/sh ../../libtool mode=link g++ O2 m64 mtune=k8 o tbinary tbinary.o L../../.libs ../../tests/libtests.la ../../libgmpxx.la ../../libgmp.la g++ O2 m64 mtune=k8 o .libs/tbinary tbinary.o L/home/simon/SAGE/sage5.0.beta7/spkg/optional/gmp4.2.1.p11.copy/src/.libs ../../tests/.libs/libtests.a ../../.libs/libgmpxx.so /home/simon/SAGE/sage5.0.beta7/spkg/optional/gmp4.2.1.p11.copy/src/.libs/libgmp.so ../../.libs/libgmp.so Wl,rpath Wl,/home/simon/SAGE/sage5.0.beta7/local/lib creating tbinary g++ DHAVE_CONFIG_H I. I. I../.. I../.. I../../tests O2 m64 mtune=k8 c o tcast.o tcast.cc /bin/sh ../../libtool mode=link g++ O2 m64 mtune=k8 o tcast tcast.o L../../.libs ../../tests/libtests.la ../../libgmpxx.la ../../libgmp.la g++ O2 m64 mtune=k8 o .libs/tcast tcast.o L/home/simon/SAGE/sage5.0.beta7/spkg/optional/gmp4.2.1.p11.copy/src/.libs ../../tests/.libs/libtests.a ../../.libs/libgmpxx.so /home/simon/SAGE/sage5.0.beta7/spkg/optional/gmp4.2.1.p11.copy/src/.libs/libgmp.so ../../.libs/libgmp.so Wl,rpath Wl,/home/simon/SAGE/sage5.0.beta7/local/lib creating tcast g++ DHAVE_CONFIG_H I. I. I../.. I../.. I../../tests O2 m64 mtune=k8 c o tconstr.o tconstr.cc /bin/sh ../../libtool mode=link g++ O2 m64 mtune=k8 o tconstr tconstr.o L../../.libs ../../tests/libtests.la ../../libgmpxx.la ../../libgmp.la g++ O2 m64 mtune=k8 o .libs/tconstr tconstr.o L/home/simon/SAGE/sage5.0.beta7/spkg/optional/gmp4.2.1.p11.copy/src/.libs ../../tests/.libs/libtests.a ../../.libs/libgmpxx.so /home/simon/SAGE/sage5.0.beta7/spkg/optional/gmp4.2.1.p11.copy/src/.libs/libgmp.so ../../.libs/libgmp.so Wl,rpath Wl,/home/simon/SAGE/sage5.0.beta7/local/lib creating tconstr g++ DHAVE_CONFIG_H I. I. I../.. I../.. I../../tests O2 m64 mtune=k8 c o theaders.o theaders.cc /bin/sh ../../libtool mode=link g++ O2 m64 mtune=k8 o theaders theaders.o L../../.libs ../../tests/libtests.la ../../libgmpxx.la ../../libgmp.la g++ O2 m64 mtune=k8 o .libs/theaders theaders.o L/home/simon/SAGE/sage5.0.beta7/spkg/optional/gmp4.2.1.p11.copy/src/.libs ../../tests/.libs/libtests.a ../../.libs/libgmpxx.so /home/simon/SAGE/sage5.0.beta7/spkg/optional/gmp4.2.1.p11.copy/src/.libs/libgmp.so ../../.libs/libgmp.so Wl,rpath Wl,/home/simon/SAGE/sage5.0.beta7/local/lib creating theaders g++ DHAVE_CONFIG_H I. I. I../.. I../.. I../../tests O2 m64 mtune=k8 c o tistream.o tistream.cc /bin/sh ../../libtool mode=link g++ O2 m64 mtune=k8 o tistream tistream.o L../../.libs ../../tests/libtests.la ../../libgmpxx.la ../../libgmp.la g++ O2 m64 mtune=k8 o .libs/tistream tistream.o L/home/simon/SAGE/sage5.0.beta7/spkg/optional/gmp4.2.1.p11.copy/src/.libs ../../tests/.libs/libtests.a ../../.libs/libgmpxx.so /home/simon/SAGE/sage5.0.beta7/spkg/optional/gmp4.2.1.p11.copy/src/.libs/libgmp.so ../../.libs/libgmp.so Wl,rpath Wl,/home/simon/SAGE/sage5.0.beta7/local/lib creating tistream g++ DHAVE_CONFIG_H I. I. I../.. I../.. I../../tests O2 m64 mtune=k8 c o tlocale.o tlocale.cc tlocale.cc: In function ‘void check_input()’: tlocale.cc:108:26: error: ‘abort’ was not declared in this scope tlocale.cc:123:26: error: ‘abort’ was not declared in this scope tlocale.cc: In function ‘void check_output()’: tlocale.cc:158:18: error: ‘abort’ was not declared in this scope make[4]: *** [tlocale.o] Fehler 1 make[4]: Leaving directory `/home/simon/SAGE/sage5.0.beta7/spkg/optional/gmp4.2.1.p11.copy/src/tests/cxx' make[3]: *** [checkam] Fehler 2 make[3]: Leaving directory `/home/simon/SAGE/sage5.0.beta7/spkg/optional/gmp4.2.1.p11.copy/src/tests/cxx' make[2]: *** [checkrecursive] Fehler 1 make[2]: Leaving directory `/home/simon/SAGE/sage5.0.beta7/spkg/optional/gmp4.2.1.p11.copy/src/tests' make[1]: *** [checkrecursive] Fehler 1 make[1]: Leaving directory `/home/simon/SAGE/sage5.0.beta7/spkg/optional/gmp4.2.1.p11.copy/src' make: *** [check] Fehler 2
comment:4 Changed 8 years ago by
While I was at it, I tried to upgrade to gmp 5.0.4. Unfortunately, it complains that it can not find bdivmod.c in the path  the old spkg contains that file in src/mpn/generic, but the original gmp 5.0.4 sources do no contain bdivmod.c in src/mpn/generic. Too bad.
comment:5 followup: ↓ 6 Changed 8 years ago by
The advantage of gmp 5.0.4 would be that the sources already include cstdio. So, no need for patching it. The problem is that 5.0.4 has removed and renamed a couple of things. For example, all files starting with the name mullow_
are now starting with mullo_
. The patches of the spkg need to be changed accordingly.
The gmp 5.0.4 package seems to build. Keeping my fingers crossed for the test...
comment:6 in reply to: ↑ 5 Changed 8 years ago by
Replying to SimonKing:
The gmp 5.0.4 package seems to build.
No, it doesn't. I get an infinite loop during configuration, it seems. /bin/sh ./config.status recheck
is repeated over and over again.
comment:7 Changed 8 years ago by
 Description modified (diff)
 Keywords gmp added
 Status changed from new to needs_review
 Summary changed from Fix optional gmp package on openSuse to Upgrade the optional GMP package
comment:8 Changed 8 years ago by
See modified ticket description: It was easy to make gmp 5.0.4 build and pass all tests. Simply one has to drop all patches of the old gmp spkg.
comment:9 followup: ↓ 10 Changed 8 years ago by
A related ticket: #12661 (optional CLooG
spkg).
CLooG
needs gmp to build.
comment:10 in reply to: ↑ 9 Changed 8 years ago by
 Cc leif added
comment:11 Changed 8 years ago by
 Description modified (diff)
comment:12 Changed 8 years ago by
 Description modified (diff)
comment:13 followup: ↓ 14 Changed 8 years ago by
FWIW, I did build Sage against GMP (5.0.1 or 5.0.2) a couple of times in 2010 (when we made the "first big PARI upgrade") and never ran into any problems, except for the Lcalc spkg, which was the only one explicitly using lmpir
instead of lgmp
, an issue which AFAIK meanwhile is fixed.
Although I looked at the (btw. completely outdated) optional GMP spkg (on other occasions as well), I didn't use any of it, in particular didn't apply any additional patches to the more recent GMP versions.
We'll probably only run into trouble with using GMP instead of MPIR the day we upgrade FLINT, as newer (2.x, but IIRC only the most recent) versions of FLINT use some MPIR internals GMP doesn't provide, or at least isn't compatible with.
But for the moment it should be ok to use GMP instead, although (for CLooG) this shouldn't be necessary.
(In contrast, as Dima recently mentioned on sagedevel and mpirdevel, GAP uses some GMP internals MPIR currently doesn't provide. The [older?] version of GAP we use apparently works with MPIR... :) )
comment:14 in reply to: ↑ 13 Changed 8 years ago by
Replying to leif:
Although I looked at the (btw. completely outdated) optional GMP spkg ...
Yes, that's what this ticket is about.
in particular didn't apply any additional patches to the more recent GMP versions.
I'm not using any patches either. The question is whether other platforms (Solaris? OS X?) need patches.
We'll probably only run into trouble with using GMP instead of MPIR the day we upgrade FLINT,
When I first tried to build the optional gcc from #12369 with graphite (hence, together with the optional cloogppl package from #12666), I thought that cloogppl needs GMP (not MPIR). This is why I opened this ticket in the first place.
I found a massive slowdown when I built sage with the optimizations provided by graphite. Big surprise. But some people suggested that the culprit might in fact be GMP. So, I am started over. I can not tell yet whether Sage is faster with MPIR than with GMP. However, GMP is definitely not required by cloogppl, which lessens the priority of this ticket considerably, from my perspective...
comment:15 Changed 7 years ago by
Building the new gmp spkg here on Mac OS X (with gcc 4.6.3) with SAGE_CHECK=yes succeeds and passess all tests!
comment:16 followup: ↓ 17 Changed 7 years ago by
GMP 5.0.5 is out since a while...
comment:17 in reply to: ↑ 16 ; followup: ↓ 18 Changed 7 years ago by
comment:18 in reply to: ↑ 17 Changed 7 years ago by
comment:19 Changed 6 years ago by
 Milestone changed from sage5.11 to sage5.12
comment:20 Changed 6 years ago by
gmp 5.1.x is out and transition to git is needed if we want to keep gmp in sage.
comment:21 Changed 6 years ago by
Yup, also note that FLINT >= 2.4 can now be built on top of GMP. IIRC it's the only spkg which explicitely depended on MPIR.
comment:22 Changed 6 years ago by
 Milestone changed from sage6.1 to sage6.2
comment:23 Changed 6 years ago by
 Cc rws added
Also, mpir was not updated since Nov 2012. However gmp isn't even in optional now, so a new ticket would have to be opened to include it.
comment:24 Changed 6 years ago by
 Description modified (diff)
 Status changed from needs_review to needs_work
 Summary changed from Upgrade the optional GMP package to include GMP as optional package
 Type changed from defect to enhancement
comment:25 Changed 6 years ago by
 Branch set to u/jpflori/ticket/12661
 Description modified (diff)
 Summary changed from include GMP as optional package to Include GMP 5.1.3 as an optional package
 Type changed from enhancement to defect
comment:26 Changed 6 years ago by
 Type changed from defect to enhancement
comment:27 Changed 6 years ago by
 Commit set to 2c34755c939f42814dd31047b98c5aeda84b9a29
Branch pushed to git repo; I updated commit sha1. New commits:
2c34755  Initial commit for GMP 5.1.3.

comment:28 Changed 6 years ago by
Problems with farey_symbol.pyx which uses additional c++ source files (farey.cpp and sl2z.cpp) but cython wants to compile them with CC and that fails when GMP is used underneath. Not sure why it succeeded on top of MPIR.
comment:29 followup: ↓ 32 Changed 6 years ago by
Anyway, it also fails with CXX:
g++ fnostrictaliasing g O2 DNDEBUG g fwrapv O3 Wall fPIC I/home/jpflori/sage.git/local/include I/home/jpflori/sage.git/local/include/csage I/home/jpflori/sage.git/src I/home/jpflori/sage.git/src/sage/ext I/home/jpflori/sage.git/local/include I/home/jpflori/sage.git/local/include/csage I/home/jpflori/sage.git/src I/home/jpflori/sage.git/src/sage/ext I/home/jpflori/sage.git/local/include/python2.7 c sage/modular/arithgroup/farey.cpp o build/temp.linuxppc642.7/sage/modular/arithgroup/farey.o fnostrictaliasing w sage/modular/arithgroup/farey.cpp: In function ‘mpz_class floor(mpq_class)’: sage/modular/arithgroup/farey.cpp:147:44: error: conversion from ‘mpq_class {aka __gmp_expr<__mpq_struct [1], __mpq_struct [1]>}’ to nonscalar type ‘mpz_class {aka __gmp_expr<__mpz_struct [1], __mpz_struct [1]>}’ requested
comment:30 Changed 6 years ago by
Similar probledm and solution here:
comment:31 Changed 6 years ago by
 Commit changed from 2c34755c939f42814dd31047b98c5aeda84b9a29 to a14aaf2fd94252c0e263c8b02003f8c640d70e0e
Branch pushed to git repo; I updated commit sha1. New commits:
a14aaf2  Explicit conversion in farey.cpp as recent GMP do not do it automatically.

comment:32 in reply to: ↑ 29 ; followup: ↓ 33 Changed 6 years ago by
Replying to jpflori:
Anyway, it also fails with CXX:
g++ fnostrictaliasing g O2 DNDEBUG g fwrapv O3 Wall fPIC I/home/jpflori/sage.git/local/include I/home/jpflori/sage.git/local/include/csage I/home/jpflori/sage.git/src I/home/jpflori/sage.git/src/sage/ext I/home/jpflori/sage.git/local/include I/home/jpflori/sage.git/local/include/csage I/home/jpflori/sage.git/src I/home/jpflori/sage.git/src/sage/ext I/home/jpflori/sage.git/local/include/python2.7 c sage/modular/arithgroup/farey.cpp o build/temp.linuxppc642.7/sage/modular/arithgroup/farey.o fnostrictaliasing w sage/modular/arithgroup/farey.cpp: In function ‘mpz_class floor(mpq_class)’: sage/modular/arithgroup/farey.cpp:147:44: error: conversion from ‘mpq_class {aka __gmp_expr<__mpq_struct [1], __mpq_struct [1]>}’ to nonscalar type ‘mpz_class {aka __gmp_expr<__mpz_struct [1], __mpz_struct [1]>}’ requested
Sorry we could have warned you about that since sageongentoo is stuck on gmp we knew about this and how to fix it.
comment:33 in reply to: ↑ 32 Changed 6 years ago by
Replying to fbissey:
Replying to jpflori:
Anyway, it also fails with CXX:
g++ fnostrictaliasing g O2 DNDEBUG g fwrapv O3 Wall fPIC I/home/jpflori/sage.git/local/include I/home/jpflori/sage.git/local/include/csage I/home/jpflori/sage.git/src I/home/jpflori/sage.git/src/sage/ext I/home/jpflori/sage.git/local/include I/home/jpflori/sage.git/local/include/csage I/home/jpflori/sage.git/src I/home/jpflori/sage.git/src/sage/ext I/home/jpflori/sage.git/local/include/python2.7 c sage/modular/arithgroup/farey.cpp o build/temp.linuxppc642.7/sage/modular/arithgroup/farey.o fnostrictaliasing w sage/modular/arithgroup/farey.cpp: In function ‘mpz_class floor(mpq_class)’: sage/modular/arithgroup/farey.cpp:147:44: error: conversion from ‘mpq_class {aka __gmp_expr<__mpq_struct [1], __mpq_struct [1]>}’ to nonscalar type ‘mpz_class {aka __gmp_expr<__mpz_struct [1], __mpz_struct [1]>}’ requestedSorry we could have warned you about that since sageongentoo is stuck on gmp we knew about this and how to fix it.
No problem, I found the solution more than easily on the web.
Any other suggestion from sageongentoo? E.g., do you have devised any simple way to switch from gmp to mpir and back again easily? Or are you really stuck with gmp and cannot use mpir? (that would be strange.)
comment:34 Changed 6 years ago by
We are really stuck with gmp unless we make great effort. It is complicated because most of the package of the tree will depend on gmp and few will use mpir directly, at some point I tried to port a lot of ebuild to mpir directly but it was a bit unsustainable. The easiest way is to use the gmp compatibility layer from mpir. Then we have to convince the toolchain people that we should have the possibility of switching between gmp and mpir to build gcc. Then we need to replace all dependencies to gmp in the tree by some virtual package which can be either gmp or mpir passing as gmp. That's a ton of work, require convincing people and so on.
if all package in sage could be configured with either real gmp or real mpir out of the box we may be able to do some easy switching between the two but it is not the case. lmona.de could go its own way and use mpir exclusively if they wanted too (by calling it gmp mainly).
comment:35 Changed 6 years ago by
FTR here are the only failing doctests:
sage t long src/sage/rings/integer.pyx ********************************************************************** File "src/sage/rings/integer.pyx", line 1955, in sage.rings.integer.Integer.__pow__ Failed example: 2^(2^632) Expected: Traceback (most recent call last): ... MemoryError: failed to allocate 1152921504606847008 bytes Got: gmp: overflow in mpz type Traceback (most recent call last): File "/home/jpflori/sage.git/local/lib/python2.7/sitepackages/sage/doctest/fo rker.py", line 480, in _run self.execute(example, compiled, test.globs) File "/home/jpflori/sage.git/local/lib/python2.7/sitepackages/sage/doctest/fo rker.py", line 839, in execute exec compiled in globs File "<doctest sage.rings.integer.Integer.__pow__[27]>", line 1, in <module> Integer(2)**(Integer(2)**Integer(63)Integer(2)) File "integer.pyx", line 1994, in sage.rings.integer.Integer.__pow__ (sage/rin gs/integer.c:14075) File "c_lib.pyx", line 85, in sage.ext.c_lib.sig_raise_exception (sage/ext/c_l ib.c:1009) RuntimeError: Aborted ********************************************************************** File "src/sage/rings/integer.pyx", line 5401, in sage.rings.integer.Integer._xgcd Failed example: 2._xgcd(2) Expected: (2, 1, 0) Got: (2, 0, 1) **********************************************************************
and
sage t long src/sage/tests/book_stein_modform.py ********************************************************************** File "src/sage/tests/book_stein_modform.py", line 94, in sage.tests.book_stein_modform Failed example: [x.lift_to_sl2z(2) for x in M.manin_generators()] Expected: [[1, 0, 0, 1], [0, 1, 1, 0], [0, 1, 1, 1]] Got: [[1, 0, 0, 1], [0, 1, 1, 0], [1, 0, 1, 1]] **********************************************************************
comment:36 Changed 6 years ago by
I got the first one in sage/rings/integer.pyx and knew it was from gmp vs mpir. I also have sage/tests/book_stein_modform.py but I didn't know it was originating from gmp vs mpir. I don't have the second failure in integer.pyx it may be because I am currently only at gmp 5.1.2.
comment:37 Changed 6 years ago by
The ones in integer.pyx are really harmless anyway.
Not sure about the one in the Manin generators computation, so I could be easily convinced it is innocuous as well.
comment:38 Changed 6 years ago by
Actually I got the second one in integer.pyx. It wasn't present when I last made a record of doctests failures in sageongentoo for sage 6.0, but it is in 6.2.beta2.
comment:39 Changed 6 years ago by
Any reason why this is still stuck at "need work"? I can give this a positive review, fixing the doctests to work with gmp is probably more work than it is worth. Especially the top one in integer.pyx.
Opinions?
comment:40 Changed 6 years ago by
I agree that I can live with the failing doctests.
As far as setting this to "needs_review" is concerned, I did not change the status because I wanted to include some patches to let SAge actually use GMP rather than MPIR and even potentially being able to switch from one to the other.
comment:41 Changed 6 years ago by
The only thing I can see right now that prevent sage to build with gmp out of the box is the inclusion of mpir.h instead of gmp.h in memory.c in clib.
comment:42 Changed 6 years ago by
Flint install script should be modified as well. Ok maybe it works without modification but passing withgmp is better than withmpir now that it is supported.
comment:43 Changed 6 years ago by
And you should also prevent Sage from building mpir I guess.
comment:44 Changed 6 years ago by
Ambitious I see. Of course I have total control in sageongentoo but I would appreciate just having to switch withgmp or something.
comment:45 Changed 6 years ago by
Sure.
I would be ok to postpone the proper way to use gmp instead of mpir for sage the distrib in a follow up ticket. Let me dig up the few simple fixes needed for minimal support and push them here first.
comment:46 Changed 6 years ago by
 Status changed from needs_work to needs_review
Ok so everything is in fact already here.
comment:47 Changed 6 years ago by
Follow up at #15957.
comment:48 Changed 6 years ago by
 Reviewers set to François Bissey
 Status changed from needs_review to positive_review
I don't know that the renaming of sage_mpir_* to sage_gmp_* is necessary. In fact I am not even sure it needs to be named gmp or mpir at all. Aside from that it is all ok and conform to what we do in sageongentoo. It also would cross a patch and a sed from my ebuilds at least.
comment:49 Changed 6 years ago by
 Branch changed from u/jpflori/ticket/12661 to a14aaf2fd94252c0e263c8b02003f8c640d70e0e
 Resolution set to fixed
 Status changed from positive_review to closed
Google told me that some people suggested to "include cstdio to obtain std::FILE". So, I tried to add
to gmp.h. However, after I attempted to build it, that line was gone!!
Can someone explain how I can avoid that my editing the file is reverted? Perhaps I need to do my change in the "patches" directory?