sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.44k stars 481 forks source link

"impossible inverse" error in PARI's ellgroup() #34900

Closed mkoeppe closed 1 year ago

mkoeppe commented 1 year ago

As reported in #34537 comment:145, the following code fails with a PariError:

sage: GF(41^16).inject_variables()
Defining z16
sage: E = EllipticCurve([z16, 1])
sage: E.gens()
# ...
PariError: impossible inverse in Fl_inv: Mod(31, 31)

Upstream bug: https://pari.math.u-bordeaux.fr/cgi-bin/bugreport.cgi?bug=2441

Upstream: Fixed upstream, in a later stable release.

CC: @yyyyx4 @JohnCremona

Component: elliptic curves

Reviewer: Lorenz Panny

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

yyyyx4 commented 1 year ago
comment:1

It looks like my new examples from #33915 were the first to trigger this issue, but it's perfectly reproducible as a very simple standalone example (see updated ticket description).

In other words, this appears to be a PARI bug.

yyyyx4 commented 1 year ago

Description changed:

--- 
+++ 
@@ -1,2 +1,17 @@
-As reported in [#34537 comment:145](https://github.com/sagemath/sage/issues/34537#comment:145)
+As reported in [#34537 comment:145](https://github.com/sagemath/sage/issues/34537#comment:145), the following code fails with a `PariError`:

+```sage
+sage: GF(41^16).inject_variables()
+Defining z16
+sage: E = EllipticCurve([
+....:         8*z16^15+23*z16^14+7*z16^13+27*z16^12+35*z16^11+19*z16^10+11*z16^9+24*z16^8+4*z16^7+12*z16^6+17*z16^5+38*z16^4+10*z16^3+24*z16^2+25*z16+16,
+....:         15*z16^15+3*z16^14+38*z16^13+23*z16^12+33*z16^11+7*z16^10+29*z16^9+22*z16^8+8*z16^7+34*z16^6+9*z16^5+9*z16^4+39*z16^3+35*z16+34,
+....:         11*z16^15+40*z16^14+30*z16^13+16*z16^12+19*z16^11+21*z16^10+13*z16^8+18*z16^7+18*z16^6+34*z16^5+33*z16^4+11*z16^3+25*z16^2+38*z16+17,
+....:         8*z16^15+20*z16^14+17*z16^13+34*z16^12+5*z16^11+38*z16^10+26*z16^9+14*z16^8+8*z16^7+38*z16^6+6*z16^5+23*z16^4+16*z16^3+25*z16^2+13*z16+25,
+....:         27*z16^15+33*z16^14+2*z16^13+3*z16^11+12*z16^10+6*z16^9+28*z16^8+34*z16^7+4*z16^6+36*z16^5+9*z16^4+25*z16^3+5*z16^2+37*z16+31,
+....:     ])
+sage: E.gens()
+# ...
+PariError: impossible inverse in Fl_inv: Mod(31, 31)
+```
+
dimpase commented 1 year ago
comment:2

it'd be most helpful to reproduce it in plain pari/gp, and file a bug on their bug tracker.

yyyyx4 commented 1 year ago

Upstream: Reported upstream. No feedback yet.

yyyyx4 commented 1 year ago

Description changed:

--- 
+++ 
@@ -3,13 +3,7 @@
 ```sage
 sage: GF(41^16).inject_variables()
 Defining z16
-sage: E = EllipticCurve([
-....:         8*z16^15+23*z16^14+7*z16^13+27*z16^12+35*z16^11+19*z16^10+11*z16^9+24*z16^8+4*z16^7+12*z16^6+17*z16^5+38*z16^4+10*z16^3+24*z16^2+25*z16+16,
-....:         15*z16^15+3*z16^14+38*z16^13+23*z16^12+33*z16^11+7*z16^10+29*z16^9+22*z16^8+8*z16^7+34*z16^6+9*z16^5+9*z16^4+39*z16^3+35*z16+34,
-....:         11*z16^15+40*z16^14+30*z16^13+16*z16^12+19*z16^11+21*z16^10+13*z16^8+18*z16^7+18*z16^6+34*z16^5+33*z16^4+11*z16^3+25*z16^2+38*z16+17,
-....:         8*z16^15+20*z16^14+17*z16^13+34*z16^12+5*z16^11+38*z16^10+26*z16^9+14*z16^8+8*z16^7+38*z16^6+6*z16^5+23*z16^4+16*z16^3+25*z16^2+13*z16+25,
-....:         27*z16^15+33*z16^14+2*z16^13+3*z16^11+12*z16^10+6*z16^9+28*z16^8+34*z16^7+4*z16^6+36*z16^5+9*z16^4+25*z16^3+5*z16^2+37*z16+31,
-....:     ])
+sage: E = EllipticCurve([z16, 1])
 sage: E.gens()
 # ...
 PariError: impossible inverse in Fl_inv: Mod(31, 31)
yyyyx4 commented 1 year ago

Description changed:

--- 
+++ 
@@ -9,3 +9,4 @@
 PariError: impossible inverse in Fl_inv: Mod(31, 31)

+Upstream bug: https://pari.math.u-bordeaux.fr/cgi-bin/bugreport.cgi?bug=2441

yyyyx4 commented 1 year ago

Changed upstream from Reported upstream. No feedback yet. to Reported upstream. Developers acknowledge bug.

dimpase commented 1 year ago
comment:5

this should be fixed by the patch I just added on #34537 - please review.

This is the patch posted by Bill on https://pari.math.u-bordeaux.fr/cgi-bin/bugreport.cgi?bug=2441

dimpase commented 1 year ago

Changed upstream from Reported upstream. Developers acknowledge bug. to Fixed upstream, in a later stable release.

yyyyx4 commented 1 year ago
comment:6

The patch seems to fix the crash. I haven't checked any details, but I trust Bill knows what he's doing.

yyyyx4 commented 1 year ago

Reviewer: Lorenz Panny

vbraun commented 1 year ago
comment:7

Merge failure on top of:

b3911d2d785 Trac #31668: Run TestSuite on polynomial rings

197591fb1d4 Trac #22067: generating function of integral points of polyhedra

92a90541a41 Trac #21003: Add package pyscipopt, add MIP backend

1db9b79b786 Trac #34838: setuptools_scm, contourpy, sphinxcontrib_websupport, typing_extensions: Add missing dependencies

c3a55f95736 Trac #34537: Upgrade to pari 2.15.2

30d58156a46 Trac #34717: Check the presence of lrsnash for obtain_nash()

c7299453922 Trac #34891: fixing some E502 in tensor, dynamics, modules, plot, numerical

bc832cb3578 Trac #34889: Installation guide: On WSL, clear /mnt/c stuff from PATH

3570a9a63bd Trac #34881: allow to remove no constraints

0ceb1032430 Trac #34878: MixedIntegerLinearProgram.add_constraint: Option to return row indices, fix handling of empty constraints

df74efb66d2 Trac #34859: sagelib: Remove unnecessary import of typing_extensions

b6a76d7a848 Trac #34857: bump giac's GIAC_MIN_VERSION to 1.9

6a8155e6e5d Trac #34854: molien_series() should not use GAP's VirtualCharacter

dddf3ca0b08 Trac #34853: sage-env: Fix misconfiguration of pip

6f631021ac4 Trac #34847: modernize some for loops in cython files

9ffabc7f3e1 Trac #34844: removal of some unused imports about string conversion

4b50bc2af27 Trac #34843: pep8 cleanup in ore_polynomial_element.pyx

bdd14cec19b Trac #34837: fix E502 in some pyx files

172ad73ae38 Trac #34836: fix E271 and E272 in rings/ and schemes/

3d861266040 Trac #34749: Packages dsdp, scip_sdp

64d232b5bfd Trac #31329: Update scipoptsuite to 8.0.2 (now open source!), rename to scip

2dcafb3ac57 Trac #34839: Support tox 4

ad68f15d97f Trac #34648: Developer's guide: warn the transition to GitHub and add links to the transition guide

acebbc1a9e8 Trac #34824: do not include parent in hash of parking functions

9a7b6310dde Trac #34818: Error when defining differentials over GCA's with relations.

bb63c582122 Trac #34807: Add Construction of Hadamard matrices up to order 664

8a0b16d93be Trac #34804: Deprecate sage.interfaces is_...Element functions

80f8f950e95 Trac #34793: clean 3 files inside modular

b0cc282500e Trac #34547: Interfaces: use more lazy imports, restore top-level functions maxima_console etc.

5905da7ebfc Trac #33915: inseparable elliptic-curve isogenies

98b22ebdcf7 Trac #32826: scalar-multiplication endomorphisms of elliptic curves

4f11a750df9 Trac #8744: Improve add_edge in BipartiteGraph to make it independent from the current coloring

08aa2f8edaa Trac #33842: Upgrade python to 3.11

75bedf95960 Trac #34783: various details in combinat/

33fa8715bd5 Trac #34742: Optional package soplex (dependency of scip)

f84915f69d8 Trac #34726: Optional package papilo (dependency of scip)

e5cf1c0d9f2 Trac #34694: Bug in ExteriorAlgebra interior product

ebb9b611d02 Trac #34416: Manage pexpect logs created during doctesting

962177a2da1 Trac #33907: interfaces/expect.py random test failure

66409243991 Trac #16522: lazy_import doesn't resolve properly when indirectly imported

2114066f877 Updated SageMath version to 9.8.beta6

ticket milestone is not intended to be merged: sage-duplicate/invalid/wontfix

dimpase commented 1 year ago
comment:8

Volker, sorry, what merge? There is no branch here.

dimpase commented 1 year ago
comment:10

The issue here is fixed in #34537