Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#16997 closed enhancement (fixed)

Upgrade PARI to git master

Reported by: jdemeyer Owned by:
Priority: major Milestone: sage-6.6
Component: packages: standard Keywords:
Cc: pbruin, fbissey Merged in:
Authors: Jeroen Demeyer Reviewers: François Bissey
Report Upstream: N/A Work issues:
Branch: 451fb95 (Commits) Commit:
Dependencies: #16976 Stopgaps:

Attachments (1)

infinity.patch (4.9 KB) - added by jdemeyer 3 years ago.
Removed patch

Download all attachments as: .zip

Change History (106)

comment:1 Changed 3 years ago by jdemeyer

  • Description modified (diff)

comment:2 Changed 3 years ago by jdemeyer

  • Branch set to u/jdemeyer/ticket/16997
  • Created changed from 09/17/14 08:28:46 to 09/17/14 08:28:46
  • Modified changed from 09/17/14 10:12:01 to 09/17/14 10:12:01

comment:3 Changed 3 years ago by jdemeyer

  • Commit set to 006f9b95451c32d68a28af15a0cd65a4bdeac097
  • Description modified (diff)

New commits:

006f9b9Upgrade to PARI git master

comment:4 Changed 3 years ago by jdemeyer

  • Dependencies set to #15808

comment:5 Changed 3 years ago by git

  • Commit changed from 006f9b95451c32d68a28af15a0cd65a4bdeac097 to 3ffdf18b24d4b8beb8976e6407d3a5cddb0efba5

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

927e70cRemove genus2reduction
3ffdf18Upgrade to PARI git master

comment:6 Changed 3 years ago by jdemeyer

  • Dependencies changed from #15808 to #15808, #17015

comment:7 Changed 3 years ago by git

  • Commit changed from 3ffdf18b24d4b8beb8976e6407d3a5cddb0efba5 to 8bbd25d10c7c477ba1b2dd1adf84057d56ea266d

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

11946d6Fix documentation
c92fba3#17015: upgrade eclib to 20140920
ec61d5e#17105 upgrade eclib to 20140921
0a4f6ecMerge branches 'ticket/15808' and 'ticket/17015' into HEAD
8bbd25dUpgrade to PARI git master

comment:8 Changed 3 years ago by git

  • Commit changed from 8bbd25d10c7c477ba1b2dd1adf84057d56ea266d to 0ad0a2a2184ed528d0232759fd2f33f8b4b8bf94

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

0ad0a2aUpgrade to PARI git master

comment:9 Changed 3 years ago by jdemeyer

  • Description modified (diff)

comment:10 Changed 3 years ago by git

  • Commit changed from 0ad0a2a2184ed528d0232759fd2f33f8b4b8bf94 to 75d40e2a152f9b4353aec7b2dc1f79797f51194f

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

75d40e2Upgrade to PARI git master

comment:11 Changed 3 years ago by git

  • Commit changed from 75d40e2a152f9b4353aec7b2dc1f79797f51194f to 66ba6dbe2499b9d832e6116a21f630e40416761e

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

66ba6dbUpgrade to PARI git master

comment:12 Changed 3 years ago by git

  • Commit changed from 66ba6dbe2499b9d832e6116a21f630e40416761e to a70061e62a90419659125f5b28b619a34b478dd4

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

a70061eUpgrade to PARI git master

comment:13 Changed 3 years ago by git

  • Commit changed from a70061e62a90419659125f5b28b619a34b478dd4 to f7db9fdf73e6a6207c150adede81afe10b51b8b0

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

f7db9fdUpgrade to PARI git master

comment:14 Changed 3 years ago by git

  • Commit changed from f7db9fdf73e6a6207c150adede81afe10b51b8b0 to 758c15d0097b33a8aade41ae732b547b89758996

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

758c15dUpgrade to PARI git master

comment:15 Changed 3 years ago by jdemeyer

  • Description modified (diff)

comment:16 Changed 3 years ago by git

  • Commit changed from 758c15d0097b33a8aade41ae732b547b89758996 to b57b237855823539b0092e04ba784076f6b7bfde

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

