Opened 2 years ago

Closed 22 months ago

#29313 closed enhancement (fixed)

Upgrade to pari 2.11.4

Reported by: gh-timokau Owned by:
Priority: major Milestone: sage-9.2
Component: packages: standard Keywords:
Cc: arojas, fbissey, saraedum, thansen, chapoton, embray, jdemeyer, dimpase Merged in:
Authors: Antonio Rojas Reviewers: Dima Pasechnik, François Bissey
Report Upstream: N/A Work issues:
Branch: 549e094 (Commits, GitHub, GitLab) Commit: 549e094174b9fb0b2fc5b7462d034f118e3fd1f3
Dependencies: #29472 Stopgaps:

Status badges

Description (last modified by dimpase)

Unfortunately the update is not trivial. Here are some doctest failures:

File "/nix/store/vqiiwb9rzmzh97gzx1wmvqyiy9d5qhxj-sage-src-8.9/src/sage/rings/number_field/number_field.py", line 3253, in sage.rings.number_field.number_field.NumberField_generic.ideals_of_bdd_norm
Failed example:
    for n in d:
        print(n)
        for I in d[n]:
            print(I)
Expected:
    1
    Fractional ideal (1)
    2
    Fractional ideal (2, 1/2*a - 1/2)
    Fractional ideal (2, 1/2*a + 1/2)
    3
    Fractional ideal (3, 1/2*a - 1/2)
    Fractional ideal (3, 1/2*a + 1/2)
    4
    Fractional ideal (4, 1/2*a + 3/2)
    Fractional ideal (2)
    Fractional ideal (4, 1/2*a + 5/2)
    5
    6
    Fractional ideal (1/2*a - 1/2)
    Fractional ideal (6, 1/2*a + 5/2)
    Fractional ideal (6, 1/2*a + 7/2)
    Fractional ideal (1/2*a + 1/2)
    7
    8
    Fractional ideal (1/2*a + 3/2)
    Fractional ideal (4, a - 1)
    Fractional ideal (4, a + 1)
    Fractional ideal (1/2*a - 3/2)
    9
    Fractional ideal (9, 1/2*a + 11/2)
    Fractional ideal (3)
    Fractional ideal (9, 1/2*a + 7/2)
    10
Got:
    1
    Fractional ideal (1)
    2
    Fractional ideal (2, 1/2*a - 1/2)
    Fractional ideal (2, 1/2*a + 1/2)
    3
    Fractional ideal (3, 1/2*a + 1/2)
    Fractional ideal (3, 1/2*a - 1/2)
    4
    Fractional ideal (4, 1/2*a + 3/2)
    Fractional ideal (2)
    Fractional ideal (4, 1/2*a + 5/2)
    5
    6
    Fractional ideal (6, 1/2*a + 7/2)
    Fractional ideal (1/2*a + 1/2)
    Fractional ideal (1/2*a - 1/2)
    Fractional ideal (6, 1/2*a + 5/2)
    7
    8
    Fractional ideal (1/2*a + 3/2)
    Fractional ideal (4, a - 1)
    Fractional ideal (4, a + 1)
    Fractional ideal (1/2*a - 3/2)
    9
    Fractional ideal (9, 1/2*a + 7/2)
    Fractional ideal (3)
    Fractional ideal (9, 1/2*a + 11/2)
    10
**********************************************************************
File "/nix/store/vqiiwb9rzmzh97gzx1wmvqyiy9d5qhxj-sage-src-8.9/src/sage/rings/number_field/number_field.py", line 4369, in sage.rings.number_field.number_field.NumberField_generic._S_class_group_and_units
Failed example:
    K._S_class_group_and_units(tuple(K.primes_above(13)))
Expected:
    ([2/13*a^2 + 1/13*a - 677/13,
      1/13*a^2 + 7/13*a - 332/13,
      -1/13*a^2 + 6/13*a + 345/13,
      -1,
      2/13*a^2 + 1/13*a - 755/13,
      1/13*a^2 - 19/13*a - 7/13],
     [(Fractional ideal (11, a - 2), 2), (Fractional ideal (19, a + 7), 2)])
Got:
    ([2/13*a^2 + 1/13*a - 677/13,
      1/13*a^2 + 7/13*a - 332/13,
      -1/13*a^2 + 6/13*a + 345/13,
      -1,
      2/13*a^2 + 1/13*a - 755/13,
      1/13*a^2 + 20/13*a - 7/13],
     [(Fractional ideal (11, a - 2), 2), (Fractional ideal (19, a + 7), 2)])
