Opened 7 years ago
Closed 6 years ago
#15767 closed enhancement (fixed)
Upgrade PARI to 2.7.1
Reported by:  jdemeyer  Owned by:  

Priority:  major  Milestone:  sage6.4 
Component:  packages: standard  Keywords:  
Cc:  jpflori, pbruin, Snark  Merged in:  
Authors:  Jeroen Demeyer  Reviewers:  Peter Bruin 
Report Upstream:  N/A  Work issues:  
Branch:  bf435f8 (Commits)  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 7 years ago by
comment:2 Changed 7 years ago by
 Milestone changed from sage6.1 to sage6.2
comment:3 Changed 7 years ago by
 Dependencies changed from #15765 to #15765, #15774
comment:4 Changed 7 years ago by
 Branch set to u/jdemeyer/ticket/15767
 Created changed from 01/30/14 16:27:35 to 01/30/14 16:27:35
 Modified changed from 02/01/14 20:55:56 to 02/01/14 20:55:56
comment:5 Changed 7 years ago by
 Commit set to 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 7 years ago by
 Commit changed from 0595a422d7c5a61858859a47c4ccc8313b37915d to 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 7 years ago by
 Commit changed from 6de648ba85ef7315acdf5a3c948c768e34d9606e to 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 7 years ago by
 Commit changed from 5b823f1ac56e6b3e4815c3002a35f33912ad0876 to 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 7 years ago by
 Commit changed from 5cb347872a7e13be6205e818eef966d58e657f0e to 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 7 years ago by
 Commit changed from 84c081c13ce3833ccb3fbe55ab87eee51e718aee to 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 7 years ago by