b57b237Upgrade to PARI git master

comment:17 Changed 3 years ago by jdemeyer

  • Status changed from new to needs_review

comment:18 Changed 3 years ago by jdemeyer

  • Cc zimmerma pbruin added

This patch changes a French book doctest.

comment:19 Changed 3 years ago by jdemeyer

  • Description modified (diff)

Changed 3 years ago by jdemeyer

Removed patch

comment:20 Changed 3 years ago by git

  • Commit changed from b57b237855823539b0092e04ba784076f6b7bfde to e882752d0fe1218033e219b3132aabcee263805d

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

c4f948dMerge with 6.4.beta4
1068feftrac #15808 correct name for _reduce + a few pep8 changes
f35c944Fix "away from 2" check
e882752Upgrade to PARI git master

comment:21 Changed 3 years ago by jdemeyer

  • Dependencies changed from #15808, #17015 to #15808

comment:22 Changed 3 years ago by zimmerma

  • Cc zimmerma removed

I removed my name in the cc list, since it makes no sense to try to maintain coherence of the french doctests with our book: there are now too many changes which break the examples from the book.

Paul

comment:23 Changed 3 years ago by git

  • Commit changed from e882752d0fe1218033e219b3132aabcee263805d to e28c799e89281dc23a0b2038011f114290bf7cc1

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

e28c799Upgrade to PARI git master

comment:24 Changed 3 years ago by git

  • Commit changed from e28c799e89281dc23a0b2038011f114290bf7cc1 to 84b34495e10d6414de76e5a336bae3a0256f67f0

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

84b3449Upgrade to PARI git master

comment:25 Changed 3 years ago by git

  • Commit changed from 84b34495e10d6414de76e5a336bae3a0256f67f0 to 105255a42adcbacd532d91f5aae7c67bd705aefc

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

105255aUpgrade to PARI git master

comment:26 Changed 3 years ago by git

  • Commit changed from 105255a42adcbacd532d91f5aae7c67bd705aefc to daaea9ff716f17a2a663822439568c5976b4dc22

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

daaea9fUpgrade to PARI git master

comment:27 Changed 3 years ago by git

  • Commit changed from daaea9ff716f17a2a663822439568c5976b4dc22 to cbc0c5a5064b22f10b4a4a81bca0a090bf92e013

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

cbc0c5aUpgrade to PARI git master

comment:28 Changed 3 years ago by jdemeyer

I have removed the controversial patches, it now needs review again.

comment:29 Changed 3 years ago by git

  • Commit changed from cbc0c5a5064b22f10b4a4a81bca0a090bf92e013 to 660eb660de4cfcb69c0866e535ff3ba4e0d2eec3

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

660eb66Upgrade to PARI git master

comment:30 Changed 3 years ago by git

  • Commit changed from 660eb660de4cfcb69c0866e535ff3ba4e0d2eec3 to db7dbd65ac7b62b561cdd4adb664ebfc04319ed0

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

db7dbd6Upgrade to PARI git master

comment:31 Changed 3 years ago by jdemeyer

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

comment:32 Changed 3 years ago by git

  • Commit changed from db7dbd65ac7b62b561cdd4adb664ebfc04319ed0 to ff7c17ffc368222d596ab610c6433f18ce5e1ce2

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

ff7c17fUpgrade to PARI git master

comment:33 Changed 3 years ago by jdemeyer

  • Dependencies changed from #15808 to #17028

comment:34 Changed 3 years ago by jdemeyer

  • Dependencies #17028 deleted

comment:35 Changed 3 years ago by git

  • Commit changed from ff7c17ffc368222d596ab610c6433f18ce5e1ce2 to 5e8d9841bac18d098e5f0b22eed30aee2421d0c2

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

5e8d984Upgrade to PARI git master

comment:36 Changed 3 years ago by jdemeyer

  • Dependencies set to #15808
  • Modified changed from 12/03/14 15:04:03 to 12/03/14 15:04:03