**********************************************************************
File "/nix/store/vqiiwb9rzmzh97gzx1wmvqyiy9d5qhxj-sage-src-8.9/src/sage/rings/number_field/number_field.py", line 4527, in sage.rings.number_field.number_field.NumberField_generic.selmer_group
Failed example:
    K.selmer_group(K.primes_above(13), 2)
Expected:
    [2/13*a^2 + 1/13*a - 677/13,
     1/13*a^2 + 7/13*a - 332/13,
     -1/13*a^2 + 6/13*a + 345/13,
     -1,
     2/13*a^2 + 1/13*a - 755/13,
     1/13*a^2 - 19/13*a - 7/13,
     -1/13*a^2 + 45/13*a - 97/13,
     2/13*a^2 + 40/13*a - 27/13]
Got:
    [2/13*a^2 + 1/13*a - 677/13,
     1/13*a^2 + 7/13*a - 332/13,
     -1/13*a^2 + 6/13*a + 345/13,
     -1,
     2/13*a^2 + 1/13*a - 755/13,
     1/13*a^2 + 20/13*a - 7/13,
     1/13*a^2 - 45/13*a + 97/13,
     -2/13*a^2 - 40/13*a + 27/13]
**********************************************************************
File "/nix/store/vqiiwb9rzmzh97gzx1wmvqyiy9d5qhxj-sage-src-8.9/src/sage/rings/number_field/number_field.py", line 5179, in sage.rings.number_field.number_field.NumberField_generic.elements_of_norm
Failed example:
    K.elements_of_norm(7)
Expected:
    [7/225*a^2 - 7/75*a - 42/25]
Got:
    [28/225*a^2 + 77/75*a - 133/25]
**********************************************************************
File "/nix/store/vqiiwb9rzmzh97gzx1wmvqyiy9d5qhxj-sage-src-8.9/src/sage/rings/number_field/number_field.py", line 5282, in sage.rings.number_field.number_field.NumberField_generic.factor
Failed example:
    f = L.factor(a + 1); f
Expected:
    (Fractional ideal (1/2*a*b - a + 1/2)) * (Fractional ideal (-1/2*a*b - a + 1/2))
Got:
    (Fractional ideal (-1/2*b + 1/2*a + 1)) * (Fractional ideal (-1/2*a*b - a + 1/2))
**********************************************************************
File "/nix/store/vqiiwb9rzmzh97gzx1wmvqyiy9d5qhxj-sage-src-8.9/src/sage/rings/number_field/number_field.py", line 6359, in sage.rings.number_field.number_field.NumberField_generic.uniformizer
Failed example:
    [K.uniformizer(P) for P,e in factor(K.ideal(7))]
Expected:
    [t^2 + 3*t + 1]
Got:
    [t^2 - 4*t + 1]
**********************************************************************
File "/nix/store/vqiiwb9rzmzh97gzx1wmvqyiy9d5qhxj-sage-src-8.9/src/sage/rings/number_field/number_field.py", line 6615, in sage.rings.number_field.number_field.NumberField_generic.S_unit_group
Failed example:
    U.log(u)
Expected:
    (1, 1, 4, 1, 5)
Got:
    (0, 1, 4, 1, 5)
**********************************************************************
File "/nix/store/vqiiwb9rzmzh97gzx1wmvqyiy9d5qhxj-sage-src-8.9/src/sage/rings/number_field/number_field.py", line 7599, in sage.rings.number_field.number_field.NumberField_absolute.optimized_representation
Failed example:
    K.optimized_representation()
Expected:
    (Number Field in a1 with defining polynomial x^3 - 7*x - 7,
     Ring morphism:
       From: Number Field in a1 with defining polynomial x^3 - 7*x - 7
       To:   Number Field in a with defining polynomial 7/9*x^3 + 7/3*x^2 - 56*x + 123
       Defn: a1 |--> 7/225*a^2 - 7/75*a - 42/25,
     Ring morphism:
       From: Number Field in a with defining polynomial 7/9*x^3 + 7/3*x^2 - 56*x + 123
       To:   Number Field in a1 with defining polynomial x^3 - 7*x - 7
       Defn: a |--> -15/7*a1^2 + 9)
Got:
    (Number Field in a1 with defining polynomial x^3 - 7*x - 7, Ring morphism:
       From: Number Field in a1 with defining polynomial x^3 - 7*x - 7
       To:   Number Field in a with defining polynomial 7/9*x^3 + 7/3*x^2 - 56*x + 123
       Defn: a1 |--> 28/225*a^2 + 77/75*a - 133/25, Ring morphism:
       From: Number Field in a with defining polynomial 7/9*x^3 + 7/3*x^2 - 56*x + 123
       To:   Number Field in a1 with defining polynomial x^3 - 7*x - 7
       Defn: a |--> -60/7*a1^2 + 15*a1 + 39)
