Upgrade: pari 2.13 - Githubissues

slel commented 4 years ago

This is to upgrade to PARI 2.13.x. https://repology.org/project/pari/versions

This new PARI release brings a lot of bug fixes, new functionality, and speedups.

pari-2.13 release announcement

Changes in the PARI library that needs adaptation in SageMath code

 34- bnfissunit is obsolete, use bnfisunit
 35- bnfsunit is mostly obsolete, use bnfunits
 36- bnfisunit and bnfissunit: torsion unit exponent is now a t_INT (used
     to be a t_INTMOD)
 1- Removed member functions .futu and .tufu [deprecated since 2.2], used in simon two descent scripts.
 56- zetamultall: add flag

Last upgrade:

29313 (Upgrade: pari 2.11.4), merged in Sage 9.2.beta7 (released 2020-08-03)

Follow-up ticket: #31754

Upstream: Fixed upstream, but not in a stable release.

CC: @dimpase @orlitzky @mkoeppe @slel @antonio-rojas @kiwifb @videlec @tobihan @collares @isuruf @dkwo @xcaruso @loefflerd @kliem

Component: packages: standard

Keywords: upgrade, pari

Author: Vincent Delecroix, Antonio Rojas, Gonzalo Tornaría

Branch: c78b147

Reviewer: Dima Pasechnik, David Loeffler

Issue created by migration from https://trac.sagemath.org/ticket/30801

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

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

86980a3 fix output of simon_two_descent

antonio-rojas commented 3 years ago

comment:118

There's still one simon-two-descent related failure in schemes/elliptic_curves/ell_number_field.py - I guess you didn't run --long tests?

File "/usr/lib/python3.9/site-packages/sage/schemes/elliptic_curves/ell_number_field.py", line 270, in sage.schemes.elliptic_curves.ell_number_field.EllipticCurve_number_field.?
Failed example:
    E.simon_two_descent()  # long time (4s on sage.math, 2013)