comment:37 Changed 3 years ago by git

  • Commit changed from 5e8d9841bac18d098e5f0b22eed30aee2421d0c2 to b2597a79976e4b8dbb6c2d40958e3e775b5ad796

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

b2597a7Upgrade to PARI git master

comment:38 Changed 3 years ago by jdemeyer

  • Status changed from needs_work to needs_review

comment:39 Changed 3 years ago by jdemeyer

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

comment:40 Changed 3 years ago by jdemeyer

  • Dependencies changed from #15808 to #17623

comment:41 Changed 3 years ago by git

  • Commit changed from b2597a79976e4b8dbb6c2d40958e3e775b5ad796 to bb12f77ccd30a3e844532494a780e4aa14294dfa

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

a13b116Fix PI declaration in complex_plot.pyx
bb12f77Upgrade to PARI git master

comment:42 Changed 3 years ago by git

  • Commit changed from bb12f77ccd30a3e844532494a780e4aa14294dfa to 7528799c56ca6f3ffafdc2c92df1468e85a3a43e

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

7528799Upgrade to PARI git master

comment:43 Changed 3 years ago by jdemeyer

  • Status changed from needs_work to needs_review

comment:44 Changed 3 years ago by jdemeyer

  • Dependencies changed from #17623 to #17623, #14894
  • Description modified (diff)

comment:45 Changed 3 years ago by git

  • Commit changed from 7528799c56ca6f3ffafdc2c92df1468e85a3a43e to 5cd1f002e107df3d7ceadf71e89188f6776676af

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

0602271Use cb_pari_err_handle() to handle PARI errors
fbd37a0Merge commit 'a13b11636e7cb6b0f5c066b6d7784361fa68091d' into HEAD
5cd1f00Upgrade PARI to git master

comment:46 Changed 3 years ago by jdemeyer

  • Dependencies #17623, #14894 deleted
  • Description modified (diff)
  • Milestone changed from sage-6.4 to sage-6.5
  • Status changed from needs_review to needs_work

comment:47 Changed 3 years ago by jdemeyer

  • Dependencies set to #17588

comment:48 Changed 3 years ago by git

  • Commit changed from 5cd1f002e107df3d7ceadf71e89188f6776676af to b8004f76206976cbc397f921e773ef54fd9669db

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

0fa866cTrac #17588: Remove darwin specific install instruction that are counterproductive.
b8004f7Upgrade PARI to git master

comment:49 Changed 3 years ago by jdemeyer

  • Status changed from needs_work to needs_review

comment:50 Changed 3 years ago by fbissey

  • Cc fbissey added

comment:51 Changed 3 years ago by vdelecroix

Do we change the package name to sage-pari as it was discussed on sage-devel?

Last edited 3 years ago by vdelecroix (previous) (diff)

comment:52 Changed 3 years ago by fbissey

After my long rant (and mostly getting stuff out of my chest) I no longer think it is necessary. Let me explain:

  • it is not just giving a name to the tarball. You would want a distinct name for the library and the executable.
  • for sanity reason you don't want to carry 2 versions of pari, one for the package and one for the library. So you get to change all the packages that link to pari at least.
  • Last but not least. There are not that many packages using pari that are not in sage and a lot of them depend on ancient pari (2.3). So unless someone pulls a list of software using pari from under their beds i think the point on maintaining the stack is moot.

I did some homework on the last point in Gentoo I am not sure what it would hold for debian but I am willing to bet it will be about the same.

comment:53 Changed 3 years ago by jdemeyer

By the way: if somebody actually feels like reviewing this, let me know. Then I will update the branch.

comment:54 follow-ups: Changed 3 years ago by fbissey

I'll need a little bit of time to get over all the dependency but once I get there I may review this. One question however: is the new patch public_memory_functions.patch something waiting for inclusion upstream? As of this morning it would probably still apply (I now have a live ebuild for pari and this morning master is not *broken*, not like 2 days ago).

comment:55 in reply to: ↑ 54 Changed 3 years ago by jdemeyer

