Opened 9 years ago

Last modified 8 years ago

#11130 closed defect

Update PARI to version 2.5.0 — at Version 137

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, William Stein
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: #11321 (install this after building PARI) Stopgaps:

Description (last modified by jdemeyer)

We need to add bugfixes for

  1. http://pari.math.u-bordeaux.fr/cgi-bin/bugreport.cgi?bug=1147 (related to #9334)
  2. http://pari.math.u-bordeaux.fr/cgi-bin/bugreport.cgi?bug=1153 (#10195)
  3. http://pari.math.u-bordeaux.fr/cgi-bin/bugreport.cgi?bug=1154 (#11604)
  4. http://pari.math.u-bordeaux.fr/cgi-bin/bugreport.cgi?bug=1184 (related to #9937)
  5. http://pari.math.u-bordeaux.fr/cgi-bin/bugreport.cgi?bug=1185 (PARI bug discovered by a Sage doctest)
  6. http://pari.math.u-bordeaux.fr/cgi-bin/bugreport.cgi?bug=1186 (PARI performance regression discovered by a Sage doctest)
  7. http://pari.math.u-bordeaux.fr/cgi-bin/bugreport.cgi?bug=1187 (PARI bug discovered by a Sage doctest)
  8. http://pari.math.u-bordeaux.fr/cgi-bin/bugreport.cgi?bug=1188 (PARI bug discovered by a Sage doctest)
  9. 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:

  1. spkg http://boxen.math.washington.edu/home/jdemeyer/spkg/pari-2.5.0.p2.spkg (jdemeyer)
  2. spkg http://boxen.math.washington.edu/home/jdemeyer/spkg/lcalc-1.23.p9.spkg (from #11321)
  3. 11130_sagelib.patch (jdemeyer, cremona)
  4. 11130-sagelib-simon-v2.patch (cremona, jdemeyer)
  5. 11130_reviewer32.patch (jdemeyer)
  6. 11130-4.7.2.alpha3.patch (jdemeyer)
  7. extcode patch 11130-extcode-v2.patch (cremona, jdemeyer)

Change History (147)

comment:1 Changed 9 years ago by jdemeyer

  • 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 jdemeyer

  • Description modified (diff)
  • Status changed from new to needs_work

comment:3 Changed 9 years ago by jdemeyer

  • Description modified (diff)

comment:4 Changed 9 years ago by mstreng

  • Cc mstreng added

comment:5 Changed 9 years ago by jdemeyer

  • Milestone changed from sage-4.7 to sage-4.7.1

comment:6 Changed 9 years ago by jdemeyer

  • Description modified (diff)

comment:7 Changed 9 years ago by jdemeyer

  • Description modified (diff)

comment:8 Changed 9 years ago by jdemeyer

  • Description modified (diff)

comment:9 Changed 9 years ago by jdemeyer

  • Description modified (diff)

comment:10 Changed 9 years ago by jdemeyer

  • Description modified (diff)

comment:11 Changed 8 years ago by jdemeyer

  • Description modified (diff)

comment:12 Changed 8 years ago by jdemeyer

  • Dependencies set to #11230
  • Description modified (diff)

comment:13 Changed 8 years ago by jdemeyer

  • Description modified (diff)

comment:14 Changed 8 years ago by jdemeyer

  • Description modified (diff)

comment:15 Changed 8 years ago by dimpase

  • Cc dimpase added

comment:16 Changed 8 years ago by jdemeyer

  • Description modified (diff)

comment:17 follow-up: Changed 8 years ago by cremona

I am currently testing this. To avoid duplication of effort, I'll fix things I find in the elliptic_curves directory only.

comment:18 follow-up: Changed 8 years ago by cremona

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: Changed 8 years ago by jdemeyer

Replying to cremona:

Did I do something wrong?

This ticket depends on #11230.

comment:20 in reply to: ↑ 19 Changed 8 years ago by cremona

Replying to jdemeyer:

Replying to cremona:

Did I do something wrong?

This ticket depends on #11230.

I see! I'll have some time to spend on this tomorrow (Monday) so I'll have another go.

comment:21 Changed 8 years ago by jdemeyer

  • Dependencies changed from #11230 to #11230, #11234
  • Description modified (diff)
  • Owner changed from tbd to jdemeyer

