Fix .multiplication_by_m when m is not coprime to characteristic (#6413)

src/sage/schemes/elliptic_curves/ell_generic.py

@@ -53,6 +53,7 @@
 # ****************************************************************************
 
 import math
+from sage.arith.misc import valuation
 
 import sage.rings.abc
 from sage.rings.finite_rings.integer_mod import mod
@@ -66,8 +67,6 @@
 
 from sage.arith.functions import lcm
 from sage.rings.integer import Integer
-from sage.rings.big_oh import O
-from sage.rings.infinity import Infinity as oo
 from sage.rings.rational import Rational
 from sage.rings.finite_rings.finite_field_constructor import FiniteField as GF
 from sage.rings.rational_field import RationalField
@@ -2019,6 +2018,30 @@
 
     torsion_polynomial = division_polynomial
 
+    @cached_method
+    def multiplication_by_p_isogeny(self):
+        r"""
+        Return the multiplication-by-\(p\) isogeny.
+
+        EXAMPLES::
+
+            sage: p = 23
+            sage: K.<a> = GF(p^3)
+            sage: E = EllipticCurve(K, [K.random_element(), K.random_element()])
+            sage: phi = E.multiplication_by_p_isogeny()
+            sage: assert phi.degree() == p**2
+            sage: P = E.random_element()
+            sage: assert phi(P) == P * p
+        """
+        from sage.rings.finite_rings.finite_field_base import FiniteField as FiniteField_generic
+
+        K = self.base_ring()
+        if not isinstance(K, FiniteField_generic):
+            raise ValueError(f"Base ring (={K}) is not a finite field.")
+
+        frob = self.frobenius_isogeny()
+        return frob.dual() * frob
+
     def _multiple_x_numerator(self, n, x=None):
         r"""
         Return the numerator of the `x`-coordinate of the `n\th` multiple of a
@@ -2333,6 +2356,20 @@
             sage: my_eval = lambda f,P: [fi(P[0],P[1]) for fi in f]
             sage: f = E.multiplication_by_m(2)
             sage: assert(E(eval(f,P)) == 2*P)
+
+        The following test shows that :trac:`6413` is indeed fixed::
+            sage: p = 7
+            sage: K.<a> = GF(p^2)
+            sage: E = EllipticCurve(K, [a + 3, 5 - a])
+            sage: k = p^2 * 3
+            sage: f, g = E.multiplication_by_m(k)
+            sage: for _ in range(100):
+            ....:     P = E.random_point()
+            ....:     if P * k == 0:
+            ....:         continue
+            ....:     Qx = f.subs(x=P[0])
+            ....:     Qy = g.subs(x=P[0], y=P[1])
+            ....:     assert (P * k).xy() == (Qx, Qy)
         """
         # Coerce the input m to be an integer
         m = Integer(m)
@@ -2352,126 +2389,51 @@
                 return x
 
         # Grab curve invariants
-        a1, a2, a3, a4, a6 = self.a_invariants()
+        a1, _, a3, _, _ = self.a_invariants()
 
         if m == -1:
             if not x_only:
                 return (x, -y-a1*x-a3)
             else:
                 return x
 
-        # the x-coordinate does not depend on the sign of m.  The work
+        # If we only require the x coordinate, it is faster to use the recursive formula
+        # since substituting polynomials is quite slow.
+        p = Integer(self.base_ring().characteristic())
+        v_p = 0 if p == 0 else valuation(m.abs(), p)
+        if not x_only:
+            m //= p**v_p
+
+        # the x-coordinate does not depend on the sign of m. The work
         # here is done by functions defined earlier:
 
         mx = (x.parent()(self._multiple_x_numerator(m.abs(), x))
               / x.parent()(self._multiple_x_denominator(m.abs(), x)))
 
         if x_only:
+            # slow.
+            if v_p > 0:
+                p_endo = self.multiplication_by_p_isogeny()
+                isog = p_endo**v_p
+                fx = isog.x_rational_map()
+                # slow.
+                mx = mx.subs(x=fx)
             # Return it if the optional parameter x_only is set.
             return mx
 
-        #  Consideration of the invariant differential
-        #  w=dx/(2*y+a1*x+a3) shows that m*w = d(mx)/(2*my+a1*mx+a3)
-        #  and hence 2*my+a1*mx+a3 = (1/m)*(2*y+a1*x+a3)*d(mx)/dx
-
+        # Consideration of the invariant differential
+        # w=dx/(2*y+a1*x+a3) shows that m*w = d(mx)/(2*my+a1*mx+a3)
+        # and hence 2*my+a1*mx+a3 = (1/m)*(2*y+a1*x+a3)*d(mx)/dx
         my = ((2*y+a1*x+a3)*mx.derivative(x)/m - a1*mx-a3)/2
 
-        return mx, my
-
-    def multiplication_by_m_isogeny(self, m):
-        r"""
-        Return the ``EllipticCurveIsogeny`` object associated to the
-        multiplication-by-`m` map on this elliptic curve.
-
-        The resulting isogeny will
-        have the associated rational maps (i.e., those returned by
-        :meth:`multiplication_by_m`) already computed.
+        if v_p > 0:
+            frob = self.frobenius_isogeny()
+            isog = (frob.dual() * frob)**v_p
+            fx, fy = isog.rational_maps()
+            # slow...
+            my = my.subs(x=fx, y=fy)
 
-        NOTE: This function is currently *much* slower than the
-        result of ``self.multiplication_by_m()``, because
-        constructing an isogeny precomputes a significant amount
-        of information. See :trac:`7368` and :trac:`8014` for the
-        status of improving this situation.
-
-        INPUT:
-
-        - ``m`` -- a nonzero integer
-
-        OUTPUT:
-
-        - An ``EllipticCurveIsogeny`` object associated to the
-          multiplication-by-`m` map on this elliptic curve.
-
-        EXAMPLES::
-
-            sage: E = EllipticCurve('11a1')
-            sage: E.multiplication_by_m_isogeny(7)
-            doctest:warning ... DeprecationWarning: ...
-            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])                                       # needs sage.rings.number_field
-            sage: E.multiplication_by_m_isogeny(1).rational_maps()                      # needs sage.rings.number_field
-            (x, y)
-
-        ::
-
-            sage: E = EllipticCurve_from_j(GF(31337).random_element())                  # needs sage.rings.finite_rings
-            sage: P = E.random_point()                                                  # needs sage.rings.finite_rings
-            sage: [E.multiplication_by_m_isogeny(m)(P) == m*P for m in (1,2,3,5,7,9)]   # needs sage.rings.finite_rings
-            [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))
-
-        Test for :trac:`34727`::
-
-            sage: E = EllipticCurve([5,5])
-            sage: E.multiplication_by_m_isogeny(-1)
-            Isogeny of degree 1
-             from Elliptic Curve defined by y^2 = x^3 + 5*x + 5 over Rational Field
-             to Elliptic Curve defined by y^2 = x^3 + 5*x + 5 over Rational Field
-            sage: E.multiplication_by_m_isogeny(-2)
-            Isogeny of degree 4
-             from Elliptic Curve defined by y^2 = x^3 + 5*x + 5 over Rational Field
-             to Elliptic Curve defined by y^2 = x^3 + 5*x + 5 over Rational Field
-            sage: E.multiplication_by_m_isogeny(-3)
-            Isogeny of degree 9
-             from Elliptic Curve defined by y^2 = x^3 + 5*x + 5 over Rational Field
-             to Elliptic Curve defined by y^2 = x^3 + 5*x + 5 over Rational Field
-            sage: mu = E.multiplication_by_m_isogeny
-            sage: all(mu(-m) == -mu(m) for m in (1,2,3,5,7))
-            True
-        """
-        from sage.misc.superseded import deprecation
-        deprecation(32826, 'The .multiplication_by_m_isogeny() method is superseded by .scalar_multiplication().')
-
-        mx, my = self.multiplication_by_m(m)
-
-        torsion_poly = self.torsion_polynomial(abs(m)).monic()
-        phi = self.isogeny(torsion_poly, codomain=self)
-        phi._EllipticCurveIsogeny__initialize_rational_maps(precomputed_maps=(mx, my))
-
-        # 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()'
+        return mx, my
 
     def scalar_multiplication(self, m):
         r"""

src/sage/schemes/elliptic_curves/hom.py

@@ -227,21 +227,6 @@ def _richcmp_(self, other, op):
             sage: phi.dual() == psi.dual()
             True
 
-        ::
-
-            sage: from sage.schemes.elliptic_curves.weierstrass_morphism import WeierstrassIsomorphism, identity_morphism
-            sage: E = EllipticCurve([9,9])
-            sage: F = E.change_ring(GF(71))
-            sage: wE = identity_morphism(E)
-            sage: wF = identity_morphism(F)
-            sage: mE = E.scalar_multiplication(1)
-            sage: mF = F.multiplication_by_m_isogeny(1)
-            doctest:warning ... DeprecationWarning: ...
-            sage: [mE == wE, mF == wF]
-            [True, True]
-            sage: [a == b for a in (wE,mE) for b in (wF,mF)]
-            [False, False, False, False]
-
         .. SEEALSO::
 
             - :meth:`_comparison_impl`

src/sage/schemes/elliptic_curves/hom_scalar.py

@@ -381,8 +381,7 @@ def scaling_factor(self):
             sage: u = phi.scaling_factor()
             sage: u == phi.formal()[1]
             True
-            sage: u == E.multiplication_by_m_isogeny(5).scaling_factor()
-            doctest:warning ... DeprecationWarning: ...
+            sage: u == 5
             True
 
         The scaling factor lives in the base ring::