Replying to fbissey:

One question however: is the new patch public_memory_functions.patch something waiting for inclusion upstream?

Consider this a "transient" patch. PARI upstream has changed their stack-handling in a non-backwards-compatible way. This patch public_memory_functions.patch is what's needed to use the current Sage hooks for dealing with the PARI stack. A follow-up ticket could use the new PARI stack mechanism.

comment:56 in reply to: ↑ 54 Changed 3 years ago by jdemeyer

Replying to fbissey:

I'll need a little bit of time to get over all the dependency

What does "get over all the dependency" mean???

comment:57 Changed 3 years ago by jdemeyer

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

comment:58 Changed 3 years ago by fbissey

To be honest I thought there would be more package depending on pari and that more would be impacted. But apart from sage itself that's just lcalc. If we are looking at optional package there may be MaCaulay2.

comment:59 follow-up: Changed 3 years ago by jdemeyer

eclib also depends on pari.

comment:60 in reply to: ↑ 59 ; follow-up: Changed 3 years ago by fbissey

Replying to jdemeyer:

eclib also depends on pari.

And I noticed that the extent of your fix is adding the standard patching code in spkg-install. I can do some more checking with it but I can believe eclib will be unaffected since John already did a fix for pari 2.8 https://github.com/JohnCremona/eclib/commit/652337883464b277f52901c7bcaf9c71001e0c3c

comment:61 follow-up: Changed 3 years ago by fbissey

And if we want to be complete sympow depends on pari but only through gp.

comment:62 in reply to: ↑ 60 Changed 3 years ago by jdemeyer

Replying to fbissey:

Replying to jdemeyer:

eclib also depends on pari.

And I noticed that the extent of your fix is adding the standard patching code in spkg-install. I can do some more checking with it but I can believe eclib will be unaffected since John already did a fix for pari 2.8

Yes, I know. In an earlier version of this branch, I had to explicitly add a patch and the "patching loop" in spkg-install. I left the patching loop since it makes sense anyway to have it.

I am currently in the process of upgrading this branch to the latest PARI.

comment:63 follow-up: Changed 3 years ago by fbissey

Can you put an spkg-src this time? "make snapshot" is not enough to reproduce a given tarball, we should know the precise commit at which you snapshot.

comment:64 in reply to: ↑ 63 ; follow-up: Changed 3 years ago by jdemeyer

Replying to fbissey:

"make snapshot" is not enough to reproduce a given tarball, we should know the precise commit at which you snapshot.

The commit is in build/pkgs/pari/package-version.txt, is that not sufficient?

comment:65 in reply to: ↑ 64 Changed 3 years ago by fbissey

Replying to jdemeyer:

Replying to fbissey:

"make snapshot" is not enough to reproduce a given tarball, we should know the precise commit at which you snapshot.

The commit is in build/pkgs/pari/package-version.txt, is that not sufficient?

I guess it is. I am too lazy ;)

comment:66 Changed 3 years ago by git

  • Commit changed from b8004f76206976cbc397f921e773ef54fd9669db to ade4f7521ad77c03e9791bc3768d9a9e3a2daca4

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

ade4f75Upgrade PARI to git master

comment:67 Changed 3 years ago by jdemeyer

  • Status changed from needs_work to needs_review

comment:68 in reply to: ↑ 61 Changed 3 years ago by jdemeyer

Replying to fbissey:

And if we want to be complete sympow depends on pari but only through gp.

I didn't know that, but you're right.

comment:69 Changed 3 years ago by fbissey

OK now I am starting tests. A technical question about upstream's intention. The current soname for pari 2.7 is libpari-gmp.so.4 the one from git master is libpari-gmp-2.8.so.0 do you know if it is temporary until 2.8 is released (and switch to something like libpari-gmp.so.5) or it is their plan to stick with that soname?

comment:70 Changed 3 years ago by fbissey

I think I found the answer to my own question in config directory. They have a different scheme for even numbered release on purpose. Which makes life hard for automated rebuilding in gentoo. I'll have to force things there.