comment:22 in reply to: ↑ 17 Changed 8 years ago by jdemeyer

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.

Changed 8 years ago by cremona

Applies after previous

comment:23 Changed 8 years ago by cremona

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 8 years ago by cremona

  • 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: Changed 8 years ago by 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.

comment:26 in reply to: ↑ 25 Changed 8 years ago by cremona

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 8 years ago by jdemeyer

  • Description modified (diff)
  • Work issues set to Test on various systems

comment:28 Changed 8 years ago by jdemeyer

  • Description modified (diff)

comment:29 Changed 8 years ago by jdemeyer

  • Description modified (diff)

comment:30 Changed 8 years ago by jdemeyer

  • Work issues changed from Test on various systems to Test on various systems with #10247 applied

comment:31 Changed 8 years ago by jdemeyer

  • 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 8 years ago by jdemeyer

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 8 years ago by jdemeyer

  • Authors changed from Jeroen Demeyer to Jeroen Demeyer, John Cremona

comment:34 follow-up: Changed 8 years ago by cremona

Testing now on a 32-bit ubuntu system.

comment:35 in reply to: ↑ 34 Changed 8 years ago by cremona

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.

Changed 8 years ago by cremona

Apply after previous

comment:36 Changed 8 years ago by cremona

  • 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 8 years ago by jdemeyer

  • Description modified (diff)
  • Work issues Test on various systems with #10247 applied deleted

comment:38 Changed 8 years ago by jdemeyer

  • Description modified (diff)

comment:39 Changed 8 years ago by jdemeyer

  • 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 8 years ago by jdemeyer

  • Description modified (diff)

comment:41 follow-up: Changed 8 years ago by jdemeyer

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: Changed 8 years ago by cremona

Replying to jdemeyer:

lcalc fails to build with the new PARI:

Mike Rubenstein needs to be made aware of this.

comment:43 follow-up: Changed 8 years ago by fbissey

What about genus2reduction? I am assuming John already made sure eclib is working.

comment:44 in reply to: ↑ 43 Changed 8 years ago by cremona

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 8 years ago by jdemeyer

Replying to cremona:

Replying to jdemeyer:

lcalc fails to build with the new PARI:

Mike Rubenstein needs to be made aware of this.

First I will see if I can fix the error and then send the patches to him.

comment:46 Changed 8 years ago by jdemeyer

  • Dependencies changed from #11230, #11234 to #11230, #11234, #11321
  • Description modified (diff)

See #11321 for lcalc.

comment:47 Changed 8 years ago by jdemeyer

  • Dependencies changed from #11230, #11234, #11321 to #11230, #11234, #11321 (install this '''after''' building PARI)

comment:48 follow-up: Changed 8 years ago by jdemeyer

  • 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: Changed 8 years ago by cremona

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: Changed 8 years ago by jdemeyer

  • Description modified (diff)

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.

comment:51 in reply to: ↑ 50 Changed 8 years ago by cremona

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?

Changed 8 years ago by cremona

Apply to sage library after previous

Changed 8 years ago by cremona

Apply to extcode repository

comment:52 Changed 8 years ago by cremona

  • Description modified (diff)
  • Status changed from needs_work to needs_review

comment:53 Changed 8 years ago by jdemeyer

  • Description modified (diff)

comment:54 Changed 8 years ago by jdemeyer

  • Description modified (diff)
  • Work issues Fix simon_two_descent doctests deleted

comment:55 Changed 8 years ago by cremona

Is the only thing stopping this going forward the latest reviewer patch?

comment:56 follow-up: Changed 8 years ago by jdemeyer

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 8 years ago by cremona

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 8 years ago by jdemeyer