This version compiles fine, but there are about a million doctest failures :(
comment:12 Changed 7 years ago by
 Commit changed from 8e2cc9ed86809921f30ffca0975eb7b762bda36c to 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 7 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 7 years ago by
 Commit changed from 19b29364c702ca639c22fc81d2135a5b209d56ce to 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 7 years ago by
 Commit changed from 52e24d0e5355d83158129b7c77acf8beb5b77649 to 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 7 years ago by
 Description modified (diff)
comment:17 Changed 7 years ago by
 Commit changed from b7809162a02febeddfacc0c9eeffc1c69c3b36cc to 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 7 years ago by
 Commit changed from 9cb3ae74ad54fe44916a4ce4f4cb29857336db3c to 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 7 years ago by
 Commit changed from 7ee964c800987b343a0578b2a60fbd3400692d2a to 73f51cdf7c9137b4e64f74e84f42e6d2abc11681
comment:20 Changed 7 years ago by
 Description modified (diff)
comment:21 Changed 7 years ago by
 Commit changed from 73f51cdf7c9137b4e64f74e84f42e6d2abc11681 to 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 7 years ago by
 Description modified (diff)
comment:23 followup: ↓ 24 Changed 7 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 in reply to: ↑ 23 Changed 7 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 7 years ago by
We could also try to merge #11005 first, maybe that helps.
comment:26 in reply to: ↑ 25 Changed 7 years ago by
comment:27 Changed 7 years ago by
 Commit changed from 6091d59de30240271265c6efc02bccecb4a3e81e to 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 7 years ago by
 Commit changed from 01c76a3c6ecffb07a71af2e7b9aed5f4660a18bd to 1db424ecd1304107bbe5e59406129bd4e16371fa
comment:29 Changed 7 years ago by
 Commit changed from 1db424ecd1304107bbe5e59406129bd4e16371fa to 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 7 years ago by
 Commit changed from 802a31ffc2955036e55c741326d5759b556ce595 to 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 7 years ago by
 Commit changed from 2964947e7572ca34850aa80e9aca7c15a0850452 to 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 7 years ago by
 Commit changed from 9cc4af75e4d6663c3b2eeec3a81371d595ff0a7a to 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 7 years ago by
 Commit changed from 1b06004413d6a23af1f0836f595f8582d0386d81 to 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 7 years ago by
 Commit changed from 14a8548b3986a089f43400558d068fed1322996a to 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 7 years ago by
 Commit changed from f36fba5234615ce973bd168b0b7202a9475ab65e to 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 7 years ago by
 Commit changed from 834368dd71b707cc3eaf50d3119095f13c5252b3 to 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 7 years ago by
 Commit changed from 4829125af240ca57dde85ee4439fd23b79dfacd1 to 4f040c87385fc17d43fd2350f6a4214769923601
Branch pushed to git repo; I updated commit sha1. New commits:
4f040c8  Merge remotetracking branch 'origin/develop' into ticket/15767

comment:38 Changed 7 years ago by
 Commit changed from 4f040c87385fc17d43fd2350f6a4214769923601 to 27897d3b9b7ac21032cfc2eee2d85f17a6fd48d2
Branch pushed to git repo; I updated commit sha1. New commits:
27897d3  PARI 2.7 doctest fixes

comment:39 Changed 7 years ago by
 Dependencies changed from #15765, #15774 to #15855, #11005, #15483
comment:40 Changed 7 years ago by
 Commit changed from 27897d3b9b7ac21032cfc2eee2d85f17a6fd48d2 to 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 7 years ago by
 Dependencies #15855, #11005, #15483 deleted
comment:42 Changed 7 years ago by
 Commit changed from ac97322e0aa2a0a772552a490ab73feedc351e4e to 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 7 years ago by
 Dependencies set to #11005, #15483, #15855
 Modified changed from 03/04/14 09:00:11 to 03/04/14 09:00:11
comment:44 Changed 7 years ago by
 Dependencies #11005, #15483, #15855 deleted
comment:45 Changed 7 years ago by
 Dependencies set to #11005, #15483, #15855
 Modified changed from 03/04/14 09:02:23 to 03/04/14 09:02:23
comment:46 Changed 7 years ago by
 Dependencies #11005, #15483, #15855 deleted
comment:47 Changed 7 years ago by
 Commit changed from c80cf8bc39c434523abaa5ad3401932327081798 to 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 7 years ago by
 Commit changed from 94d14a2e6b0139d77f9f0d46de05151ad2779fde to 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 7 years ago by
 Commit changed from d08c8fcff4d8eb2a7b6c1209c73f0226223e9b77 to 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 7 years ago by
 Dependencies set to #15888
comment:51 Changed 7 years ago by
 Commit changed from 44c0f87c3c896586459b538c2b383b26ae325654 to bedb19ad7fb799b387a2e07f69a76966488f6ec9
comment:52 Changed 7 years ago by
 Commit changed from bedb19ad7fb799b387a2e07f69a76966488f6ec9 to 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 7 years ago by
 Dependencies changed from #15888 to #15888, #15855
comment:54 Changed 7 years ago by
 Commit changed from 6a5539f9b3941feb9fd4d9ddb807f3cde82e8e38 to 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 7 years ago by
 Commit changed from 1a12d09f53d9a003b8e319d83d7976970c3d3c0e to 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 7 years ago by
 Commit changed from 5467494c1f852e503c36de8379dbff3616460fd4 to 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 7 years ago by
 Description modified (diff)
comment:58 Changed 7 years ago by
 Commit changed from 3970e579c3d74fba6f0dc39745425bc3649ad819 to ae1ae57d04ca1b9ba1acb789ed1919eda1d01a5e
comment:59 Changed 7 years ago by
 Description modified (diff)
comment:60 followup: ↓ 62 Changed 7 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 7 years ago by
 Commit changed from ae1ae57d04ca1b9ba1acb789ed1919eda1d01a5e to 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 in reply to: ↑ 60 Changed 7 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 7 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 7 years ago by
 Commit changed from f6a149b845d40c84ef63a45f71af4c8cf5abe356 to a6e2f9d188ae4e941feb83fb6b1a6daa5611a50d
comment:65 Changed 7 years ago by
This passes 64bit doctests for me. Now building a 32bit version to check that...
comment:66 Changed 7 years ago by
 Description modified (diff)
comment:67 Changed 7 years ago by
 Commit changed from a6e2f9d188ae4e941feb83fb6b1a6daa5611a50d to 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 7 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 in reply to: ↑ 68 Changed 7 years ago by
comment:70 Changed 7 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 7 years ago by
 Cc Snark added
comment:72 Changed 7 years ago by
 Commit changed from b321ea39b0751f803f48162ae222630107e426a1 to 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 7 years ago by
 Description modified (diff)
comment:74 Changed 7 years ago by
 Commit changed from ee9c845e18e6d229b7a83f8b365e4e24e0fbc1f9 to 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 7 years ago by
 Commit changed from 1d9305b0ca69ceb46edf1623690092fa1bea3f03 to 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 7 years ago by
 Commit changed from 082bcebe1f0473e5c551268d763229bd50861070 to fd2bb156f8d61533c93159388b220747256786c2
Branch pushed to git repo; I updated commit sha1. New commits:
fd2bb15  Merge remotetracking branch 'origin/develop' into ticket/15767

comment:77 Changed 7 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 7 years ago by
 Description modified (diff)
 Summary changed from Upgrade PARI to 2.7 to Upgrade PARI to 2.7.1
comment:79 Changed 7 years ago by
 Commit changed from fd2bb156f8d61533c93159388b220747256786c2 to 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 7 years ago by
 Commit changed from 2821016720f8c67f43ed8e9762dafebbd398dfe3 to 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 7 years ago by
 Description modified (diff)
comment:82 Changed 7 years ago by
 Commit changed from 1b3812333dae05081ca57d9df6094e4eb519b674 to 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 7 years ago by
 Milestone changed from sage6.2 to sage6.3
comment:84 Changed 7 years ago by
 Commit changed from f26a0a0136b0067e12b05c207e0939ba5654a4ee to 5c076d69ceeca9c9aced70cbeba52f87dcb15b68
Branch pushed to git repo; I updated commit sha1. New commits:
5c076d6  Upgrade to pari2.7.1pre2

comment:85 Changed 7 years ago by
 Description modified (diff)
New commits:
5c076d6  Upgrade to pari2.7.1pre2

comment:86 Changed 6 years ago by
Are there any obstacles to finishing this ticket soon?
comment:87 Changed 6 years ago by
Only 70...
comment:88 Changed 6 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 6 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 6 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 6 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 6 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 6 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 6 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 6 years ago by
 Description modified (diff)
comment:96 Changed 6 years ago by
 Commit changed from 5c076d69ceeca9c9aced70cbeba52f87dcb15b68 to 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 6 years ago by
 Commit changed from 1f8eb3846b5af85192848180d019486b640e57c6 to 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 6 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 6 years ago by
 Commit changed from 59a50645de5bc5e486bfd06917313f574754be90 to 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 6 years ago by
 Commit changed from 06dbe23773fb47f0a98687dfef143cab2bca4b6e to 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 in reply to: ↑ 98 ; followups: ↓ 102 ↓ 112 Changed 6 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 in reply to: ↑ 101 Changed 6 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 6 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 6 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.
comment:105 Changed 6 years ago by
It seems that the link to the updated genus2red
has broken.
comment:106 Changed 6 years ago by
Yes indeed, note that I downloaded it a few days ago without problem.
comment:107 followup: ↓ 108 Changed 6 years ago by
I have 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 in reply to: ↑ 107 Changed 6 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 6 years ago by
 Milestone changed from sage6.3 to sage6.4
comment:110 Changed 6 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:111 Changed 6 years ago by
Sure, I'll start with rebasing to the latest beta. Hold on...
comment:112 in reply to: ↑ 101 ; followup: ↓ 115 Changed 6 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:113 Changed 6 years ago by
Did you get any answers from Denis Simon yet?
comment:114 Changed 6 years ago by
 Commit changed from e5bdf4ccc2bab46b98c096e3cbb15dd0f942623e to 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 in reply to: ↑ 112 Changed 6 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 6 years ago by
 Commit changed from 5605a97a074c889f410f59a264f9a7a33aa4934b to 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 6 years ago by
I rebased this ticket and added the fixes by Peter Bruin. It now again passes doctests on 64bit.
comment:118 Changed 6 years ago by
I am now running make ptestlong
on ARM 32bit.
comment:119 Changed 6 years ago by
 Status changed from new to needs_review
Tests pass on 32bit Linux! Party!
comment:120 Changed 6 years ago by
 Branch changed from u/jdemeyer/ticket/15767 to u/pbruin/15767pari2.7.1
 Commit changed from 37fc8e88689a0197c5513161b85b8bbadd5630ab to 5db54b63a2e694423f33bd50819e46157be636db
 Reviewers set to 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 6 years ago by
 Status changed from needs_review to 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 in reply to: ↑ 121 Changed 6 years ago by
comment:123 in reply to: ↑ 121 Changed 6 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 6 years ago by
 Commit changed from 5db54b63a2e694423f33bd50819e46157be636db to 1d103da9fc0cc1629750e421a07cdfed4b94b040
Branch pushed to git repo; I updated commit sha1. New commits:
1d103da  Trac 15767: fix doctest in Ser()

comment:125 followup: ↓ 127 Changed 6 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 6 years ago by
 Commit changed from 1d103da9fc0cc1629750e421a07cdfed4b94b040 to 62ca8211de7d43d0686edf0d77f55a23aaeee5c7
Branch pushed to git repo; I updated commit sha1. New commits:
62ca821  Trac 15767: explain application of Sturm's theorem

comment:127 in reply to: ↑ 125 ; followup: ↓ 128 Changed 6 years ago by
 Status changed from needs_work to 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 in reply to: ↑ 127 Changed 6 years ago by
 Status changed from needs_review to positive_review
comment:129 followup: ↓ 130 Changed 6 years ago by
 Status changed from positive_review to needs_work
I'm getting merge failures, will release beta1 soon. Please merge with that release then.
comment:130 in reply to: ↑ 129 Changed 6 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 6 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 6 years ago by
 Branch changed from u/pbruin/15767pari2.7.1 to u/jdemeyer/ticket/15767
 Modified changed from 08/20/14 10:29:36 to 08/20/14 10:29:36
comment:133 Changed 6 years ago by
 Commit changed from 62ca8211de7d43d0686edf0d77f55a23aaeee5c7 to bf435f8db1537f63c001894fb4949879617b71f2
 Status changed from needs_work to needs_review
Done. I'm running doctests now to check that nothing got broken.
comment:134 Changed 6 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 6 years ago by
 Status changed from needs_review to positive_review
comment:136 Changed 6 years ago by
 Branch changed from u/jdemeyer/ticket/15767 to bf435f8db1537f63c001894fb4949879617b71f2
 Resolution set to fixed
 Status changed from positive_review to 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.