comment:71 Changed 3 years ago by fbissey

  • Reviewers set to François Bissey
  • Status changed from needs_review to positive_review

OK passes a few sanity checks so this is ready for bot testing.

comment:72 Changed 3 years ago by git

  • Commit changed from ade4f7521ad77c03e9791bc3768d9a9e3a2daca4 to b41dd9103a0e09878d697bc4611c1284e4a68873
  • Status changed from positive_review to needs_review

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

b41dd91Fix 32-bit doctest

comment:73 Changed 3 years ago by jdemeyer

  • Status changed from needs_review to positive_review

comment:74 Changed 3 years ago by vbraun

I also get on some buildbots:

sage -t --long src/sage/modular/modform_hecketriangle/hecke_triangle_group_element.py
**********************************************************************
File "src/sage/modular/modform_hecketriangle/hecke_triangle_group_element.py", line 38, in sage.modular.modform_hecketriangle.hecke_triangle_group_element.coerce_AA
Failed example:
    AA(p)._exact_field()
Expected:
    Number Field in a with defining polynomial y^16 + ... with a in ...
Got:
    Number Field in a with defining polynomial y^16 - 1312*y^14 + 864612*y^12 - 2512720*y^11 - 50780668*y^10 + 360028808*y^9 - 48779642409*y^8 + 290398268512*y^7 + 15186503722504*y^6 - 566858046818416*y^5 + 27431234189939174*y^4 - 226711842252026288*y^3 + 7395502353944441460*y^2 - 39603059247494358200*y + 911555406262192128578 with a in -25.34269035681775? - 10.68732885060488?*I
**********************************************************************
1 item had failures:
   1 of   5 in sage.modular.modform_hecketriangle.hecke_triangle_group_element.coerce_AA
    [709 tests, 1 failure, 115.89 s]

and

sage -t --long src/sage/lfunctions/dokchitser.py
**********************************************************************
File "src/sage/lfunctions/dokchitser.py", line 127, in sage.lfunctions.dokchitser.Dokchitser
Failed example:
    L.taylor_series(1,4)
Expected:
    2.90614059330736e-20 + (-1.64690098288532e-20)*z + 0.759316500288427*z^2 - 0.430302337583362*z^3 + O(z^4)  
Got:
    -1.15498883731660e-19 + (6.54528640417060e-20)*z + 0.759316500288427*z^2 - 0.430302337583362*z^3 + O(z^4)
**********************************************************************

comment:75 Changed 3 years ago by jdemeyer

The first one is because of interaction with #16976.

Last edited 3 years ago by jdemeyer (previous) (diff)

comment:76 Changed 3 years ago by jdemeyer

  • Milestone changed from sage-6.5 to sage-6.6

The dokchitser failure is at least related to this ticket, but I don't see what could cause the failure... Maybe there is also some interaction with another ticket?

Last edited 3 years ago by jdemeyer (previous) (diff)

comment:77 Changed 3 years ago by git

  • Commit changed from b41dd9103a0e09878d697bc4611c1284e4a68873 to 4c8b04499de97a8fd13fa5f526ee12c97e8804e6
  • Status changed from positive_review to needs_review

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. Last 10 new commits:

9d3bb76remove a useless TODO
433b9d1merge 6.5.beta5
f63c738trac #16976: clean code for Hecke group element
616b048trac #16976: removed 3 out of 5 @cached_method
67116d5addressed the review: renamed some functions, added 2 cached_methods back, improved documentation, fixed doctests
9fabb44improve documentation
6d73481merge sage-6.6.beta1 in #16976
7dcee34remove .gitignore, add further explanations on __init__, change ValueError to TypeError for element checks
02aab05Merge commit '7dcee3441862c9af9f1a63e1c3c8d4b957d2d705' into HEAD
4c8b044Fix doctest for _exact_field()

comment:78 Changed 3 years ago by jdemeyer

  • Dependencies changed from #17588 to #16976

Merged with #16976 and fixed first failure.

comment:79 Changed 3 years ago by jdemeyer

