Skip to content

Commit

Permalink
Trac #34728: change sorting for WeierstrassIsomorphism
Browse files Browse the repository at this point in the history 
Currently, `EllipticCurve_generic.automorphisms()` returns the
automorphisms in a more or less arbitrary (albeit deterministic) order.

It is much more natural to users to receive a list with the identity and
negation first, since they exist for any curve, then any other
automorphisms that may exist. (I have personally seen code making this
incorrect assumption.)

In this patch, we change `._comparison_impl()` for
`WeierstrassIsomorphism` in such a way that `[1]` and `[-1]` will appear
first in a sorted list of automorphisms.

Diff without the dependencies: https://git.sagemath.org/sage.git/diff?id
2=d92c9f4e4f671409ff695f4f294240492dbe7c86&id=c9e964632bdfd95c1f89557a5c
7e1ef1c78a794a

URL: https://trac.sagemath.org/34728
Reported by: lorenz
Ticket author(s): Lorenz Panny
Reviewer(s): John Cremona
  • Loading branch information
Release Manager committed Nov 22, 2022
2 parents 2cec793 + c9e9646 commit 795383f
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 48 deletions.
48 changes: 40 additions & 8 deletions src/sage/schemes/elliptic_curves/ell_generic.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2429,6 +2429,9 @@ def automorphisms(self, field=None):
"""
Return the set of isomorphisms from self to itself (as a list).
The identity and negation morphisms are guaranteed to appear
as the first and second entry of the returned list.
INPUT:
- ``field`` (default ``None``) -- a field into which the
Expand All @@ -2446,23 +2449,52 @@ def automorphisms(self, field=None):
sage: E = EllipticCurve_from_j(QQ(0)) # a curve with j=0 over QQ
sage: E.automorphisms()
[Elliptic-curve endomorphism of Elliptic Curve defined by y^2 + y = x^3 over Rational Field
Via: (u,r,s,t) = (-1, 0, 0, -1), Elliptic-curve endomorphism of Elliptic Curve defined by y^2 + y = x^3 over Rational Field
Via: (u,r,s,t) = (1, 0, 0, 0)]
Via: (u,r,s,t) = (1, 0, 0, 0),
Elliptic-curve endomorphism of Elliptic Curve defined by y^2 + y = x^3 over Rational Field
Via: (u,r,s,t) = (-1, 0, 0, -1)]
We can also find automorphisms defined over extension fields::
sage: K.<a> = NumberField(x^2+3) # adjoin roots of unity
sage: E.automorphisms(K)
[Elliptic-curve endomorphism of Elliptic Curve defined by y^2 + y = x^3 over Number Field in a with defining polynomial x^2 + 3
Via: (u,r,s,t) = (-1, 0, 0, -1),
...
Elliptic-curve endomorphism of Elliptic Curve defined by y^2 + y = x^3 over Number Field in a with defining polynomial x^2 + 3
Via: (u,r,s,t) = (1, 0, 0, 0)]
Via: (u,r,s,t) = (1, 0, 0, 0),
Elliptic-curve endomorphism of Elliptic Curve defined by y^2 + y = x^3 over Number Field in a with defining polynomial x^2 + 3
Via: (u,r,s,t) = (-1, 0, 0, -1),
Elliptic-curve endomorphism of Elliptic Curve defined by y^2 + y = x^3 over Number Field in a with defining polynomial x^2 + 3
Via: (u,r,s,t) = (-1/2*a - 1/2, 0, 0, 0),
Elliptic-curve endomorphism of Elliptic Curve defined by y^2 + y = x^3 over Number Field in a with defining polynomial x^2 + 3
Via: (u,r,s,t) = (1/2*a + 1/2, 0, 0, -1),
Elliptic-curve endomorphism of Elliptic Curve defined by y^2 + y = x^3 over Number Field in a with defining polynomial x^2 + 3
Via: (u,r,s,t) = (1/2*a - 1/2, 0, 0, 0),
Elliptic-curve endomorphism of Elliptic Curve defined by y^2 + y = x^3 over Number Field in a with defining polynomial x^2 + 3
Via: (u,r,s,t) = (-1/2*a + 1/2, 0, 0, -1)]
::
sage: [len(EllipticCurve_from_j(GF(q,'a')(0)).automorphisms()) for q in [2,4,3,9,5,25,7,49]]
[2, 24, 2, 12, 2, 6, 6, 6]
TESTS:
Random testing::
sage: p = random_prime(100)
sage: k = randrange(1,30)
sage: F.<t> = GF((p,k))
sage: while True:
....: try:
....: E = EllipticCurve(list((F^5).random_element()))
....: except ArithmeticError:
....: continue
....: break
sage: Aut = E.automorphisms()
sage: Aut[0] == E.multiplication_by_m_isogeny(1)
True
sage: Aut[1] == E.multiplication_by_m_isogeny(-1)
True
sage: sorted(Aut) == Aut
True
"""
if field is not None:
self = self.change_ring(field)
Expand Down Expand Up @@ -2492,9 +2524,9 @@ def isomorphisms(self, other, field=None):
sage: F = EllipticCurve('27a3') # should be the same one
sage: E.isomorphisms(F)
[Elliptic-curve endomorphism of Elliptic Curve defined by y^2 + y = x^3 over Rational Field
Via: (u,r,s,t) = (-1, 0, 0, -1),
Via: (u,r,s,t) = (1, 0, 0, 0),
Elliptic-curve endomorphism of Elliptic Curve defined by y^2 + y = x^3 over Rational Field
Via: (u,r,s,t) = (1, 0, 0, 0)]
Via: (u,r,s,t) = (-1, 0, 0, -1)]
We can also find isomorphisms defined over extension fields::
Expand Down
59 changes: 19 additions & 40 deletions src/sage/schemes/elliptic_curves/weierstrass_morphism.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,10 @@

