Skip to content

Commit

Permalink
Trac #32490: multiplication_by_m_isogeny is only correct up to automo…
Browse files Browse the repository at this point in the history
…rphism

This is wrong:

{{{#!sage
sage: E = EllipticCurve(QQbar, [1,0])
sage: E.multiplication_by_m_isogeny(1).rational_maps()
(x, -y)
}}}

{{{#!sage
sage: E = EllipticCurve(GF(127), [1,0])
sage: P = E.random_point()
sage: E.multiplication_by_m_isogeny(5)(P)
(15 : 56 : 1)
sage: 5*P
(15 : 71 : 1)
sage: -5*P
(15 : 56 : 1)
sage: E.multiplication_by_m(5) ==
(-E.multiplication_by_m_isogeny(5)).rational_maps()
True
}}}

It isn't ''consistently'' wrong:

{{{#!sage
sage: E = EllipticCurve(QQ, [1,0])
sage: E.multiplication_by_m_isogeny(1).rational_maps()
(x, y)
}}}

----

The trouble is that the method first constructs **an** isogeny with the
correct kernel and codomain, then sets the precomputed rational maps.
However, the isomorphism to the prescribed codomain is not unique, and
picking the wrong one means the resulting isogeny is off by an
automorphism — most commonly `[-1]`.

The patch is not pretty, but it seems to fix the issue for now and
shouldn't cause any significant slowdowns.
Once (if) #32388 gets in, we should probably instead use an
`EllipticCurveHom` subclass specifically for scalar multiplications that
imitates a normal isogeny but with different internals. I do have a
draft of this ready and it's not only much cleaner, but also resolves
part of #8014.

URL: https://trac.sagemath.org/32490
Reported by: lorenz
Ticket author(s): Lorenz Panny
Reviewer(s): Travis Scrimshaw
  • Loading branch information
Release Manager committed Oct 23, 2021
2 parents b6b1b25 + ead071d commit 9750b0e
Showing 1 changed file with 32 additions and 3 deletions.
35 changes: 32 additions & 3 deletions src/sage/schemes/elliptic_curves/ell_generic.py
Original file line number Diff line number Diff line change
Expand Up @@ -2150,7 +2150,7 @@ def multiplication_by_m(self, m, x_only=False):
def multiplication_by_m_isogeny(self, m):
r"""
Return the ``EllipticCurveIsogeny`` object associated to the
multiplication-by-`m` map on self.
multiplication-by-`m` map on this elliptic curve.
The resulting isogeny will
have the associated rational maps (i.e. those returned by
Expand All @@ -2169,22 +2169,51 @@ def multiplication_by_m_isogeny(self, m):
OUTPUT:
- An ``EllipticCurveIsogeny`` object associated to the
multiplication-by-`m` map on self.
multiplication-by-`m` map on this elliptic curve.
EXAMPLES::
sage: E = EllipticCurve('11a1')
sage: E.multiplication_by_m_isogeny(7)
Isogeny of degree 49 from Elliptic Curve defined by y^2 + y = x^3 - x^2 - 10*x - 20 over Rational Field to Elliptic Curve defined by y^2 + y = x^3 - x^2 - 10*x - 20 over Rational Field
TESTS:
Tests for :trac:`32490`::
sage: E = EllipticCurve(QQbar, [1,0])
sage: E.multiplication_by_m_isogeny(1).rational_maps()
(x, y)
::
sage: E = EllipticCurve_from_j(GF(31337).random_element())
sage: P = E.random_point()
sage: [E.multiplication_by_m_isogeny(m)(P) == m*P for m in (1,2,3,5,7,9)]
[True, True, True, True, True, True]
::
sage: E = EllipticCurve('99.a1')
sage: E.multiplication_by_m_isogeny(5)
Isogeny of degree 25 from Elliptic Curve defined by y^2 + x*y + y = x^3 - x^2 - 17*x + 30 over Rational Field to Elliptic Curve defined by y^2 + x*y + y = x^3 - x^2 - 17*x + 30 over Rational Field
sage: E.multiplication_by_m_isogeny(2).rational_maps()
((1/4*x^4 + 33/4*x^2 - 121/2*x + 363/4)/(x^3 - 3/4*x^2 - 33/2*x + 121/4),
(-1/256*x^7 + 1/128*x^6*y - 7/256*x^6 - 3/256*x^5*y - 105/256*x^5 - 165/256*x^4*y + 1255/256*x^4 + 605/128*x^3*y - 473/64*x^3 - 1815/128*x^2*y - 10527/256*x^2 + 2541/128*x*y + 4477/32*x - 1331/128*y - 30613/256)/(1/16*x^6 - 3/32*x^5 - 519/256*x^4 + 341/64*x^3 + 1815/128*x^2 - 3993/64*x + 14641/256))
"""
mx, my = self.multiplication_by_m(m)

torsion_poly = self.torsion_polynomial(m).monic()
phi = self.isogeny(torsion_poly, codomain=self)
phi._EllipticCurveIsogeny__initialize_rational_maps(precomputed_maps=(mx, my))

return phi
# trac 32490: using codomain=self can give a wrong isomorphism
for aut in self.automorphisms():
psi = aut * phi
if psi.rational_maps() == (mx, my):
return psi

assert False, 'bug in multiplication_by_m_isogeny()'

def isomorphism_to(self, other):
"""
Expand Down

0 comments on commit 9750b0e

Please sign in to comment.