Also lcalc (#11321) needs review.

comment:59 Changed 8 years ago by kcrisman

The part of the ticket that fixes #10240 is okay, I think.

comment:60 Changed 8 years ago by jdemeyer

  • 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 8 years ago by kcrisman

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 8 years ago by leif

See #11605 for a PARI 2.4.3.alpha.p7 spkg with minor but important fixes to spkg-install (and some corrections in SPKG.txt). I've meanwhile based this on Dima's p6 (which just adds a small, harmless change for Cygwin) from #10240.

(Haven't yet looked at this ticket / spkg here at all.)

comment:63 Changed 8 years ago by leif

  • Description modified (diff)

comment:64 Changed 8 years ago by jdemeyer

  • Description modified (diff)

Needs to be rebased to sage-2.4.3.alpha7.spkg (#11605).

comment:65 Changed 8 years ago by leif

  • Work issues set to Rebase on PARI(!) 2.4.3.alpha.p7 spkg from #11605

comment:66 Changed 8 years ago by jdemeyer

  • Description modified (diff)

comment:67 Changed 8 years ago by jdemeyer

  • Description modified (diff)
  • Work issues Rebase on PARI(!) 2.4.3.alpha.p7 spkg from #11605 deleted

comment:68 Changed 8 years ago by jdemeyer

  • Description modified (diff)
  • Status changed from needs_work to needs_review

comment:69 Changed 8 years ago by jdemeyer

  • 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 8 years ago by jdemeyer

My comment refered to a Mac OS X 10.4 PPC machine, other machines might still work.

comment:71 Changed 8 years ago by jdemeyer

  • Description modified (diff)
  • Status changed from needs_work to needs_review

comment:72 Changed 8 years ago by jdemeyer

  • 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 jdemeyer

  • 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

Changed 8 years ago by jdemeyer

Diff for the pari spkg, for reviewing only

comment:74 Changed 8 years ago by jdemeyer

  • 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: Changed 8 years ago by cremona

  • 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: Changed 8 years ago by jdemeyer

  • 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 cremona

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: Changed 8 years ago by cremona

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: Changed 8 years ago by kcrisman

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: Changed 8 years ago by leif

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 somehowTM 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: Changed 8 years ago by 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.

comment:82 in reply to: ↑ 80 ; follow-up: Changed 8 years ago by kcrisman

Looks as if you're somehowTM 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.

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: Changed 8 years ago by fbissey

Replying to kcrisman:

Looks as if you're somehowTM 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.

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 fbissey

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 leif

Replying to fbissey:

Replying to kcrisman:

Looks as if you're somehowTM 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.

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 shouldTM 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: Changed 8 years ago by 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.

comment:87 in reply to: ↑ 86 ; follow-up: Changed 8 years ago by cremona

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: Changed 8 years ago by 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.

comment:89 in reply to: ↑ 88 Changed 8 years ago by cremona

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: Changed 8 years ago by leif

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: Changed 8 years ago by cremona

Replying to leif:

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).

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 leif

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: Changed 8 years ago by fbissey

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: Changed 8 years ago by 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!

comment:95 Changed 8 years ago by fbissey

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 cremona

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 was

  • Keywords sd32 added

comment:98 Changed 8 years ago by jpflori

  • Cc jpflori added

comment:99 Changed 8 years ago by jdemeyer

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 jdemeyer

  • 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 jdemeyer

comment:101 Changed 8 years ago by jdemeyer

  • 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:

  1. 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).
  2. 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 jdemeyer

cremona's patch without the store_gens option and with some minor fixes

Changed 8 years ago by jdemeyer

comment:102 Changed 8 years ago by jdemeyer

  • Description modified (diff)
  • Status changed from needs_work to needs_review

comment:103 Changed 8 years ago by jdemeyer

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 jdemeyer

  • Description modified (diff)

Changed 8 years ago by jdemeyer

comment:105 Changed 8 years ago by jdemeyer

  • Description modified (diff)

comment:106 follow-up: Changed 8 years ago by cremona

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 jdemeyer

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: Changed 8 years ago by cremona

  • 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: Changed 8 years ago by jdemeyer

  • 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 jdemeyer

  • Status changed from needs_work to needs_review

comment:111 in reply to: ↑ 109 Changed 8 years ago by cremona

Replying to jdemeyer:

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.

Sorry, I missed alpha2. I'll build that now and try again.

comment:112 follow-up: Changed 8 years ago by cremona

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: Changed 8 years ago by 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.

comment:114 in reply to: ↑ 113 Changed 8 years ago by cremona

  • 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 davidloeffler

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: Changed 8 years ago by cremona

Successfully built and applied everything on 32-bit linux, now testing.

comment:117 in reply to: ↑ 116 Changed 8 years ago by cremona

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: Changed 8 years ago by davidloeffler

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 jdemeyer

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 jdemeyer

  • Milestone changed from sage-4.7.2 to sage-4.7.3

comment:121 Changed 8 years ago by jdemeyer

  • 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 jdemeyer

  • Status changed from needs_work to needs_review

comment:123 Changed 8 years ago by jdemeyer

  • Description modified (diff)

comment:124 follow-up: Changed 8 years ago by 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?

comment:125 in reply to: ↑ 124 Changed 8 years ago by jdemeyer

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 jdemeyer

  • Description modified (diff)

New lcalc package at #11321 needs review.

comment:127 Changed 8 years ago by jdemeyer

  • Work issues Test against sage-4.7.2.alpha3 deleted

comment:128 Changed 8 years ago by jdemeyer

  • Status changed from needs_review to needs_work

Changed 8 years ago by jdemeyer

Fix doctest for sage-4.7.2.alpha3

comment:129 Changed 8 years ago by jdemeyer

  • 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 jdemeyer

  • Description modified (diff)

comment:131 Changed 8 years ago by jdemeyer

  • Description modified (diff)

comment:132 Changed 8 years ago by ddrake

I'm trying to apply all the spkgs and patches in this ticket so I can review #11948, but I can't build the Sage library. With everything applied, I get, after ./sage -b:

[...]
gcc -fno-strict-aliasing -g -O2 -DNDEBUG -g -O3 -Wall -Wstrict-prototypes -fPIC -I/home/drake/s/sage-4.7.2.rc0/local/include -I/home/drake/s/sage-4.7.2.rc0/local/include/csage -I/home/drake/s/sage-4.7.2.rc0/devel/sage/sage/ext -I/home/drake/s/sage-4.7.2.rc0/local/include/python2.6 -c sage/rings/real_mpfr.c -o build/temp.linux-x86_64-2.6/sage/rings/real_mpfr.o -w
error: command 'gcc' failed with exit status 1

If I just rerun ./sage -b (surely that will fix it, right? :), I get

gcc -fno-strict-aliasing -g -O2 -DNDEBUG -g -O3 -Wall -Wstrict-prototypes -fPIC -I/home/drake/s/sage-4.7.2.rc0/local/lib/python/site-packages/numpy/core/include -I/home/drake/s/sage-4.7.2.rc0/local/include -I/home/drake/s/sage-4.7.2.rc0/local/include/csage -I/home/drake/s/sage-4.7.2.rc0/devel/sage/sage/ext -I/home/drake/s/sage-4.7.2.rc0/local/include/python2.6 -c sage/rings/polynomial/real_roots.c -o build/temp.linux-x86_64-2.6/sage/rings/polynomial/real_roots.o -w
sage/libs/pari/gen.c: In function ‘__pyx_pf_4sage_4libs_4pari_3gen_3gen_81Ser’:
sage/libs/pari/gen.c:12094: error: too many arguments to function ‘gtoser’
error: command 'gcc' failed with exit status 1

This is with 4.7.2.rc0 on amd64 Ubuntu 10.04.

comment:133 follow-up: Changed 8 years ago by jdemeyer

Sorry to ask the obvious, but can you double-check that you applied the new PARI spkg and all patches here?

comment:134 in reply to: ↑ 133 Changed 8 years ago by ddrake

Replying to jdemeyer:

Sorry to ask the obvious, but can you double-check that you applied the new PARI spkg and all patches here?

No need to apologize...I was certain I did everything right, but I just tried again, and Sage built and is well on its way to passing all "ptestlong" tests. So I must have made a mistake above; sorry for the noise.

comment:135 Changed 8 years ago by was

  • Status changed from needs_review to positive_review

The new patch works for me (for the 64-bit test). And I was definitely able to apply the patches and pari spkg here and get a working Sage.

comment:136 Changed 8 years ago by jdemeyer

  • Dependencies changed from #11304, #11540, #11321 (install this '''after''' building PARI) to #11321 (install this '''after''' building PARI)
  • Description modified (diff)
  • Reviewers changed from John Cremona, Jeroen Demeyer to John Cremona, Jeroen Demeyer, William Stein

comment:137 Changed 8 years ago by jdemeyer

  • Description modified (diff)
Note: See TracTickets for help on using tickets.