**********************************************************************
8 items had failures:
   1 of  10 in sage.rings.number_field.number_field.NumberField_absolute.optimized_representation
   1 of  27 in sage.rings.number_field.number_field.NumberField_generic.S_unit_group
   1 of   9 in sage.rings.number_field.number_field.NumberField_generic._S_class_group_and_units
   1 of   6 in sage.rings.number_field.number_field.NumberField_generic.elements_of_norm
   1 of  15 in sage.rings.number_field.number_field.NumberField_generic.factor
   1 of   4 in sage.rings.number_field.number_field.NumberField_generic.ideals_of_bdd_norm
   1 of  32 in sage.rings.number_field.number_field.NumberField_generic.selmer_group
   1 of  20 in sage.rings.number_field.number_field.NumberField_generic.uniformizer
    [2076 tests, 8 failures, 52.52 s]
----------------------------------------------------------------------
sage -t --long /nix/store/vqiiwb9rzmzh97gzx1wmvqyiy9d5qhxj-sage-src-8.9/src/sage/rings/number_field/number_field.py  # 8 doctests failed

I will not do the update myself in this ticket, I only opened it to notify people of the issues.

Tarfile: see checksums_ini [upstream_url]

Change History (38)

comment:1 follow-up: Changed 2 years ago by arojas

Already updated on Arch. Some of the failures are actual incorrect results which are due to a pari bug fixed in [1]. The remaining ones are due to changes in chosen ideal generators, and in computation precision. Those just need the doctests to be updated.

[1] https://pari.math.u-bordeaux.fr/cgi-bin/gitweb.cgi?p=pari.git;a=commitdiff;h=c7a1d35f382e96ddf14694be27a0ca5746880700

Will post a branch when I have some free time unless someone beats me to it.

comment:2 Changed 2 years ago by gh-timokau

  • Cc thansen chapoton embray jdemeyer added

I just now noticed #28424 (missed it initially because I searched for pari 2.11.3 in particular). That ticket proposes an update to 2.12 though, which is still in beta and probably harder to do. So I think we can keep this open.

comment:3 in reply to: ↑ 1 Changed 2 years ago by gh-timokau

Replying to arojas:

Already updated on Arch. Some of the failures are actual incorrect results which are due to a pari bug fixed in [1]. The remaining ones are due to changes in chosen ideal generators, and in computation precision. Those just need the doctests to be updated.

[1] https://pari.math.u-bordeaux.fr/cgi-bin/gitweb.cgi?p=pari.git;a=commitdiff;h=c7a1d35f382e96ddf14694be27a0ca5746880700

Will post a branch when I have some free time unless someone beats me to it.

Thats great news, amazing how fast you always come up with those fixes :) For anyone else wondering, here is Antonio's patch.

comment:4 Changed 2 years ago by arojas

  • Cc dimpase added

Since Sage can now use system pari, perhaps 2.11.3 should be blacklisted until this is merged. Ideally, distros shipping 2.11.3 should backport the patch as it is quite a serious bug.

comment:5 Changed 2 years ago by gh-timokau

Oh, I misread your original comment and I thought that you meant the tests were wrong before and 2.11.3 *fixes* the bug. I'll backport the pari fix for nix.

comment:6 follow-up: Changed 2 years ago by dimpase

Pari 2.11.3 has been released just few days ago, and that bug was fixed last year. Could it be that upstream forgot to include the fix?

What's an example of incorrect maths output without that patch?

Last edited 2 years ago by dimpase (previous) (diff)

comment:7 in reply to: ↑ 6 Changed 2 years ago by arojas

Replying to dimpase:

Pari 2.11.3 has been released just few days ago, and that bug was fixed last year. Could it be that upstream forgot to include the fix?

Yes, they forgot to backport the fix to stable. See [1]

What's an example of incorrect maths output without that patch?

You have some examples in [1] and [2]

[1] https://pari.math.u-bordeaux.fr/cgi-bin/bugreport.cgi?bug=2208

[2] https://pari.math.u-bordeaux.fr/cgi-bin/bugreport.cgi?bug=2164

This causes a few failures in Sage. Among other things, simon's scripts are totally broken.

Last edited 2 years ago by arojas (previous) (diff)

comment:8 follow-up: Changed 2 years ago by gh-timokau

When patching pari with [1] and sage with [2] I still get at least one doctest failure (I only tested the number_fields.py file for now):

