Opened 9 years ago
Last modified 8 years ago
#11130 closed defect
Update PARI to version 2.5.0 — at Version 130
Reported by: | jdemeyer | Owned by: | jdemeyer |
---|---|---|---|
Priority: | critical | Milestone: | sage-4.8 |
Component: | packages: standard | Keywords: | pari spkg sd32 |
Cc: | mstreng, dimpase, jpflori | Merged in: | |
Authors: | Jeroen Demeyer, John Cremona | Reviewers: | John Cremona, Jeroen Demeyer |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | #11304, #11540, #11321 (install this after building PARI) | Stopgaps: |
Description (last modified by )
We need to add bugfixes for
- http://pari.math.u-bordeaux.fr/cgi-bin/bugreport.cgi?bug=1147 (related to #9334)
- http://pari.math.u-bordeaux.fr/cgi-bin/bugreport.cgi?bug=1153 (#10195)
- http://pari.math.u-bordeaux.fr/cgi-bin/bugreport.cgi?bug=1154 (#11604)
- http://pari.math.u-bordeaux.fr/cgi-bin/bugreport.cgi?bug=1184 (related to #9937)
- http://pari.math.u-bordeaux.fr/cgi-bin/bugreport.cgi?bug=1185 (PARI bug discovered by a Sage doctest)
- http://pari.math.u-bordeaux.fr/cgi-bin/bugreport.cgi?bug=1186 (PARI performance regression discovered by a Sage doctest)
- http://pari.math.u-bordeaux.fr/cgi-bin/bugreport.cgi?bug=1187 (PARI bug discovered by a Sage doctest)
- http://pari.math.u-bordeaux.fr/cgi-bin/bugreport.cgi?bug=1188 (PARI bug discovered by a Sage doctest)
- http://pari.math.u-bordeaux.fr/cgi-bin/bugreport.cgi?bug=1192 (#10767)
We will do this by updating to PARI version 2.5.0, which is equal to svn revision 13228.
See also:
- #9343, #10430 (previous PARI upgrades)
- #10247 (Sage 4.6 has PARI problems on Cygwin)
- #10240 (pari-2.4.3.svn-12577.p9 incorrectly checks for the shared library on Cygwin), included in the 2.4.3.alpha.p7 spkg at #11605
- #11605 (Typos in PARI's spkg-install (2.4.3.alpha.p5)), included in the 2.4.3.alpha.p7 spkg at #11605
Apply:
- spkg http://boxen.math.washington.edu/home/jdemeyer/spkg/pari-2.5.0.p2.spkg (jdemeyer)
- spkg http://boxen.math.washington.edu/home/jdemeyer/spkg/lcalc-1.23.p9.spkg (from #11321)
- 11130_sagelib.patch (jdemeyer, cremona)
- 11130-sagelib-simon-v2.patch (cremona, jdemeyer)
- 11130_reviewer32.patch (jdemeyer)
- 11130-4.7.2.alpha3.patch (jdemeyer)
- extcode patch 11130-extcode-v2.patch (cremona, jdemeyer) needs_review
Only the last patch still needs review, the rest already has positive review.
Change History (140)
comment:1 Changed 9 years ago by
- Description modified (diff)
- Summary changed from Add some patches to PARI to Update PARI to a more recent SVN version
comment:2 Changed 9 years ago by
- Description modified (diff)
- Status changed from new to needs_work
comment:3 Changed 9 years ago by
- Description modified (diff)
comment:4 Changed 9 years ago by
- Cc mstreng added
comment:5 Changed 9 years ago by
- Milestone changed from sage-4.7 to sage-4.7.1
comment:6 Changed 9 years ago by
- Description modified (diff)
comment:7 Changed 9 years ago by
- Description modified (diff)
comment:8 Changed 9 years ago by
- Description modified (diff)
comment:9 Changed 9 years ago by
- Description modified (diff)
comment:10 Changed 9 years ago by
- Description modified (diff)
comment:11 Changed 9 years ago by
- Description modified (diff)
comment:12 Changed 9 years ago by
- Dependencies set to #11230
- Description modified (diff)
comment:13 Changed 9 years ago by
- Description modified (diff)
comment:14 Changed 9 years ago by
- Description modified (diff)
comment:15 Changed 9 years ago by
- Cc dimpase added
comment:16 Changed 9 years ago by
- Description modified (diff)
comment:17 follow-up: ↓ 22 Changed 9 years ago by
comment:18 follow-up: ↓ 19 Changed 9 years ago by
After (1) Building a fresh 4.7.alpha5 from scratch, (2) installing the spkg lined here with "sage -f", (3) applying the patch here to the sage library and (4) running "sage -b", I find that the Sage build will not start up properly:
ImportError Traceback (most recent call last) ... /home/jec/sage-4.7.alpha5.11130/local/lib/python2.6/site-packages/sage/misc/functional.py in <module>() 36 37 ---> 38 from sage.rings.complex_double import CDF 39 from sage.rings.real_double import RDF, RealDoubleElement 40 /home/jec/sage-4.7.alpha5.11130/local/bin/gen.pxd in init sage.rings.complex_double (sage/rings/complex_double.c:15178)() ImportError: /home/jec/sage-4.7.alpha5.11130/local/lib/python2.6/site-packages/sage/libs/pari/gen.so: undefined symbol: defaultOut Error importing ipy_profile_sage - perhaps you should run %upgrade? WARNING: Loading of ipy_profile_sage failed.
so I cannot test anything.
Did I do something wrong?
comment:19 in reply to: ↑ 18 ; follow-up: ↓ 20 Changed 9 years ago by
comment:20 in reply to: ↑ 19 Changed 9 years ago by
comment:21 Changed 9 years ago by
- Dependencies changed from #11230 to #11230, #11234
- Description modified (diff)
- Owner changed from tbd to jdemeyer
comment:22 in reply to: ↑ 17 Changed 9 years ago by
Replying to cremona:
I am currently testing this. To avoid duplication of effort, I'll fix things I find in the elliptic_curves directory only.
Sounds good, I will look at everything else.
comment:23 Changed 9 years ago by
I added a patch which fixes the doctest failures in sage/rings/polynomial/polynomial_quotient_ring.py . In all cases the new output was equivalent to the old.
There are now just some failures in sage/schemes/elliptic_curves/ell_rational_field.py (2 doctests failed) and sage/schemes/elliptic_curves/ell_number_field.py (1 doctests failed).
comment:24 Changed 9 years ago by
- Reviewers set to John Cremona
- Status changed from needs_work to needs_review
ell_rational_field.py: only one failure, looks completely trivial but will be left until #11005 to be fixed.
ell_number_field.py: one failure, even more trivial: the final output is exactly the same as it was but the verbose output is a little different, so this can be easily cleaned up at #11005.
As far as I am concerned this needs *no more work* on this ticket, but I'll delay marking it positive review until the same is possible at #11005. Still, it certainly gets "needs review" on that basis.
comment:25 follow-up: ↓ 26 Changed 9 years ago by
This patch should also be tested on various systems first, PARI is known to produce different results on 32-bit and 64-bit systems.
comment:26 in reply to: ↑ 25 Changed 9 years ago by
Replying to jdemeyer:
This patch should also be tested on various systems first, PARI is known to produce different results on 32-bit and 64-bit systems.
Good point. I can test it on my 32-bit laptop.
comment:27 Changed 9 years ago by
- Description modified (diff)
- Work issues set to Test on various systems
comment:28 Changed 9 years ago by
- Description modified (diff)
comment:29 Changed 9 years ago by
- Description modified (diff)
comment:30 Changed 9 years ago by
- Work issues changed from Test on various systems to Test on various systems with #10247 applied
comment:31 Changed 9 years ago by
- Status changed from needs_review to needs_work
In sage/rings/number_field/number_field.py
, there is a test which fails only when -long
is not used.
comment:32 Changed 9 years ago by
New patch fixes the long time issue (by setting a random seed explicitly). I will test on a Mac OS X 10.4 32-bit PPC system.
comment:33 Changed 9 years ago by
comment:34 follow-up: ↓ 35 Changed 9 years ago by
Testing now on a 32-bit ubuntu system.
comment:35 in reply to: ↑ 34 Changed 9 years ago by
Replying to cremona:
Testing now on a 32-bit ubuntu system.
... which gave several more failures:
sage -t devel/sage-main/sage/rings/number_field/number_field.py # 1 doctests failed sage -t devel/sage-main/sage/schemes/elliptic_curves/ell_number_field.py # 2 doctests failed sage -t devel/sage-main/sage/schemes/elliptic_curves/ell_rational_field.py # 1 doctests failed sage -t devel/sage-main/sage/rings/number_field/number_field_element.pyx # 1 doctests failed sage -t devel/sage-main/sage/rings/polynomial/polynomial_quotient_ring.py # 3 doctests failed sage -t devel/sage-main/sage/rings/integer.pyx # 1 doctests failed
Details:
sage -t "devel/sage-main/sage/rings/number_field/number_field.py" ********************************************************************** File "/home/john/sage-4.7.alpha5/devel/sage-main/sage/rings/number_field/number_field.py", line 3616: sage: L.factor(a + 1) Expected: (Fractional ideal (1/2*a*b + a + 1/2)) * (Fractional ideal (-1/2*b - 1/2*a + 1)) Got: (Fractional ideal (-1/2*a*b - a - 1/2)) * (Fractional ideal (1/2*b + 1/2*a - 1))
is trivial,
sage -t "devel/sage-main/sage/rings/number_field/number_field_element.pyx" ********************************************************************** File "/home/john/sage-4.7.alpha5/devel/sage-main/sage/rings/number_field/number_field_element.pyx", line 1142: sage: t = (2*a + b)._rnfisnorm(L); t[1] Expected: (b - 2)*a + 2*b - 3 Got: (-4*b - 5)*a + 5*b + 6
needs looking at,
sage -t "devel/sage-main/sage/rings/polynomial/polynomial_quotient_ring.py"
are just like the ones I fixed before on the other machine, and
sage -t "devel/sage-main/sage/rings/integer.pyx" ********************************************************************** File "/home/john/sage-4.7.alpha5/devel/sage-main/sage/rings/integer.pyx", line 4389: sage: 7._bnfisnorm(CyclotomicField(7)) Expected: (-zeta7 + 1, 1) Got: (-zeta7^5 + zeta7^4, 1)
is also trivial (multiplying by a power of zeta does not change the norm!).
That's all I have time for today, so I hope things look similar on the other 32-bit system being tested.
comment:36 Changed 9 years ago by
- Description modified (diff)
I have added a new patch (which affects 4 files in sage/rings) after which all doctests pass on both 32- and 64-bit, apart from the simon-related ones.
That means that it is time to check #11005 on top of all this.
comment:37 Changed 9 years ago by
- Description modified (diff)
- Work issues Test on various systems with #10247 applied deleted
comment:38 Changed 9 years ago by
- Description modified (diff)
comment:39 Changed 9 years ago by
- Description modified (diff)
- Summary changed from Update PARI to a more recent SVN version to Update PARI to version 2.4.4.BETA
comment:40 Changed 9 years ago by
- Description modified (diff)
comment:41 follow-up: ↓ 42 Changed 9 years ago by
lcalc fails to build with the new PARI:
Lcommandline.cc: In function 'int main(int, char**)': Lcommandline.cc:476:54: error: 'allocatemoremem' was not declared in this scope Lcommandline.cc:403:16: warning: unused variable 'C' [-Wunused-variable] Lcommandline.cc:38:9: warning: unused variable 'i' [-Wunused-variable] In file included from ../include/L.h:537:0, from ../include/Lcommandline.h:36, from Lcommandline.cc:31: ../include/Lvalue.h: In member function 'Complex L_function<ttype>::find_delta(Complex, Double) [with ttype = int, Complex = std::complex<double>, Double = double]': Lcommandline.cc:463:50: instantiated from here ../include/Lvalue.h:37:21: warning: unused variable 'f2' [-Wunused-variable] make[1]: *** [Lcommandline.o] Error 1 make[1]: Leaving directory `/usr/local/src/sage-4.7.alpha5/spkg/build/lcalc-20100428-1.23.p6/src/src' make: *** [all] Error 2 Error building lcalc 'make'
comment:42 in reply to: ↑ 41 ; follow-up: ↓ 45 Changed 9 years ago by
Replying to jdemeyer:
lcalc fails to build with the new PARI:
Mike Rubenstein needs to be made aware of this.
comment:43 follow-up: ↓ 44 Changed 9 years ago by
What about genus2reduction? I am assuming John already made sure eclib is working.
comment:44 in reply to: ↑ 43 Changed 9 years ago by
Replying to fbissey:
What about genus2reduction? I am assuming John already made sure eclib is working.
A reasonable assumption, but...for a while now I have used Sage's Pari and NTL builds rather than building them myself independently, so I cannot yet confirm. (However, eclib's use of Pari is minimal, it's only for integer factorization.)
I am also preparing a new version of eclib, so I will combine these two tasks, though I don't have any time for this this week.
comment:45 in reply to: ↑ 42 Changed 9 years ago by
comment:46 Changed 9 years ago by
- Dependencies changed from #11230, #11234 to #11230, #11234, #11321
- Description modified (diff)
See #11321 for lcalc.
comment:47 Changed 9 years ago by
- Dependencies changed from #11230, #11234, #11321 to #11230, #11234, #11321 (install this '''after''' building PARI)
comment:48 follow-up: ↓ 49 Changed 9 years ago by
- Work issues set to Fix simon_two_descent doctests
I built Sage completely from scratch with the new PARI and lcalc packages, everything works. All tests pass except for the ones related to simon_two_descent.
comment:49 in reply to: ↑ 48 ; follow-up: ↓ 50 Changed 9 years ago by
Replying to jdemeyer:
I built Sage completely from scratch with the new PARI and lcalc packages, everything works. All tests pass except for the ones related to simon_two_descent.
So is the "Needs work" just because this ticket is dependent on #11005? I am planning to look now at the doctest failures relating to the Simon scripts and fix them, so that this ticket can then be closed, and the additional work needed on #11005 can depend on this.
comment:50 in reply to: ↑ 49 ; follow-up: ↓ 51 Changed 9 years ago by
- Description modified (diff)
comment:51 in reply to: ↑ 50 Changed 9 years ago by
Replying to jdemeyer:
Replying to cremona:
I am planning to look now at the doctest failures relating to the Simon scripts and fix them, so that this ticket can then be closed, and the additional work needed on #11005 can depend on this.
That's precisely what I proposed.
OK, so that's what I am working on doing. Patch up shortly. However, I ran into something awkward: the script ellQ.gp in data/extcode/pari/simon crashed randomly in the function ellredgen (using this new pari version). I saw that the offending lines had been changed in the newer ellQ.gp as found in #11005, so I made the same changes. This sorts the problem, but it means that for this ticket to be finished we need to also make a small patch to extcode. Any problems with that?
comment:52 Changed 9 years ago by
- Description modified (diff)
- Status changed from needs_work to needs_review
comment:53 Changed 9 years ago by
- Description modified (diff)
comment:54 Changed 9 years ago by
- Description modified (diff)
- Work issues Fix simon_two_descent doctests deleted
comment:55 Changed 9 years ago by
Is the only thing stopping this going forward the latest reviewer patch?
comment:56 follow-up: ↓ 57 Changed 9 years ago by
As far as I know, all patches and spkgs still need review. I can review your patches (but give me some time).
comment:57 in reply to: ↑ 56 Changed 9 years ago by
Replying to jdemeyer:
As far as I know, all patches and spkgs still need review. I can review your patches (but give me some time).
Well I considered that I had reviewed (positively) everything except my last patch fixing the Simon doctests so that's the only thing left needing a review (from you perhaps, especially as you made some extra fixes for 32/64 bit issues).
comment:58 Changed 9 years ago by
Also lcalc (#11321) needs review.
comment:59 Changed 9 years ago by
The part of the ticket that fixes #10240 is okay, I think.
comment:60 Changed 9 years ago by
- Description modified (diff)
- Status changed from needs_review to needs_work
- Summary changed from Update PARI to version 2.4.4.BETA to Update PARI to version 2.5.0
comment:61 Changed 9 years ago by
See #10240 for a request to merge that in 4.7.2 (or whatever the next version is) if this isn't getting merged quickly.
comment:62 Changed 9 years ago by
comment:63 Changed 9 years ago by
- Description modified (diff)
comment:64 Changed 9 years ago by
- Description modified (diff)
Needs to be rebased to sage-2.4.3.alpha7.spkg (#11605).
comment:65 Changed 9 years ago by
- Work issues set to Rebase on PARI(!) 2.4.3.alpha.p7 spkg from #11605
comment:66 Changed 9 years ago by
- Description modified (diff)
comment:67 Changed 9 years ago by
- Description modified (diff)
- Work issues Rebase on PARI(!) 2.4.3.alpha.p7 spkg from #11605 deleted
comment:68 Changed 9 years ago by
- Description modified (diff)
- Status changed from needs_work to needs_review
comment:69 Changed 9 years ago by
- Status changed from needs_review to needs_work
The spkg does not build on Mac OS X because of an upstream bug. There is a fix in PARI svn revision 13318 which I should add to this spkg.
comment:70 Changed 9 years ago by
My comment refered to a Mac OS X 10.4 PPC machine, other machines might still work.
comment:71 Changed 9 years ago by
- Description modified (diff)
- Status changed from needs_work to needs_review
New spkg fixing the OS X issue: http://boxen.math.washington.edu/home/jdemeyer/spkg/pari-2.5.0.p2.spkg
comment:72 Changed 9 years ago by
- Status changed from needs_review to needs_work
Never mind, still doesn't work. Need to also add revision 13330.
comment:73 Changed 8 years ago by
- Status changed from needs_work to needs_review
New version which does build on OS X 10.4, same location: http://boxen.math.washington.edu/home/jdemeyer/spkg/pari-2.5.0.p2.spkg
comment:74 Changed 8 years ago by
- Milestone changed from sage-4.7.1 to sage-4.7.2
- Priority changed from major to critical
I did a small rebase of 11130_sagelib.patch regarding sage/matrix/matrix2.pyx
comment:75 follow-up: ↓ 76 Changed 8 years ago by
- Status changed from needs_review to needs_info
Before I set about applying everything and testing, which I will do, can you tell me which Sage version I should be patching against? I have 4.7.1.rc1, is that the best? Or plain 4.7?
comment:76 in reply to: ↑ 75 ; follow-up: ↓ 77 Changed 8 years ago by
- Reviewers changed from John Cremona to John Cremona, Jeroen Demeyer
- Status changed from needs_info to needs_review
Replying to cremona:
Before I set about applying everything and testing, which I will do, can you tell me which Sage version I should be patching against? I have 4.7.1.rc1, is that the best? Or plain 4.7?
sage-4.7.1.rc1 for sure.
I am also preparing a Sage tarball with all spkgs and patches already applied, it should appear in http://boxen.math.washington.edu/home/jdemeyer/release/sage-4.7.1.rc1-pari-2.5.0/
comment:77 in reply to: ↑ 76 Changed 8 years ago by
Replying to jdemeyer:
Replying to cremona:
Before I set about applying everything and testing, which I will do, can you tell me which Sage version I should be patching against? I have 4.7.1.rc1, is that the best? Or plain 4.7?
sage-4.7.1.rc1 for sure.
OK, I am testing against that right now... John
I am also preparing a Sage tarball with all spkgs and patches already applied, it should appear in http://boxen.math.washington.edu/home/jdemeyer/release/sage-4.7.1.rc1-pari-2.5.0/
comment:78 follow-up: ↓ 81 Changed 8 years ago by
Starting with a clean 4.7.1.rc1 (I think) I successfully applied both spkgs (using SAGE_CHECK=yes) and 5 patches to sagelib and 1 to data/extcode, and did sage -ba.
Sage now starts up OK but I'm having a lot of problems with any more testing. For example, moving into devel/sage and doing "../../sage -tp 20 ./sage" results in very many errors, looks like in every file, like this:
Traceback (most recent call last): File "/home/jec/.sage//tmp/facade_sets.py", line 2, in <module> from sage.all_cmdline import *; File "/home/jec/sage-4.7.1.rc1/local/lib/python/site-packages/sage/all_cmdline.py", line 24, in <module> raise ValueError, msg ValueError: Attempted relative import in non-package
I checked (using bash's history) that I had remembered to hg qpush each patch.
Does this make any sense? I suppose I could rebuild 4.7.1.rc1 from scratch.
comment:79 follow-up: ↓ 80 Changed 8 years ago by
hen trying to install this on Cygwin, I get a lot of errors while trying to create the binary gp-2.5.exe along the lines of the following three types.
.../libpari.a(default.o): In function `sd_prompt': .../default.c:962: multiple definition of `_sd_prompt' gp.o:.../gp/gp.c:2359: first defined here
and
gp.o:gp.c:(.text+0x9e2): undefined reference to `_err_printf'
(lots of this kind of thing, for many functions), and
.../libpari.a(mp.o): In function `umodiu': .../kernel/gmp/mp.c::567: undefined reference to `__imp____gmpn_mod_1'
The latter type I've already seen while trying to ./sage -f the previous Pari (2.4.3.alpha.px), but I haven't seen the first kind or second kind before. Something similar to the third one I've seen in some build notes of William's as being a problem with gmp being -I'ed before some other include directories, and the first one looks like one of these "double inclusion" problems.
comment:80 in reply to: ↑ 79 ; follow-up: ↓ 82 Changed 8 years ago by
Replying to kcrisman:
When trying to install this on Cygwin, I get a lot of errors while trying to create the binary gp-2.5.exe along the lines of the following three types.
.../libpari.a(default.o): In function `sd_prompt': .../default.c:962: multiple definition of `_sd_prompt' gp.o:.../gp/gp.c:2359: first defined here
Looks as if you're somehow^{TM} mixing the old PARI (at least the static library libpari.a
) with the new one; PARI 2.5.0 doesn't have an sd_prompt()
function in default.c
at all.
and
gp.o:gp.c:(.text+0x9e2): undefined reference to `_err_printf'
... while this function doesn't exist in the older PARI.
(lots of this kind of thing, for many functions), and
.../libpari.a(mp.o): In function `umodiu': .../kernel/gmp/mp.c::567: undefined reference to `__imp____gmpn_mod_1'
The latter type I've already seen while trying to ./sage -f the previous Pari (2.4.3.alpha.px), but I haven't seen the first kind or second kind before. Something similar to the third one I've seen in some build notes of William's as being a problem with gmp being -I'ed before some other include directories, and the first one looks like one of these "double inclusion" problems.
comment:81 in reply to: ↑ 78 ; follow-up: ↓ 86 Changed 8 years ago by
Replying to cremona:
ValueError?: Attempted relative import in non-package
I have no idea why this happens. Can you try to build http://boxen.math.washington.edu/home/jdemeyer/release/sage-4.7.1.rc1-pari-2.5.0/sage-4.7.1.rc1-pari-2.5.0.tar from scratch and test that? If this works, probably you did something wrong in merging the spkgs/patches.
comment:82 in reply to: ↑ 80 ; follow-up: ↓ 83 Changed 8 years ago by
Looks as if you're somehow^{TM} mixing the old PARI (at least the static library
libpari.a
) with the new one; PARI 2.5.0 doesn't have ansd_prompt()
function indefault.c
at all.
Interestingly, it works fine from scratch (building)! Sorry for the noise, if noise it was.
Is there something about Pari that you shouldn't build it twice in the same location? I notice that it removes and replaces various things in $SAGE_LOCAL
before it's all done, so maybe that could happen. In principle, one should be able to ./sage -f
a new version, have something break, and then be able to ./sage -f
the original version and have everything fine. Or not even HAVE to ./sage -f
the original version. The fact that this doesn't work is not good - shouldn't we only copy things over/delete things after the build has successfully completed?
comment:83 in reply to: ↑ 82 ; follow-up: ↓ 85 Changed 8 years ago by
Replying to kcrisman:
Looks as if you're somehow^{TM} mixing the old PARI (at least the static library
libpari.a
) with the new one; PARI 2.5.0 doesn't have ansd_prompt()
function indefault.c
at all.Interestingly, it works fine from scratch (building)! Sorry for the noise, if noise it was.
Is there something about Pari that you shouldn't build it twice in the same location? I notice that it removes and replaces various things in
$SAGE_LOCAL
before it's all done, so maybe that could happen. In principle, one should be able to./sage -f
a new version, have something break, and then be able to./sage -f
the original version and have everything fine. Or not even HAVE to./sage -f
the original version. The fact that this doesn't work is not good - shouldn't we only copy things over/delete things after the build has successfully completed?
Well the gp executable is linked against libpari and there is a danger that the old one will be used when linking if you are not careful enough. It should be ok in principle but the precaution of removing previous libpari is a good one.
comment:84 Changed 8 years ago by
Just a quick question to confirm: do we need to apply the patch to extcode without worrying about #11005? It is not a dependency of this bug.
comment:85 in reply to: ↑ 83 Changed 8 years ago by
Replying to fbissey:
Replying to kcrisman:
Looks as if you're somehow^{TM} mixing the old PARI (at least the static library
libpari.a
) with the new one; PARI 2.5.0 doesn't have ansd_prompt()
function indefault.c
at all.Interestingly, it works fine from scratch (building)! Sorry for the noise, if noise it was.
Is there something about Pari that you shouldn't build it twice in the same location? I notice that it removes and replaces various things in
$SAGE_LOCAL
before it's all done, so maybe that could happen.
The PARI spkg doesn't delete anything of previous installs in advance. (The .alpha
.p5
though had a bug which caused the install script not to exit with an error if make
failed. Also, on Cygwin make -k
("keep going") was used, but that shouldn't matter here, and has been removed in the 2.5.0 spkgs.)
In principle, one should be able to
./sage -f
a new version, have something break, and then be able to./sage -f
the original version and have everything fine. Or not even HAVE to./sage -f
the original version.
Both should^{TM} be possible, at least unless the build succeeds but the actual installation fails at some point, in which case the former method should still work.
The fact that this doesn't work is not good - shouldn't we only copy things over/delete things after the build has successfully completed?
That's what we do.
Well the gp executable is linked against libpari and there is a danger that the old one will be used when linking if you are not careful enough. It should be ok in principle but the precaution of removing previous libpari is a good one.
Things are a little different on Cygwin, since the Sage Cygwin developers ;-) copy the DLL also to $SAGE_LOCAL/bin
such that gp
will find its shared library also there.
PARI (2.4.3) prepends its build path (with -L
) when using -lpari
, except for when it links gp
[-2.4
] "in place", but this happens after the new shared library (.so*
, *.dll
on Cygwin) has been copied and new symbolic links have been created. The new static library is built and copied later though, but that shouldn't matter unless gp
is statically linked on Cygwin, which shouldn't be the case.
If despite that something weird happens, this is IMHO limited to Cygwin; you may attach your spkg install logs ($SAGE_ROOT/spkg/logs/pari-*.log
) such that we can check this.
We also didn't have problems with upgrading to 2.4.3 btw., except that initially some dependencies of Sage's extension modules were missing such that those didn't get rebuilt in the first place (and used the old ones with a different version they had been linked to earlier).
comment:86 in reply to: ↑ 81 ; follow-up: ↓ 87 Changed 8 years ago by
Replying to jdemeyer:
Replying to cremona:
ValueError?: Attempted relative import in non-package
I have no idea why this happens. Can you try to build http://boxen.math.washington.edu/home/jdemeyer/release/sage-4.7.1.rc1-pari-2.5.0/sage-4.7.1.rc1-pari-2.5.0.tar from scratch and test that? If this works, probably you did something wrong in merging the spkgs/patches.
I'm downloading that now and will report back.
comment:87 in reply to: ↑ 86 ; follow-up: ↓ 88 Changed 8 years ago by
Replying to cremona:
Replying to jdemeyer:
Replying to cremona:
ValueError?: Attempted relative import in non-package
I have no idea why this happens. Can you try to build http://boxen.math.washington.edu/home/jdemeyer/release/sage-4.7.1.rc1-pari-2.5.0/sage-4.7.1.rc1-pari-2.5.0.tar from scratch and test that? If this works, probably you did something wrong in merging the spkgs/patches.
I'm downloading that now and will report back.
Built with no problems. But "make ptestlong" failed 100% just as before. The install and ptestlong logs are at http://www.warwick.ac.uk/staff/J.E.Cremona/ptestlong.log and http://www.warwick.ac.uk/staff/J.E.Cremona/install.log
comment:88 in reply to: ↑ 87 ; follow-up: ↓ 89 Changed 8 years ago by
Replying to cremona:
Built with no problems. But "make ptestlong" failed 100% just as before. The install and ptestlong logs are at http://www.warwick.ac.uk/staff/J.E.Cremona/ptestlong.log and http://www.warwick.ac.uk/staff/J.E.Cremona/install.log
The strange thing is that the traceback you get seems totally unrelated to PARI.
comment:89 in reply to: ↑ 88 Changed 8 years ago by
Replying to jdemeyer:
Replying to cremona:
Built with no problems. But "make ptestlong" failed 100% just as before. The install and ptestlong logs are at http://www.warwick.ac.uk/staff/J.E.Cremona/ptestlong.log and http://www.warwick.ac.uk/staff/J.E.Cremona/install.log
The strange thing is that the traceback you get seems totally unrelated to PARI.
I know, it is mad. After this happened with my 4.7.1.rc1 yesterday I wanted to review something else, so I did that using my 4.7.1.rc0 which was untouched, and found the exact same errors arising whenever I tried "sage -t". This is on a machine I have been using for Sage development and testing for a year, on which no-one but me installs stuff.
Here are some possibly relevant environment variables (not changed recently):
SAGE_PARALLEL_SPKG_BUILD=yes SHELL=/bin/bash LD_LIBRARY_PATH=:/usr/local/lib:/home/jec/sage-current/local/lib
The reason for the LD_LIBRARY_PATH is that I use Sage's built libraries for other work (libntl and libpari); but seeting that to just /usr/local/lib made no difference anyway.
comment:90 follow-up: ↓ 91 Changed 8 years ago by
Same happens with just ./sage -t ...
or make test[long]
, i.e. non-parallel?
We had strange errors with ptest[long]
or ./sage -tp N ...
when SAGE_PATH
was set (even to the empty string).
comment:91 in reply to: ↑ 90 ; follow-up: ↓ 92 Changed 8 years ago by
Replying to leif:
Same happens with just
./sage -t ...
ormake test[long]
, i.e. non-parallel?We had strange errors with
ptest[long]
or./sage -tp N ...
whenSAGE_PATH
was set (even to the empty string).
That is not set:
jec@fermat%env | grep SAGE SAGE_PARALLEL_SPKG_BUILD=yes
make testlong was no better, neither is testing a single file (e.g. ./sage -t devel/sage/sage/rings/infinity.py). Sage does start up properly.
Looking at the actual error message (why not?) I see that it complains that the line
from sage.all_cmdline import *;
which is presumably included in the testing framework, causes sage.all_cmdline.py to be imported, and then in line 24 of that file the line 24:
raise ValueError, msg
raises the error, with the message coming from a few lines above where there is a try/except block containing
from sage.all import * from sage.calculus.predefined import x preparser(on=True)
and this is causing some kind of circular import.
Anyway, by 4.7 build which I have been using for weeks, and which passed all the tests when I built it, now gives rise to exactly the same problem. So this is nothing to do with any of the code on this ticket, but instead something weird that has happened with my Sage installations.
Here's an idea: I have several background jobs running sage; is it possible that this is the problem? They will also be writing to ~/.sage. I'll try again when they have finished.
comment:92 in reply to: ↑ 91 Changed 8 years ago by
Replying to cremona:
Here's an idea: I have several background jobs running sage; is it possible that this is the problem? They will also be writing to ~/.sage. I'll try again when they have finished.
Well, easily checked:
$ env DOT_SAGE=/tmp ./sage -t devel/sage/sage/rings/infinity.py
comment:93 follow-up: ↓ 94 Changed 8 years ago by
considering the amount of broken test you have, does sage even start properly without wanting to test it.
comment:94 in reply to: ↑ 93 ; follow-up: ↓ 96 Changed 8 years ago by
Replying to fbissey:
considering the amount of broken test you have, does sage even start properly without wanting to test it.
Yes, all the version I have start and run fine, but none of them cab run any tests. I have never seen this before, but it's something to do with my setup (and several sage jobs running in the background on the same machine) so we should probably not fill up more of this ticket with it -- as long as someone else can do some testing!
comment:95 Changed 8 years ago by
Build against current 4.7.2_alpha0 on linux amd64. No problem that can be attributed to pari after running the long tests.
comment:96 in reply to: ↑ 94 Changed 8 years ago by
Replying to cremona:
Replying to fbissey:
considering the amount of broken test you have, does sage even start properly without wanting to test it.
Yes, all the version I have start and run fine, but none of them cab run any tests. I have never seen this before, but it's something to do with my setup (and several sage jobs running in the background on the same machine) so we should probably not fill up more of this ticket with it -- as long as someone else can do some testing!
By deleting most of my ~/.sage I successfully ran all the tests (i.e. make ptestlong completed successfully). This is on 64-bit ubuntu.
Sorry for the noise -- is there any reason not to give this a positive review and get it into the 4.7.2 series as soon as possible?
comment:97 Changed 8 years ago by
- Keywords sd32 added
comment:98 Changed 8 years ago by
- Cc jpflori added
comment:99 Changed 8 years ago by
One doctest failure with sage-4.7.2.alpha2, fix included in new 11130_sagelib.patch:
sage -t -long -force_lib devel/sage/sage/rings/number_field/number_field_ideal.py ********************************************************************** File "/mnt/usb1/scratch/jdemeyer/merger/sage-4.7.2.alpha2-pari-2.5.0/devel/sage-main/sage/rings/number_field/number_field_ideal.py", line 1097: sage: [P._S_ideal_class_log(S) for P in K.primes_above(11)] Expected: [[5, 2], [1, 1]] Got: [[3, 2], [3, 1]] **********************************************************************
comment:100 Changed 8 years ago by
- Description modified (diff)
Positive review for trac_11130-doctest-poly.patch and 11130_sagelib32.patch. To make things simpler, I have folded these two patches into my 11130_sagelib.patch (in any case, the commit message of one of your patches needed to be fixed). Together with John's earlier review of my 11130_sagelib.patch, this one patch now has positive review.
Changed 8 years ago by
comment:101 Changed 8 years ago by
- Status changed from needs_review to needs_work
John, your patch changes the functionality of the simon_two_descent()
function by adding a new option store_gens
. I have two problems with this:
- I think a ticket related to upgrading PARI is not the correct place to make such a change (or at least, it should be justified why).
- I don't like how
store_gens
changes the output of the function. The name op this option does not imply that the output would be changed. I can agree that you store the saturated points (as before), but why also return the saturated points? This is a real functional change.
Changed 8 years ago by
comment:102 Changed 8 years ago by
- Description modified (diff)
- Status changed from needs_work to needs_review
comment:103 Changed 8 years ago by
I have a added a new patch 11130-sagelib-simon-v2.patch which has parts of John Cremona's patch (those parts get positive review from me) with some minor fixes added.
comment:104 Changed 8 years ago by
- Description modified (diff)
Changed 8 years ago by
comment:105 Changed 8 years ago by
- Description modified (diff)
comment:106 follow-up: ↓ 107 Changed 8 years ago by
Sorry for delay, I was on holiday.
The point about not adding new functionality on this specific ticket is a good one. It was so long ago I had to look at my own patch to see what you were talking about! I think I added that at someone's suggestion that after a successful 2-descent using simon_two_descent it was silly not to keep the point found. This will be requested again at some point, so perhaps a new ticket should be made for it before we forget,
But *nothing* should be done which delays this ticket finally getting closed!
comment:107 in reply to: ↑ 106 Changed 8 years ago by
Replying to cremona:
Sorry for delay, I was on holiday.
No apologies needed, I have also been away for a while...
John, it would be great if you could review the remaining patches here. They get positive_review from me, but since they contain a mixture of code of you and of me, you should also look at them.
comment:108 follow-up: ↓ 109 Changed 8 years ago by
- Status changed from needs_review to needs_work
Starting with a build of 4.7.2.alpha1 (the latest I have built) I applied both spkgs OK, then the first two patches ok, but the third patch 11130_reviewer32.patch had 2 failed hunks. It looks trivial, but as they are listed the patches do no apply cleanly.
comment:109 in reply to: ↑ 108 ; follow-up: ↓ 111 Changed 8 years ago by
- Dependencies changed from #11230, #11234, #11321 (install this '''after''' building PARI) to #11304, #11540, #11321 (install this '''after''' building PARI)
Replying to cremona:
Starting with a build of 4.7.2.alpha1 (the latest I have built) I applied both spkgs OK, then the first two patches ok, but the third patch 11130_reviewer32.patch had 2 failed hunks. It looks trivial, but as they are listed the patches do no apply cleanly.
The patches do apply to sage-4.7.2.alpha2. Alternatively, try applying the patch from #11540 (merged in sage-4.7.2.alpha2) first.
comment:110 Changed 8 years ago by
- Status changed from needs_work to needs_review
comment:111 in reply to: ↑ 109 Changed 8 years ago by
comment:112 follow-up: ↓ 113 Changed 8 years ago by
OK, so with a freshly built 4.7.2.alpha2, I successfully installed both new spkgs and all patches, and rebuilt Sage using -ba. Now doing a full test.
Question: in the extcode patch we refer to version 2.4.4 -- should that be 2.5.0?
I'll be ready with a positive review as soon as the testing is done (assuming success!).
comment:113 in reply to: ↑ 112 ; follow-up: ↓ 114 Changed 8 years ago by
Replying to cremona:
Question: in the extcode patch we refer to version 2.4.4 -- should that be 2.5.0?
The patch was created for PARI version 2.4.4 but by now there is version 2.5.0. So in fact both numbers are correct. Since 2.4.4 is the first affected version, I say we use that version number.
comment:114 in reply to: ↑ 113 Changed 8 years ago by
- Status changed from needs_review to positive_review
Replying to jdemeyer:
Replying to cremona:
Question: in the extcode patch we refer to version 2.4.4 -- should that be 2.5.0?
The patch was created for PARI version 2.4.4 but by now there is version 2.5.0. So in fact both numbers are correct. Since 2.4.4 is the first affected version, I say we use that version number.
OK -- and all tests pass (on a 64-bit machine) so I'll give this a positive review. I'll also do a 32-bit test tomorrow.
comment:115 Changed 8 years ago by
Just a remark: I tried this out on Solaris, and all seems well -- it builds fine and the selection of tests that I ran (including everything in sage/rings/number_field
,sage/libs/pari
and sage/interfaces/gp
) all worked.
comment:116 follow-up: ↓ 117 Changed 8 years ago by
Successfully built and applied everything on 32-bit linux, now testing.
comment:117 in reply to: ↑ 116 Changed 8 years ago by
Replying to cremona:
Successfully built and applied everything on 32-bit linux, now testing.
All tests pass (SuSE linux 32-bit).
comment:118 follow-up: ↓ 119 Changed 8 years ago by
I'm confused: is #11321 a prerequisite for this ticket, or not? I'm puzzled because these two tickets are listed as prerequisites for each other, and yet this one is at "positive review" and the other at "needs review". Is the proposal to close this ticket and merge the lcalc 1.23.p8 spkg from #11321, but not the two subsequent patches on that ticket, and the "needs review" there applies to the subsequent patches? Or are we going to wait for a p9 spkg at #11321, in which case we would probably need to test everything here over again?
comment:119 in reply to: ↑ 118 Changed 8 years ago by
Replying to davidloeffler:
I'm confused: is #11321 a prerequisite for this ticket, or not?
Yes, it is.
I'm puzzled because these two tickets are listed as prerequisites for each other.
That's correct. These two tickets need to be merged together, one will not work without the other. leif wants to change #11321 such that it does not depend on this ticket, but I think that's unnecessary.
Is the proposal to close this ticket and merge the lcalc 1.23.p8 spkg from #11321, but not the two subsequent patches on that ticket, and the "needs review" there applies to the subsequent patches?
needs_review on that ticket applies to everything. I created the .p8, leif raised some concerns about the .p8 and is proposing a .p9 but it doesn't materialize.
Or are we going to wait for a p9 spkg at #11321, in which case we would probably need to test everything here over again?
As far as I'm concerned, we can merge the .p8 as it is but it still needs_review. I don't believe leif's concerns are serious enough that they should prevent the spkg from being merged.
comment:120 Changed 8 years ago by
- Milestone changed from sage-4.7.2 to sage-4.7.3
comment:121 Changed 8 years ago by
- Status changed from positive_review to needs_work
- Work issues set to Test against sage-4.7.2.alpha3
comment:122 Changed 8 years ago by
- Status changed from needs_work to needs_review
comment:123 Changed 8 years ago by
- Description modified (diff)
comment:124 follow-up: ↓ 125 Changed 8 years ago by
Wouldn't it be more natural to change the output of the doctest in 11130-4.7.2.alpha3.patch instead of the input?
comment:125 in reply to: ↑ 124 Changed 8 years ago by
Replying to mstreng:
Wouldn't it be more natural to change the output of the doctest in 11130-4.7.2.alpha3.patch instead of the input?
I guess in this case, it does not really matter. I went for a minimal change.
With the test added, all short and long doctests pass on sage.math (x86_64 GNU/Linux).
comment:126 Changed 8 years ago by
- Description modified (diff)
New lcalc package at #11321 needs review.
comment:127 Changed 8 years ago by
- Work issues Test against sage-4.7.2.alpha3 deleted
comment:128 Changed 8 years ago by
- Status changed from needs_review to needs_work
comment:129 Changed 8 years ago by
- Status changed from needs_work to needs_review
New patch, tested on 32-bit x86 Linux, 64-bit x86_64 Linux, OS X 10.4 PPC.
comment:130 Changed 8 years ago by
- Description modified (diff)
I am currently testing this. To avoid duplication of effort, I'll fix things I find in the elliptic_curves directory only.