Opened 9 years ago
Closed 8 years ago
#15767 closed enhancement (fixed)
Upgrade PARI to 2.7.1
Reported by:  Jeroen Demeyer  Owned by:  

Priority:  major  Milestone:  sage6.4 
Component:  packages: standard  Keywords:  
Cc:  JeanPierre Flori, Peter Bruin, Julien Puydt  Merged in:  
Authors:  Jeroen Demeyer  Reviewers:  Peter Bruin 
Report Upstream:  N/A  Work issues:  
Branch:  bf435f8 (Commits, GitHub, GitLab)  Commit:  bf435f8db1537f63c001894fb4949879617b71f2 
Dependencies:  #15888, #15855  Stopgaps: 
Description (last modified by )
Upstream: http://pari.math.ubordeaux.fr/pub/pari/unix/pari2.7.1.tar.gz
Updated (changes have always been made directly in the source tarball and the package will be removed anyway in #15808): http://boxen.math.washington.edu/home/jdemeyer/spkg/genus2reduction20140211.tar.bz2
Attachments (1)
Change History (137)
comment:1 Changed 9 years ago by
comment:2 Changed 9 years ago by
Milestone:  sage6.1 → sage6.2 

comment:3 Changed 9 years ago by
Dependencies:  #15765 → #15765, #15774 

comment:4 Changed 9 years ago by
Branch:  → u/jdemeyer/ticket/15767 

Created:  Jan 30, 2014, 4:27:35 PM → Jan 30, 2014, 4:27:35 PM 
Modified:  Feb 1, 2014, 8:55:56 PM → Feb 1, 2014, 8:55:56 PM 
comment:5 Changed 9 years ago by
Authors:  → Jeroen Demeyer 

Commit:  → 0595a422d7c5a61858859a47c4ccc8313b37915d 
Description:  modified (diff) 
New commits:
17f864c  Remove mp.c.patch; instead call init_memory_functions()

ad63daf  Move PARI databases out of PARI package

1678b89  Merge remotetracking branch 'origin/develop' into ticket/15765

bfdad13  Merge branch 'u/jdemeyer/ticket/15765' of git://trac.sagemath.org/sage into ticket/15774

0595a42  Upgrade to PARI 2.6.2

comment:6 Changed 9 years ago by
Commit:  0595a422d7c5a61858859a47c4ccc8313b37915d → 6de648ba85ef7315acdf5a3c948c768e34d9606e 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
6de648b  Upgrade to PARI 2.6.2

comment:7 Changed 9 years ago by
Commit:  6de648ba85ef7315acdf5a3c948c768e34d9606e → 5b823f1ac56e6b3e4815c3002a35f33912ad0876 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
5b823f1  Upgrade to PARI 2.6.2

comment:8 Changed 9 years ago by
Commit:  5b823f1ac56e6b3e4815c3002a35f33912ad0876 → 5cb347872a7e13be6205e818eef966d58e657f0e 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
5cb3478  Upgrade to PARI 2.6.2

comment:9 Changed 9 years ago by
Commit:  5cb347872a7e13be6205e818eef966d58e657f0e → 84c081c13ce3833ccb3fbe55ab87eee51e718aee 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
84c081c  Upgrade to PARI 2.6.2

comment:10 Changed 9 years ago by
Commit:  84c081c13ce3833ccb3fbe55ab87eee51e718aee → 8e2cc9ed86809921f30ffca0975eb7b762bda36c 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
8e2cc9e  Upgrade to PARI 2.6.2

comment:11 Changed 9 years ago by
This version compiles fine, but there are about a million doctest failures :(
comment:12 Changed 9 years ago by
Commit:  8e2cc9ed86809921f30ffca0975eb7b762bda36c → 19b29364c702ca639c22fc81d2135a5b209d56ce 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
19b2936  Upgrade to PARI 2.6.2

comment:13 Changed 9 years ago by
This is a serious regression:
sage: EllipticCurve('90c3').integral_points(both_signs=False) [(15 : 7 : 1), (1 : 39 : 1), (21 : 79 : 1)]
comment:14 Changed 9 years ago by
Commit:  19b29364c702ca639c22fc81d2135a5b209d56ce → 52e24d0e5355d83158129b7c77acf8beb5b77649 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
52e24d0  Upgrade to PARI 2.6.2

comment:15 Changed 9 years ago by
Commit:  52e24d0e5355d83158129b7c77acf8beb5b77649 → b7809162a02febeddfacc0c9eeffc1c69c3b36cc 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
b780916  Upgrade to PARI 2.6.2

comment:16 Changed 9 years ago by
Description:  modified (diff) 

comment:17 Changed 9 years ago by
Commit:  b7809162a02febeddfacc0c9eeffc1c69c3b36cc → 9cb3ae74ad54fe44916a4ce4f4cb29857336db3c 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
9cb3ae7  Upgrade to PARI 2.6.2

comment:18 Changed 9 years ago by
Commit:  9cb3ae74ad54fe44916a4ce4f4cb29857336db3c → 7ee964c800987b343a0578b2a60fbd3400692d2a 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
7ee964c  Upgrade to PARI 2.6.2

comment:19 Changed 9 years ago by
Commit:  7ee964c800987b343a0578b2a60fbd3400692d2a → 73f51cdf7c9137b4e64f74e84f42e6d2abc11681 

comment:20 Changed 9 years ago by
Description:  modified (diff) 

comment:21 Changed 9 years ago by
Commit:  73f51cdf7c9137b4e64f74e84f42e6d2abc11681 → 6091d59de30240271265c6efc02bccecb4a3e81e 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
6091d59  Upgrade to PARI 2.7

comment:22 Changed 9 years ago by
Description:  modified (diff) 

comment:23 followup: 24 Changed 9 years ago by
I know this isn't ready yet; I was just browsing the branch to see how many changes need to be made. Are you sure the following is correct? I think the purpose of the ellinit()
call is to allow the user to specify the curve in short Weierstrass form (ellinit([a, b]) > y^2 = x^3 + a*x + b
).
@@ 1903,7 +1903,7 @@ bnfellrank(bnf,ell,help=[],bigflag=1,flag3=1) = local(urst,urst1,den,factden,eqtheta,rnfeq,bbnf,ext,rang); if( DEBUGLEVEL_ell >= 3, print("entree dans bnfellrank"));  if( #ell <= 5, ell = ellinit(ell,1)); + ell = vector(5, i, ell[i]); \\ removes the coefficients a1 and a3 urst = [1,0,0,0];
comment:24 Changed 9 years ago by
Replying to pbruin:
I know this isn't ready yet; I was just browsing the branch to see how many changes need to be made. Are you sure the following is correct? I think the purpose of the
ellinit()
call is to allow the user to specify the curve in short Weierstrass form (ellinit([a, b]) > y^2 = x^3 + a*x + b
).@@ 1903,7 +1903,7 @@ bnfellrank(bnf,ell,help=[],bigflag=1,flag3=1) = local(urst,urst1,den,factden,eqtheta,rnfeq,bbnf,ext,rang); if( DEBUGLEVEL_ell >= 3, print("entree dans bnfellrank"));  if( #ell <= 5, ell = ellinit(ell,1)); + ell = vector(5, i, ell[i]); \\ removes the coefficients a1 and a3 urst = [1,0,0,0];
OK, I see what you mean. We could change it to
ell = ellinit(ell); ell = vector(5, i, ell[i]);
comment:25 followup: 26 Changed 9 years ago by
We could also try to merge #11005 first, maybe that helps.
comment:26 Changed 9 years ago by
comment:27 Changed 9 years ago by
Commit:  6091d59de30240271265c6efc02bccecb4a3e81e → 01c76a3c6ecffb07a71af2e7b9aed5f4660a18bd 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
01c76a3  Upgrade to PARI 2.7

comment:28 Changed 9 years ago by
Commit:  01c76a3c6ecffb07a71af2e7b9aed5f4660a18bd → 1db424ecd1304107bbe5e59406129bd4e16371fa 

comment:29 Changed 9 years ago by
Commit:  1db424ecd1304107bbe5e59406129bd4e16371fa → 802a31ffc2955036e55c741326d5759b556ce595 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
802a31f  Upgrade to PARI 2.7

comment:30 Changed 9 years ago by
Commit:  802a31ffc2955036e55c741326d5759b556ce595 → 2964947e7572ca34850aa80e9aca7c15a0850452 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
2964947  Upgrade to PARI 2.7

comment:31 Changed 9 years ago by
Commit:  2964947e7572ca34850aa80e9aca7c15a0850452 → 9cc4af75e4d6663c3b2eeec3a81371d595ff0a7a 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
9cc4af7  Upgrade to PARI 2.7

comment:32 Changed 9 years ago by
Commit:  9cc4af75e4d6663c3b2eeec3a81371d595ff0a7a → 1b06004413d6a23af1f0836f595f8582d0386d81 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
1b06004  Upgrade to PARI 2.7

comment:33 Changed 9 years ago by
Commit:  1b06004413d6a23af1f0836f595f8582d0386d81 → 14a8548b3986a089f43400558d068fed1322996a 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
14a8548  Upgrade to PARI 2.7

comment:34 Changed 9 years ago by
Commit:  14a8548b3986a089f43400558d068fed1322996a → f36fba5234615ce973bd168b0b7202a9475ab65e 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
f36fba5  Upgrade to PARI 2.7

comment:35 Changed 9 years ago by
Commit:  f36fba5234615ce973bd168b0b7202a9475ab65e → 834368dd71b707cc3eaf50d3119095f13c5252b3 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
834368d  Upgrade to PARI 2.7

comment:36 Changed 9 years ago by
Commit:  834368dd71b707cc3eaf50d3119095f13c5252b3 → 4829125af240ca57dde85ee4439fd23b79dfacd1 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
4829125  Upgrade to PARI 2.7

comment:37 Changed 9 years ago by
Commit:  4829125af240ca57dde85ee4439fd23b79dfacd1 → 4f040c87385fc17d43fd2350f6a4214769923601 

Branch pushed to git repo; I updated commit sha1. New commits:
4f040c8  Merge remotetracking branch 'origin/develop' into ticket/15767

comment:38 Changed 9 years ago by
Commit:  4f040c87385fc17d43fd2350f6a4214769923601 → 27897d3b9b7ac21032cfc2eee2d85f17a6fd48d2 

Branch pushed to git repo; I updated commit sha1. New commits:
27897d3  PARI 2.7 doctest fixes

comment:39 Changed 9 years ago by
Dependencies:  #15765, #15774 → #15855, #11005, #15483 

comment:40 Changed 9 years ago by
Commit:  27897d3b9b7ac21032cfc2eee2d85f17a6fd48d2 → ac97322e0aa2a0a772552a490ab73feedc351e4e 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
ac97322  PARI 2.7 doctest fixes

comment:41 Changed 9 years ago by
Dependencies:  #15855, #11005, #15483 

comment:42 Changed 9 years ago by
Commit:  ac97322e0aa2a0a772552a490ab73feedc351e4e → c80cf8bc39c434523abaa5ad3401932327081798 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
c80cf8b  Upgrade to PARI 2.7

comment:43 Changed 9 years ago by
Dependencies:  → #11005, #15483, #15855 

Modified:  Mar 4, 2014, 9:00:11 AM → Mar 4, 2014, 9:00:11 AM 
comment:44 Changed 9 years ago by
Dependencies:  #11005, #15483, #15855 

comment:45 Changed 9 years ago by
Dependencies:  → #11005, #15483, #15855 

Modified:  Mar 4, 2014, 9:02:23 AM → Mar 4, 2014, 9:02:23 AM 
comment:46 Changed 9 years ago by
Dependencies:  #11005, #15483, #15855 

comment:47 Changed 9 years ago by
Commit:  c80cf8bc39c434523abaa5ad3401932327081798 → 94d14a2e6b0139d77f9f0d46de05151ad2779fde 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
94d14a2  Upgrade to PARI 2.7

comment:48 Changed 9 years ago by
Commit:  94d14a2e6b0139d77f9f0d46de05151ad2779fde → d08c8fcff4d8eb2a7b6c1209c73f0226223e9b77 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
d08c8fc  Upgrade to PARI 2.7

comment:49 Changed 9 years ago by
Commit:  d08c8fcff4d8eb2a7b6c1209c73f0226223e9b77 → 44c0f87c3c896586459b538c2b383b26ae325654 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
44c0f87  Upgrade to PARI 2.7

comment:50 Changed 9 years ago by
Dependencies:  → #15888 

comment:51 Changed 9 years ago by
Commit:  44c0f87c3c896586459b538c2b383b26ae325654 → bedb19ad7fb799b387a2e07f69a76966488f6ec9 

comment:52 Changed 9 years ago by
Commit:  bedb19ad7fb799b387a2e07f69a76966488f6ec9 → 6a5539f9b3941feb9fd4d9ddb807f3cde82e8e38 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
0e4fff5  Clean up weierstrass_p function

666714e  Fix changelog entry for ell_wp

d6fdd0c  CC>PARI: convert purely imaginary numbers to t_COMPLEX with real part exactly 0

2af767a  Merge branches 'ticket/15855' and 'ticket/15888' into ticket/15767

6a5539f  Upgrade to PARI 2.7

comment:53 Changed 9 years ago by
Dependencies:  #15888 → #15888, #15855 

comment:54 Changed 9 years ago by
Commit:  6a5539f9b3941feb9fd4d9ddb807f3cde82e8e38 → 1a12d09f53d9a003b8e319d83d7976970c3d3c0e 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
1a12d09  Upgrade to PARI 2.7

comment:55 Changed 9 years ago by
Commit:  1a12d09f53d9a003b8e319d83d7976970c3d3c0e → 5467494c1f852e503c36de8379dbff3616460fd4 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
5467494  Upgrade to PARI 2.7

comment:56 Changed 9 years ago by
Commit:  5467494c1f852e503c36de8379dbff3616460fd4 → 3970e579c3d74fba6f0dc39745425bc3649ad819 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
3970e57  Upgrade to PARI 2.7

comment:57 Changed 9 years ago by
Description:  modified (diff) 

comment:58 Changed 9 years ago by
Commit:  3970e579c3d74fba6f0dc39745425bc3649ad819 → ae1ae57d04ca1b9ba1acb789ed1919eda1d01a5e 

comment:59 Changed 9 years ago by
Description:  modified (diff) 

comment:60 followup: 62 Changed 9 years ago by
Hi Jeroen,
What's the status of this? Are there still hundreds of failing stuff? With the forced pushes its not that easy to see how the situtation evolves. And I've not tested it yet.
Whatsoever, thanks for the hard work.
comment:61 Changed 9 years ago by
Commit:  ae1ae57d04ca1b9ba1acb789ed1919eda1d01a5e → f6a149b845d40c84ef63a45f71af4c8cf5abe356 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
f6a149b  Upgrade to PARI 2.7

comment:62 Changed 9 years ago by
Replying to jpflori:
Hi Jeroen,
What's the status of this? Are there still hundreds of failing stuff? With the forced pushes its not that easy to see how the situtation evolves. And I've not tested it yet.
It's almost ready. The announcement of the PARI2.7 release candidate gave me a good reason to finish this ticket. Sage doctests found many (10 or so) bugs in PARI, all of which have been fixed upstream.
The forced pushes are the most convenient way to develop this, otherwise I would have 100 commits and several merges in between. I will now look into the remaining doctest failures.
comment:63 Changed 9 years ago by
Sure, I wasn't criticizing your way of doing things. Just get a status report if possible at this point and know if it's worth having a look at it and potentially give you a hand.
comment:64 Changed 9 years ago by
Commit:  f6a149b845d40c84ef63a45f71af4c8cf5abe356 → a6e2f9d188ae4e941feb83fb6b1a6daa5611a50d 

comment:65 Changed 9 years ago by
This passes 64bit doctests for me. Now building a 32bit version to check that...
comment:66 Changed 9 years ago by
Description:  modified (diff) 

comment:67 Changed 9 years ago by
Commit:  a6e2f9d188ae4e941feb83fb6b1a6daa5611a50d → b321ea39b0751f803f48162ae222630107e426a1 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
b321ea3  Upgrade to PARI 2.7

comment:68 followup: 69 Changed 9 years ago by
Hi,
I didn't see any changes in spkginstall in the last commit. Is there any? I am asking because in #16017 we noticed that pari doesn't have the right install_name for OS X. It turns out http://trac.sagemath.org/ticket/16017#comment:43 that it is caused by cruft in spkginstall. I don't think it has any value and I am wondering if you could remove it as a part of this ticket.
comment:69 Changed 9 years ago by
comment:70 Changed 9 years ago by
Let me also give a general status of this ticket: the single one remaining issue is the following doctest failure on 32bit:
sage t long src/doc/en/reference/coercion/index.rst ********************************************************************** File "src/doc/en/reference/coercion/index.rst", line 65, in doc.en.reference.coercion.index Failed example: f = EllipticCurve('37a').lseries().taylor_series(10); f Expected: 0.997997869801216 + 0.00140712894524925*z  0.000498127610960097*z^2 + 0.000118835596665956*z^3  0.0000215906522442707*z^4 + (3.20363155418419e6)*z^5 + O(z^6) Got: 0.0000405744363329341 + 2.21638825958869*z  2.40591010134898*z^2 + 1.70010490236379*z^3  0.876833052881789*z^4 + 0.350599228549426*z^5 + O(z^6) **********************************************************************
The problem is that the result is totally wrong.
I have been busy with other things lately, so I haven't looked into this issue.
comment:71 Changed 9 years ago by
Cc:  Julien Puydt added 

comment:72 Changed 9 years ago by
Commit:  b321ea39b0751f803f48162ae222630107e426a1 → ee9c845e18e6d229b7a83f8b365e4e24e0fbc1f9 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
ee9c845  Upgrade to PARI 2.7

comment:73 Changed 9 years ago by
Description:  modified (diff) 

comment:74 Changed 9 years ago by
Commit:  ee9c845e18e6d229b7a83f8b365e4e24e0fbc1f9 → 1d9305b0ca69ceb46edf1623690092fa1bea3f03 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
1d9305b  Upgrade to PARI 2.7

comment:75 Changed 9 years ago by
Commit:  1d9305b0ca69ceb46edf1623690092fa1bea3f03 → 082bcebe1f0473e5c551268d763229bd50861070 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
082bceb  Upgrade to PARI 2.7

comment:76 Changed 9 years ago by
Commit:  082bcebe1f0473e5c551268d763229bd50861070 → fd2bb156f8d61533c93159388b220747256786c2 

Branch pushed to git repo; I updated commit sha1. New commits:
fd2bb15  Merge remotetracking branch 'origin/develop' into ticket/15767

comment:77 Changed 9 years ago by
Any progress on the remaining doctest failure? I don't have access to a 32bit machine, otherwise I could try to look into it.
comment:78 Changed 9 years ago by
Description:  modified (diff) 

Summary:  Upgrade PARI to 2.7 → Upgrade PARI to 2.7.1 
comment:79 Changed 9 years ago by
Commit:  fd2bb156f8d61533c93159388b220747256786c2 → 2821016720f8c67f43ed8e9762dafebbd398dfe3 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
2821016  Upgrade to PARI2.7.1

comment:80 Changed 9 years ago by
Commit:  2821016720f8c67f43ed8e9762dafebbd398dfe3 → 1b3812333dae05081ca57d9df6094e4eb519b674 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
1b38123  Upgrade to PARI2.7.1

comment:81 Changed 9 years ago by
Description:  modified (diff) 

comment:82 Changed 9 years ago by
Commit:  1b3812333dae05081ca57d9df6094e4eb519b674 → f26a0a0136b0067e12b05c207e0939ba5654a4ee 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
f26a0a0  Upgrade to PARI2.7.1

comment:83 Changed 9 years ago by
Milestone:  sage6.2 → sage6.3 

comment:84 Changed 9 years ago by
Commit:  f26a0a0136b0067e12b05c207e0939ba5654a4ee → 5c076d69ceeca9c9aced70cbeba52f87dcb15b68 

Branch pushed to git repo; I updated commit sha1. New commits:
5c076d6  Upgrade to pari2.7.1pre2

comment:85 Changed 9 years ago by
Description:  modified (diff) 

New commits:
5c076d6  Upgrade to pari2.7.1pre2

comment:88 Changed 8 years ago by
Good news, I found one of my boxes is able to reproduce the problem of comment 70. Now I need to dig what happens... first step would be trying to see if I can reproduce it in gp, because at that point that could also be an upstream issue.
comment:89 Changed 8 years ago by
Using ~/.sage/dokchitser.log, I was able to check the computation against debian's parigp package, and find there is the same discrepancy between amd64 and i386, so I reported a problem in debian.
comment:90 Changed 8 years ago by
I can reproduce this inside GP 2.7.1 (patched version of this ticket) on ARM using the following commands (also adapted from dokchitser.log
):
read("local/share/sage/ext/pari/dokchitser/computel.gp"); default(realprecision, 19); conductor = 37; gammaV = [0, 1]; weight = 2; sgn = 1; Lpoles = []; Lresidues = automatic; e = ellinit([0, 0, 1, 1, 0]); a(k) = ellak(e, k); initLdata("a(k)", 1.0); v = Vec(Lseries(10.0, , 5)); print(v)
One gets the correct answer after replacing 19 by 20 in the second line, presumably because this increases the precision by one word.
comment:91 Changed 8 years ago by
According to git bisect
, the bug is triggered by the following upstream change (PARI Git commit 8cfb9c6b, incorrect rounding in mulur):

src/kernel/none/mp_indep.c
diff git a/src/kernel/none/mp_indep.c b/src/kernel/none/mp_indep.c index 522e8a6..63f3311 100644
a b mulur_2(ulong x, GEN y, long s) 143 143 y; garde = mulll(x,y[lx]); 144 144 for (i=lx1; i>=3; i) z[i]=addmul(x,y[i]); 145 145 z[2]=hiremainder; /* != 0 since y normalized and x > 1 */ 146 147 146 sh = bfffo(hiremainder); m = BITS_IN_LONGsh; 148 147 if (sh) shift_left(z,z, 2,lx1, garde,sh); 149 z[1] = evalsigne(s)  evalexpo(m+e); return z; 148 z[1] = evalsigne(s)  evalexpo(m+e); 149 if ((garde << sh) & HIGHBIT) roundr_up_ip(z, lx); 150 return z; 150 151 } 151 152 152 153 INLINE GEN
comment:92 Changed 8 years ago by
It seems to me that the function SeriesToContFrac
in Dokchitser's computel
program is affected by some numerical instability, so that the above rounding fix in PARI leads to wildly different output of this function for the same input. It is probably possible to solve (or at least diminish) the problem by increasing the variable asympdigits
in the script.
comment:93 Changed 8 years ago by
Indeed that's what Bill Allombert answered to my bug report in debian: the pari code is correct, and the computeL code looks like it uses catastrophic cancellation... hence it should really shoot for more precision. Either by increasing asymdigits at Peter suggests, or (notice that the following only solves that one example) change the line in dokchitser.py which sets the realprecision to prec//3+2
to prec//3+3
.
comment:94 Changed 8 years ago by
This seems to be a minimal fix:

src/ext/pari/dokchitser/computel.gp
diff git a/src/ext/pari/dokchitser/computel.gp b/src/ext/pari/dokchitser/computel.gp index 9768874..3494fc9 100644
a b SeriesToContFrac(vec, 297 297 while (1, 298 298 res=concat(res,[vec[1]]); 299 299 ind=0; 300 until(ind==length(vec)  abs(vec[ind+1])>10^ asympdigits,ind++;vec[ind]=0);300 until(ind==length(vec)  abs(vec[ind+1])>10^(asympdigits1),ind++;vec[ind]=0); 301 301 if(ind>=length(vec),break); 302 302 res=concat(res,[ind]); 303 303 vec=Vec(x^ind/Ser(vec));
There are also two doctest failures in elliptic_curves/ell_number_field.py
and elliptic_curves/height.py
(at least on 32bit ARM), which appear to be related to simon_two_descent
.
comment:95 Changed 8 years ago by
Description:  modified (diff) 

comment:96 Changed 8 years ago by
Commit:  5c076d69ceeca9c9aced70cbeba52f87dcb15b68 → 1f8eb3846b5af85192848180d019486b640e57c6 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
1f8eb38  Upgrade to PARI2.7.1

comment:97 Changed 8 years ago by
Commit:  1f8eb3846b5af85192848180d019486b640e57c6 → 59a50645de5bc5e486bfd06917313f574754be90 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
59a5064  Upgrade to PARI2.7.1

comment:98 followup: 101 Changed 8 years ago by
We have a new regression (in a doctest added by #8828):
sage: K.<a> = NumberField(x^32) sage: E = EllipticCurve([0,0,0,0,a]) sage: E.gens() []
This used to return
sage: K.<a> = NumberField(x^32) sage: E = EllipticCurve([0,0,0,0,a]) sage: E.gens() [(1/3*a^2 + a + 5/3 : 2*a^2  4/3*a  5/3 : 1)]
comment:99 Changed 8 years ago by
Commit:  59a50645de5bc5e486bfd06917313f574754be90 → 06dbe23773fb47f0a98687dfef143cab2bca4b6e 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
06dbe23  Upgrade to PARI2.7.1

comment:100 Changed 8 years ago by
Commit:  06dbe23773fb47f0a98687dfef143cab2bca4b6e → e5bdf4ccc2bab46b98c096e3cbb15dd0f942623e 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
e5bdf4c  Upgrade to PARI2.7.1

comment:101 followups: 102 112 Changed 8 years ago by
Replying to jdemeyer:
We have a new regression (in a doctest added by #8828):
sage: K.<a> = NumberField(x^32) sage: E = EllipticCurve([0,0,0,0,a]) sage: E.gens() []This used to return
sage: K.<a> = NumberField(x^32) sage: E = EllipticCurve([0,0,0,0,a]) sage: E.gens() [(1/3*a^2 + a + 5/3 : 2*a^2  4/3*a  5/3 : 1)]
The parameter lim1
for Simon's 2descent program needs to be adjusted (default is 2, minimal value for which a point is found is now apparently 5):

src/sage/schemes/elliptic_curves/height.py
diff git a/src/sage/schemes/elliptic_curves/height.py b/src/sage/schemes/elliptic_curves/height.py index 8d0ef41..6917594 100644
a b class EllipticCurveCanonicalHeight: 1800 1800 This curve does have a point of good reduction whose canonical 1801 1801 point is approximately 1.68:: 1802 1802 1803 sage: P = E.gens( )[0]1803 sage: P = E.gens(lim1=5)[0] 1804 1804 sage: P.height() 1805 1805 1.68038085233673 1806 1806 sage: P.has_good_reduction()
comment:102 Changed 8 years ago by
Replying to pbruin:
The parameter
lim1
for Simon's 2descent program needs to be adjusted (default is 2, minimal value for which a point is found is now apparently 5):
Actually the same can be done much more efficiently using lim3
instead (with lim1
it needlessly increases the search bound for quartics that are not everywhere locally soluble):

src/sage/schemes/elliptic_curves/height.py
diff git a/src/sage/schemes/elliptic_curves/height.py b/src/sage/schemes/elliptic_curves/height.py index 8d0ef41..6917594 100644
a b class EllipticCurveCanonicalHeight: 1800 1800 This curve does have a point of good reduction whose canonical 1801 1801 point is approximately 1.68:: 1802 1802 1803 sage: P = E.gens( )[0]1803 sage: P = E.gens(lim3=5)[0] 1804 1804 sage: P.height() 1805 1805 1.68038085233673 1806 1806 sage: P.has_good_reduction()
comment:103 Changed 8 years ago by
The other remaining failure on 32bit (see comment:94) is in the following doctest (in ell_number_field.py
, introduced in #15483):
sage: K.<s> = QuadraticField(229) sage: c4 = 2173  235*(1  s)/2 sage: c6 = 124369 + 15988*(1  s)/2 sage: E = EllipticCurve([c4/48, c6/864]) sage: E.simon_two_descent() (0, 0, [])
This returns (1, 1, [])
on 32bit because a certain quartic is incorrectly determined to be locally soluble at infinity. It appears to be caused by polsturm
being called on a polynomial whose precision is too low. This bug is similar to but different than the one in #15483, and seems somewhat harder to fix.
comment:104 Changed 8 years ago by
It seems the above problem can be solved by computing the exact inequalities appearing in Sturm's method and checking them using the function nfrealsign()
from the script. I am uploading a patch that I also sent to Denis Simon.
Changed 8 years ago by
Attachment:  ell.gp.patch added 

Trac 15767: more exact Archimedean computations in ell.gp
comment:106 Changed 8 years ago by
Yes indeed, note that I downloaded it a few days ago without problem.
comment:107 followup: 108 Changed 8 years ago by
I have been away for 3 weeks without touching anything Sagerelated, so I'm just coming back to this now.
Strange that the boxen link doesn't work anymore, even this gives a 404: http://boxen.math.washington.edu/home/jdemeyer (luckily, the directory on boxen still exists). Does anybody have a clue what could have caused this? If no, I should ask on sagemathadmins.
comment:108 Changed 8 years ago by
Replying to jdemeyer:
Strange that the boxen link doesn't work anymore, even this gives a 404: http://boxen.math.washington.edu/home/jdemeyer (luckily, the directory on boxen still exists). Does anybody have a clue what could have caused this?
This could be relevant: https://groups.google.com/forum/#!topic/sagedevel/dhON2NJoSoc
comment:109 Changed 8 years ago by
Milestone:  sage6.3 → sage6.4 

comment:110 Changed 8 years ago by
The issue with the links is fixed, so perhaps it's time to review this (e.g. #16165 depends on this).
comment:112 followup: 115 Changed 8 years ago by
Replying to pbruin:
The parameter
lim1
for Simon's 2descent program needs to be adjusted (default is 2, minimal value for which a point is found is now apparently 5):
Do you happen to know the deeper reason why this needs to be changed?
I'll make the change in the doctest.
comment:114 Changed 8 years ago by
Commit:  e5bdf4ccc2bab46b98c096e3cbb15dd0f942623e → 5605a97a074c889f410f59a264f9a7a33aa4934b 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
5605a97  Upgrade to PARI2.7.1

comment:115 Changed 8 years ago by
Replying to jdemeyer:
Replying to pbruin:
The parameter
lim1
for Simon's 2descent program needs to be adjusted (default is 2, minimal value for which a point is found is now apparently 5):Do you happen to know the deeper reason why this needs to be changed?
No, I only found this experimentally. I would guess some PARI number field function returns a more complicated or "unlucky" result than before in this example.
Did you get any answers from Denis Simon yet?
So far I didn't.
comment:116 Changed 8 years ago by
Commit:  5605a97a074c889f410f59a264f9a7a33aa4934b → 37fc8e88689a0197c5513161b85b8bbadd5630ab 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
37fc8e8  Upgrade to PARI2.7.1

comment:117 Changed 8 years ago by
I rebased this ticket and added the fixes by Peter Bruin. It now again passes doctests on 64bit.
comment:120 Changed 8 years ago by
Branch:  u/jdemeyer/ticket/15767 → u/pbruin/15767pari2.7.1 

Commit:  37fc8e88689a0197c5513161b85b8bbadd5630ab → 5db54b63a2e694423f33bd50819e46157be636db 
Reviewers:  → Peter Bruin 
And all the developers rejoiced!
If you agree with the (small) changes in the reviewer patch, this ticket is good to go as far as I'm concerned.
comment:121 followups: 122 123 Changed 8 years ago by
Status:  needs_review → needs_work 

Fails on OpenSuSE 12.3:
sage t long src/sage/libs/pari/gen.pyx ********************************************************************** File "src/sage/libs/pari/gen.pyx", line 2236, in sage.libs.pari.gen.gen.Ser Failed example: pari(Mod(0, 7)).Ser() Expected: O(x^16) Got: Mod(0, 7) + O(x^16) ********************************************************************** 1 item had failures: 1 of 9 in sage.libs.pari.gen.gen.Ser [1270 tests, 1 failure, 1.48 s]
comment:122 Changed 8 years ago by
comment:123 Changed 8 years ago by
Replying to rws:
Fails on OpenSuSE 12.3:
Ouch, that is my fault. The change to Ser()
was the last thing I added to my reviewer patch; I tested everything that I thought might call this code, but inexplicably forgot to test the file itself. I'll fix the doctest.
comment:124 Changed 8 years ago by
Commit:  5db54b63a2e694423f33bd50819e46157be636db → 1d103da9fc0cc1629750e421a07cdfed4b94b040 

Branch pushed to git repo; I updated commit sha1. New commits:
1d103da  Trac 15767: fix doctest in Ser()

comment:125 followup: 127 Changed 8 years ago by
Peter, could you please add some documentation for the application of Sturm's Theorem in ell.gp
? I checked the calculations by hand and it looks correct, but it would be nice to have the calculation as comment in the file.
comment:126 Changed 8 years ago by
Commit:  1d103da9fc0cc1629750e421a07cdfed4b94b040 → 62ca8211de7d43d0686edf0d77f55a23aaeee5c7 

Branch pushed to git repo; I updated commit sha1. New commits:
62ca821  Trac 15767: explain application of Sturm's theorem

comment:127 followup: 128 Changed 8 years ago by
Status:  needs_work → needs_review 

Replying to jdemeyer:
Peter, could you please add some documentation for the application of Sturm's Theorem in
ell.gp
? I checked the calculations by hand and it looks correct, but it would be nice to have the calculation as comment in the file.
Done in the above commit.
comment:128 Changed 8 years ago by
Status:  needs_review → positive_review 

comment:129 followup: 130 Changed 8 years ago by
Status:  positive_review → needs_work 

I'm getting merge failures, will release beta1 soon. Please merge with that release then.
comment:130 Changed 8 years ago by
Replying to vbraun:
I'm getting merge failures, will release beta1 soon. Please merge with that release then.
OK, I will do that. Unfortunately, this ticket tends to give a lot of conflicts because it touches so much code (in a way which cannot really be avoided). For beta2, I hope you will give this ticket priority and ask other branches to merge with this.
comment:131 Changed 8 years ago by
I've released beta1, if you can resolve the conflicts soon then I'll merge this as the first ticket.
comment:132 Changed 8 years ago by
Branch:  u/pbruin/15767pari2.7.1 → u/jdemeyer/ticket/15767 

Modified:  Aug 20, 2014, 10:29:36 AM → Aug 20, 2014, 10:29:36 AM 
comment:133 Changed 8 years ago by
Commit:  62ca8211de7d43d0686edf0d77f55a23aaeee5c7 → bf435f8db1537f63c001894fb4949879617b71f2 

Status:  needs_work → needs_review 
Done. I'm running doctests now to check that nothing got broken.
comment:134 Changed 8 years ago by
This branch agrees (up to whitespace) with a merge I did. If it passes doctests, you can set it back to positive review.
comment:135 Changed 8 years ago by
Status:  needs_review → positive_review 

comment:136 Changed 8 years ago by
Branch:  u/jdemeyer/ticket/15767 → bf435f8db1537f63c001894fb4949879617b71f2 

Resolution:  → fixed 
Status:  positive_review → closed 
Good plan; there have been quite a few changes (also in the interface) from 2.5 to 2.6, but 2.7 (being a stable release) will be close to 2.6.2.