sage -t --long /nix/store/nsvlj78rfc0s8077drna9xw54xw2z50v-sage-src-8.9/src/sage/rings/number_field/number_field.py
**********************************************************************
File "/nix/store/nsvlj78rfc0s8077drna9xw54xw2z50v-sage-src-8.9/src/sage/rings/number_field/number_field.py", line 6615, in sage.rings.number_field.number_field.NumberField_generic.S_unit_group
Failed example:
    U.log(u)
Expected:
    (1, 1, 4, 1, 5)
Got:
    (0, 1, 4, 1, 5)
**********************************************************************
1 item had failures:
   1 of  27 in sage.rings.number_field.number_field.NumberField_generic.S_unit_group
    [2076 tests, 1 failure, 54.23 s]

Am I missing something?

[1] https://pari.math.u-bordeaux.fr/cgi-bin/gitweb.cgi?p=pari.git;a=patch;h=c7a1d35f382e96ddf14694be27a0ca5746880700

[2] https://aur.archlinux.org/cgit/aur.git/plain/sagemath-pari-2.11.3.patch?h=sagemath-git&id=02e1d58bd1cd70935d69a4990469d18be6bd2c43

Last edited 2 years ago by gh-timokau (previous) (diff)

comment:9 Changed 2 years ago by fbissey

Given that this seem to be a serious bug, can we expect upstream to release 2.11.4 shortly to fix it?

comment:10 in reply to: ↑ 8 Changed 2 years ago by arojas

Replying to gh-timokau:

When patching pari with [1] and sage with [2] I still get at least one doctest failure (I only tested the number_fields.py file for now):

sage -t --long /nix/store/nsvlj78rfc0s8077drna9xw54xw2z50v-sage-src-8.9/src/sage/rings/number_field/number_field.py
**********************************************************************
File "/nix/store/nsvlj78rfc0s8077drna9xw54xw2z50v-sage-src-8.9/src/sage/rings/number_field/number_field.py", line 6615, in sage.rings.number_field.number_field.NumberField_generic.S_unit_group
Failed example:
    U.log(u)
Expected:
    (1, 1, 4, 1, 5)
Got:
    (0, 1, 4, 1, 5)
**********************************************************************
1 item had failures:
   1 of  27 in sage.rings.number_field.number_field.NumberField_generic.S_unit_group
    [2076 tests, 1 failure, 54.23 s]

Am I missing something?

That's one of the failures that should be fixed with pari's patch, are you sure you're running the patched version?

comment:11 Changed 2 years ago by gh-timokau

No, I wasn't. I had a typo ("patch" instead of "patches") that resulted in the patch silently being ignored. Sorry for the noise.

As an aside, the upstream patch doesn't quite apply cleanly but the arch patch [1] is rebased. Thanks!

[1] https://git.archlinux.org/svntogit/community.git/plain/repos/community-x86_64/c7a1d35f.patch?h=packages/pari&id=27893d227290dc3821d68aa25877d9765c204dad

comment:12 Changed 2 years ago by arojas

  • Branch set to u/arojas/upgrade_to_pari_2_11_3

comment:13 Changed 2 years ago by arojas

  • Authors set to Antonio Rojas
  • Commit set to ce42f11db5d1d31fee916013492d69d56a6572c5
  • Status changed from new to needs_review

Needs testing for the check_functional_equation() output on 32 bits, i've only checked 64 bits. Also, some elliptic curves expert should check if the changes in elliptic_curves/ell_number_field.py are mathematically correct. All other ones seem OK to me.


New commits:

4ba1ea8Update pari to 2.11.3
d934a82Remove wrong patch
ce42f11Adjust doctests for new output with pari 2.11.3

comment:14 follow-up: Changed 2 years ago by gh-kliem

The way I understand the configuration, once this ticket is through, it will reject a system pari unless it is at least version 2.11.3. Is this correct?

comment:15 in reply to: ↑ 14 Changed 2 years ago by arojas

Replying to gh-kliem:

The way I understand the configuration, once this ticket is through, it will reject a system pari unless it is at least version 2.11.3. Is this correct?

No, the minimum pari system version is defined in spkg-configure and is independent of the version bundled in sage

comment:16 follow-up: Changed 2 years ago by gh-kliem

Ok, then I get why you want sage's test to be work with pari 2.11.3 and previous versions.

Do you want this to be done in this ticket or in a seperate ticket?

comment:17 in reply to: ↑ 16 Changed 2 years ago by arojas

Replying to gh-kliem:

Ok, then I get why you want sage's test to be work with pari 2.11.3 and previous versions.

Do you want this to be done in this ticket or in a seperate ticket?

I don't really mind as long as I can get rid of my downstream patch. Feel free to take over this branch and modify the test results so they pass with both versions.

