Skip to content

Commit

Permalink
gh-35644: make EllipticCurve.lift_x() deterministic
Browse files Browse the repository at this point in the history 
    
Resolves #35281.
    
URL: #35644
Reported by: Lorenz Panny
Reviewer(s): John Cremona
  • Loading branch information
Release Manager committed Jun 3, 2023
2 parents 1f83fc3 + 6da5d12 commit d89ea60
Show file tree
Hide file tree
Showing 7 changed files with 98 additions and 60 deletions.
2 changes: 1 addition & 1 deletion src/sage/rings/polynomial/multi_polynomial_ideal.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5418,7 +5418,7 @@ def weil_restriction(self):
We pick a point on ``E``::
sage: p = E.lift_x(1); p # optional - sage.rings.number_field
(1 : 2 : 1)
(1 : -6 : 1)
sage: I = E.defining_ideal(); I # optional - sage.rings.number_field
Ideal (-x^3 - 2*x^2*z + x*y*z + y^2*z - 4*x*z^2 + 3*y*z^2 - 5*z^3)
Expand Down
45 changes: 30 additions & 15 deletions src/sage/schemes/elliptic_curves/ell_generic.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -711,6 +711,9 @@ def lift_x(self, x, all=False, extend=False):
r"""
Return one or all points with given `x`-coordinate.
This method is deterministic: It returns the same data each
time when called again with the same `x`.
INPUT:
- ``x`` -- an element of the base ring of the curve, or of an extension.
Expand Down Expand Up @@ -745,11 +748,11 @@ def lift_x(self, x, all=False, extend=False):
sage: E = EllipticCurve('37a'); E
Elliptic Curve defined by y^2 + y = x^3 - x over Rational Field
sage: E.lift_x(1)
(1 : 0 : 1)
(1 : -1 : 1)
sage: E.lift_x(2)
(2 : 2 : 1)
(2 : -3 : 1)
sage: E.lift_x(1/4, all=True)
[(1/4 : -3/8 : 1), (1/4 : -5/8 : 1)]
[(1/4 : -5/8 : 1), (1/4 : -3/8 : 1)]
There are no rational points with `x`-coordinate 3::
Expand All @@ -765,23 +768,23 @@ def lift_x(self, x, all=False, extend=False):
the base ring to the parent of `x`::
sage: P = E.lift_x(3, extend=True); P # optional - sage.rings.number_field
(3 : y : 1)
(3 : -y - 1 : 1)
sage: P.curve() # optional - sage.rings.number_field
Elliptic Curve defined by y^2 + y = x^3 + (-1)*x
over Number Field in y with defining polynomial y^2 + y - 24
Or we can extend scalars. There are two such points in `E(\RR)`::
sage: E.change_ring(RR).lift_x(3, all=True)
[(3.00000000000000 : 4.42442890089805 : 1.00000000000000),
(3.00000000000000 : -5.42442890089805 : 1.00000000000000)]
[(3.00000000000000 : -5.42442890089805 : 1.00000000000000),
(3.00000000000000 : 4.42442890089805 : 1.00000000000000)]
And of course it always works in `E(\CC)`::
sage: E.change_ring(RR).lift_x(.5, all=True)
[]
sage: E.change_ring(CC).lift_x(.5)
(0.500000000000000 : -0.500000000000000 + 0.353553390593274*I : 1.00000000000000)
(0.500000000000000 : -0.500000000000000 - 0.353553390593274*I : 1.00000000000000)
In this example we start with a curve defined over `\QQ`
which has no rational points with `x=0`, but using
Expand All @@ -791,7 +794,7 @@ def lift_x(self, x, all=False, extend=False):
sage: E = EllipticCurve([0,0,0,0,2]); E
Elliptic Curve defined by y^2 = x^3 + 2 over Rational Field
sage: P = E.lift_x(0, extend=True); P # optional - sage.rings.number_field
(0 : y : 1)
(0 : -y : 1)
sage: P.curve() # optional - sage.rings.number_field
Elliptic Curve defined by y^2 = x^3 + 2
over Number Field in y with defining polynomial y^2 - 2
Expand All @@ -801,7 +804,7 @@ def lift_x(self, x, all=False, extend=False):
sage: E = EllipticCurve('37a').change_ring(GF(17)); E # optional - sage.rings.finite_rings
Elliptic Curve defined by y^2 + y = x^3 + 16*x over Finite Field of size 17
sage: E.lift_x(7) # optional - sage.rings.finite_rings
(7 : 11 : 1)
(7 : 5 : 1)
sage: E.lift_x(3) # optional - sage.rings.finite_rings
Traceback (most recent call last):
...
Expand All @@ -820,14 +823,14 @@ def lift_x(self, x, all=False, extend=False):
sage: E = EllipticCurve('37a')
sage: P = E.lift_x(pAdicField(17, 5)(6)); P # optional - sage.rings.padics
(6 + O(17^5) : 2 + 16*17 + 16*17^2 + 16*17^3 + 16*17^4 + O(17^5) : 1 + O(17^5))
(6 + O(17^5) : 14 + O(17^5) : 1 + O(17^5))
sage: P.curve() # optional - sage.rings.padics
Elliptic Curve defined by
y^2 + (1+O(17^5))*y = x^3 + (16+16*17+16*17^2+16*17^3+16*17^4+O(17^5))*x
over 17-adic Field with capped relative precision 5
sage: K.<t> = PowerSeriesRing(QQ, 't', 5)
sage: P = E.lift_x(1 + t); P
(1 + t : 2*t - t^2 + 5*t^3 - 21*t^4 + O(t^5) : 1)
(1 + t : -1 - 2*t + t^2 - 5*t^3 + 21*t^4 + O(t^5) : 1)
sage: K.<a> = GF(16) # optional - sage.rings.finite_rings
sage: P = E.change_ring(K).lift_x(a^3); P # optional - sage.rings.finite_rings
(a^3 : a^3 + a : 1)
Expand All @@ -840,7 +843,7 @@ def lift_x(self, x, all=False, extend=False):
Elliptic Curve defined by y^2 = x^3 + 2 over Rational Field
sage: x = polygen(QQ)
sage: P = E.lift_x(x, extend=True); P
(x : y : 1)
(x : -y : 1)
This point is a generic point on E::
Expand All @@ -850,9 +853,9 @@ def lift_x(self, x, all=False, extend=False):
over Fraction Field of Univariate Polynomial Ring in x over Rational Field
with modulus y^2 - x^3 - 2
sage: -P
(x : -y : 1)
(x : y : 1)
sage: 2*P
((1/4*x^4 - 4*x)/(x^3 + 2) : ((1/8*x^6 + 5*x^3 - 4)/(x^6 + 4*x^3 + 4))*y : 1)
((1/4*x^4 - 4*x)/(x^3 + 2) : ((-1/8*x^6 - 5*x^3 + 4)/(x^6 + 4*x^3 + 4))*y : 1)
Check that :trac:`30297` is fixed::
Expand All @@ -873,6 +876,15 @@ def lift_x(self, x, all=False, extend=False):
[]
sage: E.lift_x(7, all=True) # optional - sage.rings.finite_rings
[(7 : 3 : 1), (7 : 14 : 1)]
Check determinism::
sage: F.<t> = GF((101,3))
sage: {(t+1).sqrt() for _ in range(1000)} # both square roots can occur
{29*t^2 + 56*t + 26, 72*t^2 + 45*t + 75}
sage: E = EllipticCurve(F, [1,1])
sage: {E.lift_x(t+1) for _ in range(1000)} # but .lift_x() uses a fixed one
{(t + 1 : 39*t^2 + 14*t + 12 : 1)}
"""
K = self.base_ring()
L = x.parent()
Expand Down Expand Up @@ -909,6 +921,8 @@ def lift_x(self, x, all=False, extend=False):
if D.is_square(): # avoid automatic creation of sqrts
ys = [(-b+d)/2 for d in D.sqrt(all=True)]