Expected:
    (3,
     3,
     [(0 : 0 : 1),
      (-1/2*zeta43_0^2 - 1/2*zeta43_0 + 7 : -3/2*zeta43_0^2 - 5/2*zeta43_0 + 18 : 1)...)
Got:
    (3,
     3,
     [(5/8*zeta43_0^2 + 17/8*zeta43_0 - 9/4 : -27/16*zeta43_0^2 - 103/16*zeta43_0 + 39/8 : 1),
      (0 : 0 : 1)])

Also, spkg-configure.m4 needs updating as per comment:76 to reject pari <2.13. Do we want to also add a check for the rnfdisc fix?

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

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

860a1ea one long doctest

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from 86980a3 to 860a1ea

videlec commented 3 years ago

comment:120

I will fix spkg-configure.m4 and add check for rnfdisc.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

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

558b5e8 rewrite spkg-configure.m4

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from 860a1ea to 558b5e8

videlec commented 3 years ago

Description changed:

--- 
+++ 
@@ -19,3 +19,5 @@
 Last upgrade:

 - #29313 (Upgrade: pari 2.11.4), merged in Sage 9.2.beta7 (released 2020-08-03)
+
+Follow-up ticket: #31754

dimpase commented 3 years ago

comment:124

I'm getting

sage -t --random-seed=0 src/sage/rings/number_field/number_field_rel.py
**********************************************************************
File "src/sage/rings/number_field/number_field_rel.py", line 2251, in sage.rings.number_field.number_field_rel.NumberField_relative.relative_discriminant
Failed example:
    K.relative_discriminant() == F.ideal(4*b)
Expected:
    True
Got:
    False

not 100% sure it's related to this ticket, though.

collares commented 3 years ago

comment:125

This ticket adds a Pari patch in build/pkgs/pari to fix https://pari.math.u-bordeaux.fr/cgi-bin/bugreport.cgi?bug=2284, which causes the failure above.

dimpase commented 3 years ago

comment:126

Ah, OK, this explains - I'm trying this with 2.13.1 from Gentoo. So this means it's not detected by spkg-configure,m4, this bug.

dimpase commented 3 years ago

comment:127

add a test for the latest patch in spkg-configure

dimpase commented 3 years ago

Reviewer: Dima Pasechnik

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from 558b5e8 to f014e79

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

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

`b0814bd`	`Merge branch 'develop' of https://github.com/sagemath/sage into t/30801/public/30801`
`f014e79`	`Install pari spkg if test for rnfdisc bug fix fails`

dimpase commented 3 years ago

comment:130

lgtm.

Francois, by the way, could you add the latest patch (build/pkgs/pari/patches/pari-rnfdisc.patch), not in 2.13.1, to Gentoo's pari?

kiwifb commented 3 years ago

comment:131

Replying to @dimpase:

lgtm.

Francois, by the way, could you add the latest patch (build/pkgs/pari/patches/pari-rnfdisc.patch), not in 2.13.1, to Gentoo's pari?

That would be @orlitzky's job :) but I guess I can make a PR for it.

orlitzky commented 3 years ago

comment:132

The current patch is missing the changes to the test cases:

https://pari.math.u-bordeaux.fr/cgi-bin/gitweb.cgi?p=pari.git;a=commitdiff;h=3edb98db78dd49bb8b4137b46781a7cd570c2556

Won't that affect SAGE_CHECK=y?

I am also required to complain about copy/pasting a test into spkg-configure.m4 that will reject pari-2.13.1 for having this one particular bug but not any of the other 47 bugs that have been fixed since: https://pari.math.u-bordeaux.fr/cgi-bin/gitweb.cgi?p=pari.git;a=blob;f=CHANGES

The cost/benefit of using the system package jumps when you go from zero custom patches to one. We should allow people to proceed with the upstream version if that's what they have installed.

(Having completed my rant, I will still add this to the Gentoo package)

dimpase commented 3 years ago

comment:133

2.13 broke a test in Sage, and it was not fixed in 2.13.1, but only later. No changes in Sage needed.

Yes, this points at poor quality of Pari's own testsuite.

you can force Sage to use system version via a configure option.

dimpase commented 3 years ago

comment:134

good point about SAGE_CHECK.

dimpase commented 3 years ago

comment:136

and of course the full patch does not apply cleanly to 2.13.1, grrr...

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

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

10a4531 Fix pari test suite after the rnfdisc fix

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from f014e79 to 10a4531

collares commented 3 years ago

comment:139

I think src/test/in/rnf needs to be added to the PARI patch too (like in https://github.com/NixOS/nixpkgs/blob/fb58ee7fd4baa7d49bdb8de8662d6ad20bf3bd19/pkgs/applications/science/math/pari/rnfdisc.patch).

collares commented 3 years ago

comment:140

Ah, nevermind, I see you did remove the new test's output. My bad.

dimpase commented 3 years ago

comment:141

ok, this works for me.

vbraun commented 3 years ago

comment:142

On 32-bit:

**********************************************************************
File "src/sage/libs/pari/__init__.py", line 165, in sage.libs.pari
Failed example:
    eta1.sage()
Expected:
    3.6054636014326520859158205642077267748
Got:
    3.605463601432652085915820564
**********************************************************************
File "src/sage/libs/pari/__init__.py", line 168, in sage.libs.pari
Failed example:
    eta1.sage()
Expected:
    3.605463601432652085915820564207726774810268996598024745444380641429820491740
Got:
    3.60546360143265208591582056420772677481026899659802474544
**********************************************************************
1 item had failures:
   2 of  41 in sage.libs.pari
    [40 tests, 2 failures, 0.02 s]
**********************************************************************
File "src/sage/libs/pari/tests.py", line 1764, in sage.libs.pari.tests
Failed example:
    eta1.sage()
Expected:
    3.6054636014326520859158205642077267748
Got:
    3.605463601432652085915820564
**********************************************************************
File "src/sage/libs/pari/tests.py", line 1767, in sage.libs.pari.tests
Failed example:
    eta1.sage()
Expected:
    3.605463601432652085915820564207726774810268996598024745444380641429820491740
Got:
    3.60546360143265208591582056420772677481026899659802474544
**********************************************************************
1 item had failures:
   2 of 869 in sage.libs.pari.tests
    [868 tests, 2 failures, 1.54 s]
**********************************************************************

and

**********************************************************************
File "src/sage/rings/padics/relaxed_template.pxi", line 2640, in sage.rings.padics.relaxed_template.RelaxedElement_random.__reduce__
Failed example:
    a[:30]
Expected:
    ...?343214211432220241412003314311
Got:
    ...?311111122142113024041330312403
**********************************************************************
File "src/sage/rings/padics/relaxed_template.pxi", line 2642, in sage.rings.padics.relaxed_template.RelaxedElement_random.__reduce__
Failed example:
    b[:30]
Expected:
    ...?343214211432220241412003314311
Got:
    ...?311111122142113024041330312403
**********************************************************************

dimpase commented 3 years ago

comment:144

tests in src/sage/rings/padics/relaxed_template.pxi were added by @xcaruso very recently. Perhaps he can comment whether this is a bug.

the other 32-bit errors are obviously just numerical noise, I'll fix these.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from 10a4531 to 0bba178

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

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

`fb5a9a4`	`Merge remote-tracking branch 'trac/develop' into public/30801`
`0bba178`	`fix some 32 vs 64 bit discrepancies in tests`

videlec commented 3 years ago

comment:146

Replying to @vbraun:

On 32-bit:

**********************************************************************
File "src/sage/rings/padics/relaxed_template.pxi", line 2640, in sage.rings.padics.relaxed_template.RelaxedElement_random.__reduce__
Failed example:
    a[:30]
Expected:
    ...?343214211432220241412003314311
Got:
    ...?311111122142113024041330312403
**********************************************************************
File "src/sage/rings/padics/relaxed_template.pxi", line 2642, in sage.rings.padics.relaxed_template.RelaxedElement_random.__reduce__
Failed example:
    b[:30]
Expected:
    ...?343214211432220241412003314311
Got:
    ...?311111122142113024041330312403
**********************************************************************

Printing a random element in a doctest is usually not what is intended to be tested. As explained in the documentation the above code is supposed to test whether a[:30] and b[:30] are equal.

videlec commented 3 years ago

comment:147

Moreover, few lines above in the code, the tests involve a # random tag...

xcaruso commented 3 years ago

comment:148

Oops sorry, a tag #random was missing; it's actually weird that this test used to pass earlier.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from 0bba178 to 32b52cc

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

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

32b52cc add tag #random

dimpase commented 3 years ago

comment:150

Replying to @xcaruso:

Oops sorry, a tag #random was missing; it's actually weird that this test used to pass earlier.

hmm, so somehow 32-bit vs 64-bit rngs now behave differently?

It passed before on with whatever value you put in, as it's pseudo-random, and as long as you didn't change the rng seed, it should be the same.

dimpase commented 3 years ago

comment:151

IMHO the difference in RNG values on different platforms here may be ignored.

videlec commented 3 years ago

comment:152

Replying to @xcaruso:

Oops sorry, a tag #random was missing; it's actually weird that this test used to pass earlier.

Don't you want to actually compare the values of a[:30] and b[:30]? Adding the # random tag actually makes the test pass in any scenario.

xcaruso commented 3 years ago

comment:153

Well, the test a == b below does this checking (precisely, equality is checked at the default halting precision of the parent, which is 40 in the example).

antonio-rojas commented 3 years ago

comment:154

Many new test failures (and a merge conflict) caused by #26635

sage -t --long --random-seed=0 src/sage/modular/local_comp/local_comp.py  # 7 doctests failed
sage -t --long --random-seed=0 src/sage/modular/local_comp/smoothchar.py  # 11 doctests failed

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

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

`bf5a59a`	`Merge branch 'develop' of https://github.com/sagemath/sage into t/30801/public/30801`
`316d62f`	`Merge branch 'public/30801' of git://trac.sagemath.org/sage into t/30801/public/30801`

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from 32b52cc to 316d62f

dimpase commented 3 years ago

comment:156

It's not hard to fix the tests to pass on pari 2.13, but it's much harder to make them also work for 2.11 (do we need ther latter?) Here is the diff for the most affected file (only the 1st test is made version-free)

diff --git a/src/sage/modular/local_comp/smoothchar.py b/src/sage/modular/local_comp/smoothchar.py
index e54d5ad1d4..92135f7f71 100644
--- a/src/sage/modular/local_comp/smoothchar.py
+++ b/src/sage/modular/local_comp/smoothchar.py
@@ -1131,9 +1131,8 @@ class SmoothCharacterGroupQuadratic(SmoothCharacterGroupGeneric):
             sage: from sage.modular.local_comp.smoothchar import SmoothCharacterGroupRamifiedQuadratic
             sage: G = SmoothCharacterGroupRamifiedQuadratic(3, 1, QQ)
             sage: s = G.number_field().gen()
-            sage: G.discrete_log(4, 3 + 2*s)
-            [1, 2, 2, 1]
-            sage: gs = G.unit_gens(4); gs[0] * gs[1]^2 * gs[2]^2 * gs[3] - (3 + 2*s) in G.ideal(4)
+            sage: dl = G.discrete_log(4, 3 + 2*s)
+            sage: gs = G.unit_gens(4); gs[0]^dl[0] * gs[1]^dl[1] * gs[2]^dl[2] * gs[3]^dl[3] - (3 + 2*s) in G.ideal(4)
             True

         An example with a custom generating set::
@@ -1191,11 +1190,11 @@ class SmoothCharacterGroupQuadratic(SmoothCharacterGroupGeneric):
             sage: from sage.modular.local_comp.smoothchar import SmoothCharacterGroupUnramifiedQuadratic
             sage: G = SmoothCharacterGroupUnramifiedQuadratic(7,QQ)
             sage: G.quotient_gens(1)
-            [s]
+            [2*s - 2]
             sage: G.quotient_gens(2)
-            [23*s - 16]
+            [15*s + 21]
             sage: G.quotient_gens(3)
-            [-124*s - 2]
+            [-75*s + 33]

         A ramified case::

@@ -1208,11 +1207,11 @@ class SmoothCharacterGroupQuadratic(SmoothCharacterGroupGeneric):

             sage: G = SmoothCharacterGroupUnramifiedQuadratic(2,QQ)
             sage: G.quotient_gens(1)
-            [s]
+            [s + 1]
             sage: G.quotient_gens(2)
-            [-s + 1]
+            [-s + 2]
             sage: G.quotient_gens(3)
-            [7*s + 5, -s + 3]
+            [-17*s - 14, 3*s - 2]
         """

         # silly special case
@@ -1327,18 +1326,18 @@ class SmoothCharacterGroupQuadratic(SmoothCharacterGroupGeneric):
             sage: G.extend_character(3, chi, [1])
             Character of unramified extension Q_5(s)* (s^2 + 4*s + 2 = 0), of level 0, mapping 5 |--> 7
             sage: K.<z> = CyclotomicField(6); G.base_extend(K).extend_character(1, chi, [z])
-            Character of unramified extension Q_5(s)* (s^2 + 4*s + 2 = 0), of level 1, mapping s |--> z, 5 |--> 7
+            Character of unramified extension Q_5(s)* (s^2 + 4*s + 2 = 0), of level 1, mapping s |--> -z + 1, 5 |--> 7

         We extend the nontrivial quadratic character::

             sage: chi = SmoothCharacterGroupQp(5, QQ).character(1, [-1, 7])
             sage: K.<z> = CyclotomicField(24); G.base_extend(K).extend_character(1, chi, [z^6])
-            Character of unramified extension Q_5(s)* (s^2 + 4*s + 2 = 0), of level 1, mapping s |--> z^6, 5 |--> 7
+            Character of unramified extension Q_5(s)* (s^2 + 4*s + 2 = 0), of level 1, mapping s |--> -z^6, 5 |--> 7

         Extensions of higher level::

             sage: K.<z> = CyclotomicField(20); rho = G.base_extend(K).extend_character(2, chi, [z]); rho
-            Character of unramified extension Q_5(s)* (s^2 + 4*s + 2 = 0), of level 2, mapping 11*s - 10 |--> -z^5, 6 |--> 1, 5*s + 1 |--> z^4, 5 |--> 7
+            Character of unramified extension Q_5(s)* (s^2 + 4*s + 2 = 0), of level 2, mapping 11*s - 10 |--> z^5, 6 |--> 1, 5*s + 1 |--> z^4, 5 |--> 7
             sage: rho(3)
             -1

dimpase commented 3 years ago

comment:157

There are also failures in src/sage/modular/local_comp/local_comp.py, the 1st two trivial (different ordering of a list), but the following two look rather suspect to me:

File "src/sage/modular/local_comp/local_comp.py", line 656, in sage.modular.local_comp.local_comp.PrimitiveSupercuspidal.characters
Failed example:
    Pi.characters()
Expected:
    [
    Character of unramified extension Q_5(s)* (s^2 + 4*s + 2 = 0), of level 1, mapping s |--> d, 5 |--> 5,
    Character of unramified extension Q_5(s)* (s^2 + 4*s + 2 = 0), of level 1, mapping s |--> -d - 1/3*j0^3, 5 |--> 5
    ]
Got:
    [
    Character of unramified extension Q_5(s)* (s^2 + 4*s + 2 = 0), of level 1, mapping s |--> 1/3*j0^2*d - 1/3*j0^3, 5 |--> 5,
    Character of unramified extension Q_5(s)* (s^2 + 4*s + 2 = 0), of level 1, mapping s |--> -1/3*j0^2*d, 5 |--> 5
    ]
**********************************************************************
File "src/sage/modular/local_comp/local_comp.py", line 661, in sage.modular.local_comp.local_comp.PrimitiveSupercuspidal.characters
Failed example:
    Pi.characters()[0].base_ring()
Expected:
    Number Field in d with defining polynomial x^2 + 1/3*j0^3*x - 1/3*j0^2 over its base field
Got:
    Number Field in d with defining polynomial x^2 - j0*x + 1/3*j0^2 over its base field

David, can you have a look?

11d1fc49-71a1-44e1-869f-76be013245a0 commented 3 years ago

comment:158

Looks harmless to me. I gather from the discussions above that the default generators for multiplicative groups of non-prime finite fields have been changed, and that would inevitably lead to changes in the output of these doctests (without affecting the mathematical validity of the calculations).

dimpase commented 3 years ago

comment:159

Replying to @loefflerd:

Looks harmless to me. I gather from the discussions above that the default generators for multiplicative groups of non-prime finite fields have been changed, and that would inevitably lead to changes in the output of these doctests (without affecting the mathematical validity of the calculations).

right under these two tests one has

        .. warning::

            The above output isn't actually the same as in Example 2 of
            [LW2012]_, due to an error in the published paper (correction
            pending) -- the published paper has the inverses of the above
            characters.

I didn't look at the paper cited there - but presumably you know what's going on. And I imagine it won't be "the inverses of the above characters" any more.

11d1fc49-71a1-44e1-869f-76be013245a0 commented 3 years ago

comment:160

The characters returned by the doctest are the same characters as they were before -- just notated differently. So the warning is no more or less problematic than it was before.

dimpase commented 3 years ago

comment:161

OK, thanks, we'll be able to proceed then.

dimpase commented 3 years ago

comment:162

as we bump the Pari version to 2.13+, we should remove tests for bugs fixed earlier.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

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

`da40045`	`doctest fixes for #26635 on Pari 2.13`
`73591c0`	`do not test for bugs fixed in pari 2.13+`
`4a9c3b6`	`remove space between 2 and >> in script`

sagemath / sage

Upgrade: pari 2.13 #30801

29313 (Upgrade: pari 2.11.4), merged in Sage 9.2.beta7 (released 2020-08-03)