comment:18 Changed 2 years ago by gh-kliem

  • Dependencies set to #29472

comment:19 Changed 2 years ago by git

  • Commit changed from ce42f11db5d1d31fee916013492d69d56a6572c5 to 507576f492419feff2c567988c11fb1abddc648f

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

507576fUpdate to 2.11.4

comment:20 Changed 2 years ago by arojas

  • Summary changed from Upgrade to pari 2.11.3 to Upgrade to pari 2.11.4

comment:21 Changed 2 years ago by mkoeppe

  • Milestone changed from sage-9.1 to sage-9.2

comment:22 Changed 2 years ago by dimpase

  • Reviewers set to Dima Pasechnik
  • Status changed from needs_review to needs_work

please rebase this over the branch of #29472, as we have a conflict. (perhaps, better, rebase both over the recent beta, too)

comment:23 Changed 2 years ago by git

  • Commit changed from 507576f492419feff2c567988c11fb1abddc648f to 4a23a123580410bca14cfd89d1a22ac21bd094ab

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

4a23a12Merge branch 'develop' of git://git.sagemath.org/sage into t/29313/upgrade_to_pari_2_11_3

comment:24 Changed 2 years ago by git

  • Commit changed from 4a23a123580410bca14cfd89d1a22ac21bd094ab to dfcdcb0fab6173ab598745e72706b0efa43f2e86

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

dfcdcb0Drop test fixes, handled in #29472

comment:25 Changed 2 years ago by dimpase

please also apply

  • build/pkgs/pari/checksums.ini

    a b tarball=pari-VERSION.tar.gz 
    22sha1=2b9ff51feb388664b834dc346a44867546c78618
    33md5=fb2968d7805424518fe44a59a2024afd
    44cksum=1247903778
     5upstream_url=https://pari.math.u-bordeaux.fr/pub/pari/unix/pari-2.11.4.tar.gz

comment:26 Changed 2 years ago by git

  • Commit changed from dfcdcb0fab6173ab598745e72706b0efa43f2e86 to 549e094174b9fb0b2fc5b7462d034f118e3fd1f3

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

549e094Add upstream url

comment:27 Changed 2 years ago by dimpase

  • Description modified (diff)

comment:28 follow-up: Changed 2 years ago by dimpase

  • Status changed from needs_work to needs_review

some tests on macOS and Cygwin would be great. Perhaps adapting GH Actions to restrict platforms...

comment:29 in reply to: ↑ 28 Changed 2 years ago by mkoeppe

Replying to dimpase:

Perhaps adapting GH Actions to restrict platforms...

There's a ticket for that (#29535 - Customize CI workflows on GitHub Actions using specially structured tag names)

comment:30 follow-up: Changed 2 years ago by dimpase

for some reason I don't get macOS runs on GH Actions any more. Is it by design, or GH pulled them?

comment:31 in reply to: ↑ 30 Changed 2 years ago by mkoeppe

Replying to dimpase:

for some reason I don't get macOS runs on GH Actions any more. Is it by design, or GH pulled them?

No, works just fine for me -- for example here: https://github.com/mkoeppe/sage/actions/runs/141135018

comment:32 Changed 2 years ago by mkoeppe

You probably need to cancel some of your old queued workflows such as this one: https://github.com/dimpase/sage/runs/784000040?check_suite_focus=true

comment:33 Changed 22 months ago by mkoeppe

What's missing on this ticket?

comment:34 follow-up: Changed 22 months ago by dimpase

basically, someone should bite the bullet and make pari-related doctests less rigid (cf. e.g. the 1st example in the ticket description - it assumes an output order which is not really fixed),so that different versions of pari would still pass.

comment:35 in reply to: ↑ 34 Changed 22 months ago by arojas

Replying to dimpase:

basically, someone should bite the bullet and make pari-related doctests less rigid (cf. e.g. the 1st example in the ticket description - it assumes an output order which is not really fixed),so that different versions of pari would still pass.

That's done in #29472 already

comment:36 Changed 22 months ago by dimpase

ah, ok, then in principle it should be good to go.

comment:37 Changed 22 months ago by fbissey

  • Reviewers changed from Dima Pasechnik to Dima Pasechnik, François Bissey
  • Status changed from needs_review to positive_review

Been using it for a little while in sage-on-gentoo and I don't need any patches for doctests anymore for some time now. So, I am putting this to positive.

Last edited 22 months ago by fbissey (previous) (diff)

comment:38 Changed 22 months ago by vbraun

  • Branch changed from u/arojas/upgrade_to_pari_2_11_3 to 549e094174b9fb0b2fc5b7462d034f118e3fd1f3
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.