ys.sort() # ensure deterministic behavior

# Return the point(s) if any:

if ys:
Expand Down Expand Up @@ -939,6 +953,7 @@ def lift_x(self, x, all=False, extend=False):
ys = [y1]
else:
ys = [y1, y2]
ys.sort() # ensure deterministic behavior
one = M.one()
if all:
return [EM.point([x, y, one], check=False) for y in ys]
Expand Down Expand Up @@ -1692,7 +1707,7 @@ def division_polynomial_0(self, n, x=None):
sage: xlist = pol.roots(multiplicities=False); xlist
[9, 2, -1/3, -5]
sage: [E.lift_x(x, all=True) for x in xlist]
[[(9 : 23 : 1), (9 : -33 : 1)], [(2 : 2 : 1), (2 : -5 : 1)], [], []]
[[(9 : -33 : 1), (9 : 23 : 1)], [(2 : -5 : 1), (2 : 2 : 1)], [], []]
.. NOTE::
Expand Down
8 changes: 4 additions & 4 deletions src/sage/schemes/elliptic_curves/ell_point.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2631,7 +2631,7 @@ def height(self, precision=None, normalised=True, algorithm='pari'):
sage: K.<i> = NumberField(x^2 + 1) # optional - sage.rings.number_field
sage: E = EllipticCurve(K, [0,0,4,6*i,0]) # optional - sage.rings.number_field
sage: Q = E.lift_x(-9/4); Q # optional - sage.rings.number_field
(-9/4 : -27/8*i : 1)
(-9/4 : 27/8*i - 4 : 1)
sage: Q.height() # optional - sage.rings.number_field
2.69518560017909
sage: (15*Q).height() / Q.height() # optional - sage.rings.number_field
Expand Down Expand Up @@ -2783,7 +2783,7 @@ def archimedean_local_height(self, v=None, prec=None, weighted=False):
Elliptic Curve defined by y^2 + y = x^3 + (-1)*x^2 over Number Field
in a with defining polynomial x^2 + 2 with a = 1.414213562373095?*I
sage: P = E.lift_x(2 + a); P # optional - sage.rings.number_field
(a + 2 : 2*a + 1 : 1)
(a + 2 : -2*a - 2 : 1)
sage: P.archimedean_local_height(K.places(prec=170)[0]) / 2 # optional - sage.rings.number_field
0.45754773287523276736211210741423654346576029814695
Expand All @@ -2796,7 +2796,7 @@ def archimedean_local_height(self, v=None, prec=None, weighted=False):
0.510184995162373
sage: Q = E.lift_x(-9/4); Q # optional - sage.rings.number_field
(-9/4 : -27/8*i : 1)
(-9/4 : 27/8*i - 4 : 1)
sage: Q.archimedean_local_height(K.places()[0]) / 2 # optional - sage.rings.number_field
0.654445619529600
Expand Down Expand Up @@ -3019,7 +3019,7 @@ def non_archimedean_local_height(self, v=None, prec=None,
0
sage: Q = E.lift_x(-9/4); Q # optional - sage.rings.number_field
(-9/4 : -27/8*i : 1)
(-9/4 : 27/8*i - 4 : 1)
sage: Q.non_archimedean_local_height(K.ideal(1+i)) # optional - sage.rings.number_field
2*log(2)
sage: Q.non_archimedean_local_height(K.ideal(3)) # optional - sage.rings.number_field
Expand Down
85 changes: 54 additions & 31 deletions src/sage/schemes/elliptic_curves/ell_rational_field.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5821,7 +5821,7 @@ def integral_x_coords_in_interval(self,xmin,xmax):
sage: E = EllipticCurve("141d1")
sage: E.integral_points()
[(0 : 0 : 1), (2 : 1 : 1)]
[(0 : -1 : 1), (2 : -2 : 1)]
"""
xmin = pari(xmin)
xmax = pari(xmax)
Expand Down Expand Up @@ -5868,10 +5868,11 @@ def integral_points(self, mw_base='auto', both_signs=False, verbose=False):
sage: E = EllipticCurve([0,0,1,-7,6])
sage: P1 = E.point((2,0)); P2 = E.point((-1,3)); P3 = E.point((4,6))
sage: a = E.integral_points([P1,P2,P3]); a
[(-3 : 0 : 1), (-2 : 3 : 1), (-1 : 3 : 1), (0 : 2 : 1), (1 : 0 : 1),
(2 : 0 : 1), (3 : 3 : 1), (4 : 6 : 1), (8 : 21 : 1), (11 : 35 : 1),
(14 : 51 : 1), (21 : 95 : 1), (37 : 224 : 1), (52 : 374 : 1),
(93 : 896 : 1), (342 : 6324 : 1), (406 : 8180 : 1), (816 : 23309 : 1)]
[(-3 : -1 : 1), (-2 : -4 : 1), (-1 : -4 : 1), (0 : -3 : 1),
(1 : -1 : 1), (2 : -1 : 1), (3 : -4 : 1), (4 : -7 : 1),
(8 : -22 : 1), (11 : -36 : 1), (14 : -52 : 1), (21 : -96 : 1),
(37 : -225 : 1), (52 : -375 : 1), (93 : -897 : 1),
(342 : -6325 : 1), (406 : -8181 : 1), (816 : -23310 : 1)]
::
Expand Down Expand Up @@ -5908,7 +5909,7 @@ def integral_points(self, mw_base='auto', both_signs=False, verbose=False):
An example with negative discriminant::
sage: EllipticCurve('900d1').integral_points()
[(-11 : 27 : 1), (-4 : 34 : 1), (4 : 18 : 1), (16 : 54 : 1)]
[(-11 : -27 : 1), (-4 : -34 : 1), (4 : -18 : 1), (16 : -54 : 1)]
Another example with rank 5 and no torsion points::
Expand All @@ -5924,7 +5925,7 @@ def integral_points(self, mw_base='auto', both_signs=False, verbose=False):
The bug reported on :trac:`4525` is now fixed::
sage: EllipticCurve('91b1').integral_points()
[(-1 : 3 : 1), (1 : 0 : 1), (3 : 4 : 1)]
[(-1 : -4 : 1), (1 : -1 : 1), (3 : -5 : 1)]
::
Expand Down Expand Up @@ -6307,19 +6308,49 @@ def S_integral_points(self, S, mw_base='auto', both_signs=False, verbose=False,
x-coords of points with bounded absolute value
[-3, -2, -1, 0, 1, 2]
Total number of S-integral points: 43
[(-3 : 0 : 1), (-26/9 : 28/27 : 1), (-8159/2916 : 233461/157464 : 1),
(-2759/1024 : 60819/32768 : 1), (-151/64 : 1333/512 : 1),
(-1343/576 : 36575/13824 : 1), (-2 : 3 : 1), (-7/4 : 25/8 : 1), (-1 : 3 : 1),
(-47/256 : 9191/4096 : 1), (0 : 2 : 1), (1/4 : 13/8 : 1), (4/9 : 35/27 : 1),
(9/16 : 69/64 : 1), (58/81 : 559/729 : 1), (7/9 : 17/27 : 1),
(6169/6561 : 109871/531441 : 1), (1 : 0 : 1), (17/16 : -25/64 : 1), (2 : 0 : 1),
(33/16 : 17/64 : 1), (172/81 : 350/729 : 1), (9/4 : 7/8 : 1), (25/9 : 64/27 : 1),
(3 : 3 : 1), (31/9 : 116/27 : 1), (4 : 6 : 1), (25/4 : 111/8 : 1),
(1793/256 : 68991/4096 : 1), (8 : 21 : 1), (625/64 : 14839/512 : 1), (11 : 35 : 1),
(14 : 51 : 1), (21 : 95 : 1), (37 : 224 : 1), (52 : 374 : 1),
(6142/81 : 480700/729 : 1), (93 : 896 : 1), (4537/36 : 305425/216 : 1),
(342 : 6324 : 1), (406 : 8180 : 1), (816 : 23309 : 1),
(207331217/4096 : 2985362173625/262144 : 1)]
[(-3 : -1 : 1),
(-26/9 : -55/27 : 1),
(-8159/2916 : -390925/157464 : 1),
(-2759/1024 : -93587/32768 : 1),
(-151/64 : -1845/512 : 1),
(-1343/576 : -50399/13824 : 1),
(-2 : -4 : 1),
(-7/4 : -33/8 : 1),
(-1 : -4 : 1),
(-47/256 : -13287/4096 : 1),
(0 : -3 : 1),
(1/4 : -21/8 : 1),
(4/9 : -62/27 : 1),
(9/16 : -133/64 : 1),
(58/81 : -1288/729 : 1),
(7/9 : -44/27 : 1),
(6169/6561 : -641312/531441 : 1),
(1 : -1 : 1),
(17/16 : -39/64 : 1),
(2 : -1 : 1),
(33/16 : -81/64 : 1),
(172/81 : -1079/729 : 1),
(9/4 : -15/8 : 1),
(25/9 : -91/27 : 1),
(3 : -4 : 1),
(31/9 : -143/27 : 1),
(4 : -7 : 1),
(25/4 : -119/8 : 1),
(1793/256 : -73087/4096 : 1),
(8 : -22 : 1),
(625/64 : -15351/512 : 1),
(11 : -36 : 1),
(14 : -52 : 1),
(21 : -96 : 1),
(37 : -225 : 1),
(52 : -375 : 1),
(6142/81 : -481429/729 : 1),
(93 : -897 : 1),
(4537/36 : -305641/216 : 1),
(342 : -6325 : 1),
(406 : -8181 : 1),
(816 : -23310 : 1),
(207331217/4096 : -2985362435769/262144 : 1)]
It is not necessary to specify mw_base; if it is not provided,
then the Mordell-Weil basis must be computed, which may take
Expand Down Expand Up @@ -6352,17 +6383,9 @@ def S_integral_points(self, S, mw_base='auto', both_signs=False, verbose=False,
This is curve "7690e1" which failed until :trac:`4805` was fixed::
sage: EllipticCurve([1,1,1,-301,-1821]).S_integral_points([13,2])
[(-13 : 16 : 1),
(-9 : 20 : 1),
(-7 : 4 : 1),
(21 : 30 : 1),
(23 : 52 : 1),
(63 : 452 : 1),
(71 : 548 : 1),
(87 : 756 : 1),
(2711 : 139828 : 1),
(7323 : 623052 : 1),
(17687 : 2343476 : 1)]
[(-13 : -4 : 1), (-9 : -12 : 1), (-7 : 2 : 1), (21 : -52 : 1),
(23 : -76 : 1), (63 : -516 : 1), (71 : -620 : 1), (87 : -844 : 1),
(2711 : -142540 : 1), (7323 : -630376 : 1), (17687 : -2361164 : 1)]
- Some parts of this implementation are partially based on the
function integral_points()
Expand Down
6 changes: 3 additions & 3 deletions src/sage/schemes/elliptic_curves/height.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1152,9 +1152,9 @@ def psi(self, xi, v):
sage: L = E.period_lattice(v)
sage: P = E.lift_x(10/9)
sage: L(P)
1.53151606047462
0.958696500380439
sage: L(P) / L.real_period()
0.615014189772115
0.384985810227885
sage: H = E.height_function()
sage: H.psi(10/9, v)
0.615014189772115
Expand Down Expand Up @@ -1221,7 +1221,7 @@ def S(self, xi1, xi2, v):
sage: v = K.real_places()[0] # optional - sage.rings.number_field
sage: H = E.height_function() # optional - sage.rings.number_field
sage: H.S(9, 10, v) # optional - sage.rings.number_field
([0.0781194447253472, 0.0823423732016403] U [0.917657626798360, 0.921880555274653])
([0.078119444725347..., 0.082342373201640...] U [0.91765762679836..., 0.92188055527465...])
"""
L = self.E.period_lattice(v)
w1, w2 = L.basis(prec=v.codomain().prec())
Expand Down
4 changes: 2 additions & 2 deletions src/sage/schemes/elliptic_curves/hom_composite.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
From: Elliptic Curve defined by y^2 = x^3 + 1 over Finite Field of size 419
To: Elliptic Curve defined by y^2 = x^3 + 101*x + 285 over Finite Field of size 419
sage: psi(E.lift_x(11)) # optional - sage.rings.finite_rings
(352 : 73 : 1)
(352 : 346 : 1)
sage: psi.rational_maps() # optional - sage.rings.finite_rings
((x^35 + 162*x^34 + 186*x^33 + 92*x^32 - ... + 44*x^3 + 190*x^2 + 80*x
- 72)/(x^34 + 162*x^33 - 129*x^32 + 41*x^31 + ... + 66*x^3 - 191*x^2 + 119*x + 21),
Expand Down Expand Up @@ -472,7 +472,7 @@ def _call_(self, P):
sage: psi = EllipticCurveHom_composite(E, E.torsion_points()) # optional - sage.rings.number_field
sage: R = E.lift_x(15/4 * (a+3)) # optional - sage.rings.number_field
sage: psi(R) # indirect doctest # optional - sage.rings.number_field
(1033648757/303450 : 58397496786187/1083316500*a - 62088706165177/2166633000 : 1)
(1033648757/303450 : -58397496786187/1083316500*a + 54706287407197/2166633000 : 1)
Check that copying the order over works::
Expand Down
Loading

0 comments on commit d89ea60

Please sign in to comment.