For the dokchitser failure, there is some annoying global state (which is probably a bug) in the computel.gp script. For example, in the doctest

        sage: E = EllipticCurve('389a')
        sage: L = E.lseries().dokchitser()
        sage: L.num_coeffs ()
        156
        sage: L.derivative(1,E.rank())
        1.51863300057685
        sage: L.taylor_series(1,4)
        2.90760251490292e-20 + (-1.64772944938078e-20)*z + 0.759316500288427*z^2 - 0.430302337583362*z^3 + O(z^4)  # 32-bit
        2.90614059330736e-20 + (-1.64690098288532e-20)*z + 0.759316500288427*z^2 - 0.430302337583362*z^3 + O(z^4)  # 64-bit

removing the L.num_coeffs () test changes the last results.

comment:80 Changed 3 years ago by vbraun

Aren't both answers valid (up to numerical precision). Its somewhat annoying for testing purposes, maybe reset the state with other rng seeds.

comment:81 Changed 3 years ago by vdelecroix

Indeed

sage: E = EllipticCurve('389a')
sage: L = E.lseries().dokchitser()
sage: L.derivative(1,E.rank())
1.51863300057685
sage: s4_1 = L.taylor_series(1,4)
sage: L.num_coeffs ()
156
sage: s4_2 = L.taylor_series(1,4)
sage: s4_1 == s4_2
False
sage: s4_1 - s4_2
-2.90886971068887e-20 + (1.64844756535569e-20)*z + O(z^4)

comment:82 Changed 3 years ago by jdemeyer

AFAIK, it's not a rng issue. It's just that the result depends on seemingly unrelated things (like whether or not L.num_coeffs() has been called).

Volker, do have an idea which elliptic-curve/L-function related ticket could have caused this?

comment:83 follow-up: Changed 3 years ago by vbraun

All doctests pass without the pari update, so its this ticket.

comment:84 in reply to: ↑ 83 Changed 3 years ago by jdemeyer

Replying to vbraun:

All doctests pass without the pari update, so its this ticket.

All doctests also pass when just this ticket is applied, so it must be some interaction between this ticket and another unspecified ticket.

comment:85 follow-up: Changed 3 years ago by vdelecroix

Sided note: why is dokchister included as such in Sage source files? Wouldn't it be better as an external gp library (let say $SAGE_ROOT/local/pari/)? Morever, we ship version 1.3 and current version is 1.3.3 (see http://www.maths.bris.ac.uk/~matyd/computel/index.html). Though the last version does not solve the issue at all (but at least the src/ext/pari/dokchister/testall script is not emitting any warning anymore).

comment:86 in reply to: ↑ 85 Changed 3 years ago by jdemeyer

Replying to vdelecroix:

Sided note: why is dokchister included as such in Sage source files?

Historical reasons. There are various such scripts in src/ext.

Wouldn't it be better as an external gp library (let say $SAGE_ROOT/local/pari/)? Morever, we ship version 1.3 and current version is 1.3.3 (see http://www.maths.bris.ac.uk/~matyd/computel/index.html). Though the last version does not solve the issue at all (but at least the src/ext/pari/dokchister/testall script is not emitting any warning anymore).

Perhaps, but let's leave that for a different ticket.

comment:87 Changed 3 years ago by git

  • Commit changed from 4c8b04499de97a8fd13fa5f526ee12c97e8804e6 to 73e4040bc212753a9549688bfe5189a48df8be96

Branch pushed to git repo; I updated commit sha1. New commits:

73e4040Fix for LaTeX doc

comment:88 Changed 3 years ago by jdemeyer

I cannot reproduce the src/sage/lfunctions/dokchitser.py failure, even with 6.6.beta2. Can you confirm that the problem still exists?

comment:89 Changed 3 years ago by vbraun

Yep, still happens (this time on UW redhawk)

comment:90 Changed 3 years ago by fbissey

What kind of machine is redhawk?

comment:91 Changed 3 years ago by jdemeyer

