Opened 2 years ago

Closed 22 months ago

## #29313 closed enhancement (fixed)

Reported by: Owned by: gh-timokau major sage-9.2 packages: standard arojas, fbissey, saraedum, thansen, chapoton, embray, jdemeyer, dimpase Antonio Rojas Dima Pasechnik, François Bissey N/A 549e094 549e094174b9fb0b2fc5b7462d034f118e3fd1f3 #29472

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)
**********************************************************************
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]

### comment:1 follow-up: ↓ 3 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.

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

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.

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

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

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]

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: ↓ 10 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 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?

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

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 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!

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

 ​4ba1ea8 Update pari to 2.11.3 ​d934a82 Remove wrong patch ​ce42f11 Adjust doctests for new output with pari 2.11.3

### comment:14 follow-up: ↓ 15 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

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: ↓ 17 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

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:

 ​507576f Update 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:

 ​4a23a12 Merge 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:

 ​dfcdcb0 Drop test fixes, handled in #29472

• ## build/pkgs/pari/checksums.ini

 a tarball=pari-VERSION.tar.gz sha1=2b9ff51feb388664b834dc346a44867546c78618 md5=fb2968d7805424518fe44a59a2024afd cksum=1247903778 upstream_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:

### comment:27 Changed 2 years ago by dimpase

• Description modified (diff)

### comment:28 follow-up: ↓ 29 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

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: ↓ 31 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

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: ↓ 35 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

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