from .constructor import EllipticCurve
from sage.schemes.elliptic_curves.hom import EllipticCurveHom
from sage.structure.richcmp import (richcmp_method, richcmp, richcmp_not_equal,
op_NE)
from sage.structure.richcmp import (richcmp, richcmp_not_equal, op_EQ, op_NE)
from sage.structure.sequence import Sequence
from sage.rings.all import Integer, PolynomialRing


@richcmp_method
class baseWI():
r"""
This class implements the basic arithmetic of isomorphisms between
Expand Down Expand Up @@ -87,38 +84,6 @@ def __init__(self, u=1, r=0, s=0, t=0):
self.s = s
self.t = t

def __richcmp__(self, other, op):
"""
Standard comparison function.
The ordering is just lexicographic on the tuple `(u,r,s,t)`.
.. NOTE::
In a list of automorphisms, there is no guarantee that the
identity will be first!
EXAMPLES::
sage: from sage.schemes.elliptic_curves.weierstrass_morphism import baseWI
sage: baseWI(1,2,3,4) == baseWI(1,2,3,4)
True
sage: baseWI(1,2,3,4) != baseWI(1,2,3,4)
False
sage: baseWI(1,2,3,4) < baseWI(1,2,3,5)
True
sage: baseWI(1,2,3,4) > baseWI(1,2,3,4)
False
It will never return equality if ``other`` is of another type::
sage: baseWI() == 1
False
"""
if not isinstance(other, baseWI):
return op == op_NE
return richcmp(self.tuple(), other.tuple(), op)

def tuple(self):
r"""
Return the parameters `u,r,s,t` as a tuple.
Expand Down Expand Up @@ -310,8 +275,8 @@ def _isomorphisms(E, F):
....: 2: j not in (0, 1728),
....: 4: p >= 5 and j == 1728,
....: 6: p >= 5 and j == 0,
....: 12: p == 3 and j in (0, 1728),
....: 24: p == 2 and j in (0, 1728),
....: 12: p == 3 and j == 0, # note 1728 == 0
....: 24: p == 2 and j == 0, # note 1728 == 0
....: }[len(Aut)]
True
sage: u,r,s,t = (F^4).random_element()
Expand Down Expand Up @@ -543,7 +508,7 @@ def _comparison_impl(left, right, op):
sage: w1 = E.isomorphism_to(F)
sage: w1 == w1
True
sage: w2 = F.automorphisms()[0] * w1
sage: w2 = F.automorphisms()[1] * w1
sage: w1 == w2
False
Expand Down Expand Up @@ -572,7 +537,21 @@ def _comparison_impl(left, right, op):
if lx != rx:
return richcmp_not_equal(lx, rx, op)

return baseWI.__richcmp__(left, right, op)
if op in (op_EQ, op_NE):
return richcmp(left.tuple(), right.tuple(), op)

# This makes sure that the identity and negation morphisms
# come first in a sorted list of WeierstrassIsomorphisms.
# More generally, we're making sure that a morphism and its
# negative appear next to each other, and that those pairs
# of isomorphisms satisfying u=+-1 come first.
def _sorting_key(iso):
v, w = iso.tuple(), (-iso).tuple()
i = 0 if (1,0,0,0) in (v,w) else 1
j = 0 if v[0] == 1 else 1 if w[0] == 1 else 2
return (i,) + min(v,w) + (j,) + v

return richcmp(_sorting_key(left), _sorting_key(right), op)

def _eval(self, P):
r"""
Expand Down

0 comments on commit 795383f

Please sign in to comment.