I guess we could easily solve this with some # tol but I'm very curious to know why this is happening. It would be the first time that the result of PARI was different on different 64-bit systems (it is well known that PARI often has different results for 32-bit vs. 64-bit, but all 32-bit and all 64-bit systems should be the same).

comment:92 follow-up: Changed 3 years ago by zimmerma

but all 32-bit and all 64-bit systems should be the same).

I would not bet on this. It suffices that some processor provides a fused multiply-add and that Pari uses it to get different results.

Paul

comment:93 in reply to: ↑ 92 Changed 3 years ago by jdemeyer

Replying to zimmerma:

but all 32-bit and all 64-bit systems should be the same).

I would not bet on this. It suffices that some processor provides a fused multiply-add and that Pari uses it to get different results.

As far as I know, PARI doesn't use native floating-point operations. They use custom arbitrary-precision arithmetic which shouldn't depend on the system.

The reason there is a difference between 32-bit and 64-bit is because their precision is measured in words: some computation which would use 96 bits on a 32-bit system would use 128 bits on a 64-bit system.

comment:94 Changed 3 years ago by jdemeyer

  • Status changed from needs_review to needs_work

I confirm that PARI indeed gives a different result on redhawk. Upgrading Dokchitser's GP scripts seems to fix the issue, so that is what I will do.

comment:95 Changed 3 years ago by git

  • Commit changed from 73e4040bc212753a9549688bfe5189a48df8be96 to 32e0b0667399bb31c46c0999bf3b5a86aea977de

Branch pushed to git repo; I updated commit sha1. New commits:

32e0b06Upgrade Dokchitser's GP scripts

comment:96 Changed 3 years ago by git

  • Commit changed from 32e0b0667399bb31c46c0999bf3b5a86aea977de to 504f6feedf354b735d4cd340ec4d6c144014c552

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

504f6feUpgrade Dokchitser's GP scripts

comment:97 Changed 3 years ago by jdemeyer

  • Status changed from needs_work to needs_review

comment:98 Changed 3 years ago by jdemeyer

Note: the file computel.gp is exactly Dokchitser's computel with 2 minor changes: DOS newlines converted to Unix and trailing whitespace removed.

comment:99 Changed 3 years ago by fbissey

  • Status changed from needs_review to positive_review

I am happy to put it back to positive review.

comment:100 Changed 3 years ago by jdemeyer

Thanks, let's hope it can be merged this time.

comment:101 Changed 3 years ago by vbraun

  • Status changed from positive_review to needs_work

I now get this on linux 32-bit:

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.20363155418419e-6)*z^5 + O(z^6)
Got:
    0.990010459847588 + 0.0191338632530789*z - 0.0197489006172923*z^2 + 0.0137240085327618*z^3 - 0.00703880791607153*z^4 + 0.00280906165766519*z^5 + O(z^6)
**********************************************************************
1 item had failures:
   1 of  99 in doc.en.reference.coercion.index
    [84 tests, 1 failure, 0.98 s]

comment:102 Changed 3 years ago by git

  • Commit changed from 504f6feedf354b735d4cd340ec4d6c144014c552 to 451fb95b37c47f1f7290593b4e1c36fdca5e15d3

Branch pushed to git repo; I updated commit sha1. New commits:

451fb95Fix another 32-bit doctest

comment:103 Changed 3 years ago by jdemeyer

  • Status changed from needs_work to positive_review

Fixed and tested on arando (32-bit).

comment:104 Changed 3 years ago by vbraun

  • Branch changed from u/jdemeyer/ticket/16997 to 451fb95b37c47f1f7290593b4e1c36fdca5e15d3
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:105 Changed 3 years ago by tmonteil

  • Commit 451fb95b37c47f1f7290593b4e1c36fdca5e15d3 deleted

Doctests should help tracking bugs, not hide them (last commit). Last digit noise is ok but 0.990010459847588 should not be considered as a correct 32-bits approximation of 0.997997869801216, this is too much noise and there should be a bug somewhere. Since the ticket is closed, there is a followup at #17903.

Note: See TracTickets for help on using tickets.