Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Construct isogenies graph in elliptic curves #16942

Open
sagetrac-sbesnier mannequin opened this issue Sep 7, 2014 · 38 comments
Open

Construct isogenies graph in elliptic curves #16942

sagetrac-sbesnier mannequin opened this issue Sep 7, 2014 · 38 comments

Comments

@sagetrac-sbesnier
Copy link
Mannequin

sagetrac-sbesnier mannequin commented Sep 7, 2014

Let E be an EllipticCurve.

We can not yet construct the graph of the j-invariants of the curves which are isogenous to E. This ticket fills this little gap.

CC: @defeo @JohnCremona @jpflori

Component: elliptic curves

Author: Sébastien Besnier

Branch/Commit: public/ticket/16942 @ 4a943f8

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

@sagetrac-sbesnier sagetrac-sbesnier mannequin added this to the sage-6.4 milestone Sep 7, 2014
@sagetrac-sbesnier
Copy link
Mannequin Author

sagetrac-sbesnier mannequin commented Sep 7, 2014

New commits:

0d583b4First try to build the volcanoe of an elliptic curve.
777a5c7First draft for isogenies_graph

@sagetrac-sbesnier
Copy link
Mannequin Author

sagetrac-sbesnier mannequin commented Sep 7, 2014

Branch: u/sbesnier/ticket/16942

@sagetrac-sbesnier
Copy link
Mannequin Author

sagetrac-sbesnier mannequin commented Sep 7, 2014

Commit: 777a5c7

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 7, 2014

Branch pushed to git repo; I updated commit sha1. New commits:

e6f92a6Add some doc tests

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 7, 2014

Changed commit from 777a5c7 to e6f92a6

@fchapoton
Copy link
Contributor

comment:3

I have made a cosmetic cleanup of the branch.

But it does not work, test do not pass for me. The doc should say (and the tests should also care) that this is optional, because requiring a database.


New commits:

167a6baMerge branch 'u/sbesnier/ticket/16942' of ssh://trac.sagemath.org:22/sage into 16942
a4740c6trac #16942 cosmetic cleanup

@fchapoton
Copy link
Contributor

Changed branch from u/sbesnier/ticket/16942 to public/ticket/16942

@fchapoton
Copy link
Contributor

Changed commit from e6f92a6 to a4740c6

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 5, 2014

Changed commit from a4740c6 to b4bf82d

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 5, 2014

Branch pushed to git repo; I updated commit sha1. New commits:

b4bf82dAdd note about the necessity of having the db_modular_polynomials database.

@fchapoton
Copy link
Contributor

comment:5

Do you want to put this in the state "needs review" ?

@sagetrac-sbesnier
Copy link
Mannequin Author

sagetrac-sbesnier mannequin commented Oct 9, 2014

comment:6

I thought I did it... Thank you.

@fchapoton
Copy link
Contributor

comment:7

It seems that db_modular_polynomials is not available using
sage -i db_modular_polynomials

I think it would be good to explain how to install this database in the doc, if there is some other way.

And AttributeError should be ValueError instead.

And .. note:: should be .. NOTE::

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 28, 2014

Changed commit from b4bf82d to 53fbe4b

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 28, 2014

Branch pushed to git repo; I updated commit sha1. New commits:

537097fMerge branch 'public/ticket/16942' into 6.5.b4
53fbe4btrac #16942 details

@fchapoton

This comment has been minimized.

@fchapoton fchapoton modified the milestones: sage-6.4, sage-6.8 Jul 24, 2015
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 24, 2015

Branch pushed to git repo; I updated commit sha1. New commits:

153b354Merge branch 'public/ticket/16942' of trac.sagemath.org:sage into 6.8.rc1
5ebe464trac #16942 no longer depending on database

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 24, 2015

Changed commit from 53fbe4b to 5ebe464

@fchapoton
Copy link
Contributor

comment:11

This now does no longer depends on any database.

@pjbruin
Copy link
Contributor

pjbruin commented Jul 24, 2015

comment:12

Should the method perhaps be renamed isogeny_graph()? I think "isogeny graph" is much more widely used than "isogenies graph" (and Google agrees).

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 24, 2015

Branch pushed to git repo; I updated commit sha1. New commits:

e5b72e8changing name

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 24, 2015

Changed commit from 5ebe464 to e5b72e8

@JohnCremona
Copy link
Member

comment:14

I did not understand the purpose of this ticket from its description, but I do now that I read the code. For elliptic curves over Q and other number fields we have a class for isogeny classes, which has (amongst other methods) a method for displaying the graph. So I wonder whether it might be possible to put this new code under the same umbrella. But that would be more work, so I do not want to delay this.

Certainly "isogeny graph" is a better term.

@pjbruin
Copy link
Contributor

pjbruin commented Jul 24, 2015

comment:15

I don't have time to really review this now (last working day before my holidays), but why does this return a directed graph? If there is an isogeny in one direction, there always is the dual isogeny in the opposite direction.

@JohnCremona
Copy link
Member

comment:16

Good point. And the behaviour for "l = characteristic" should be documented; probably not allowed?

Using the modular polynomial is one way to do it. You could also make use of the existing method E.isogenies_prime_degree(). But in fact as it stands this function is not a method of elliptic curves at all, the input is really just and element j of a finite field. So surely there could be a separate function (not a class method) with input an element of F_q and a prime l, and output a graph on some subset of Fq, and then the elliptic curve class method would call that on its j-invariant.

@pjbruin
Copy link
Contributor

pjbruin commented Jul 24, 2015

comment:17

Replying to @JohnCremona:

Using the modular polynomial is one way to do it. You could also make use of the existing method E.isogenies_prime_degree(). But in fact as it stands this function is not a method of elliptic curves at all, the input is really just and element j of a finite field. So surely there could be a separate function (not a class method) with input an element of F_q and a prime l, and output a graph on some subset of Fq, and then the elliptic curve class method would call that on its j-invariant.

Actually, the method should perhaps even (optionally?) return the graph of actual elliptic curves instead of the graph of j-invariants. It is likely that the user will want to know exactly which curves over the base field appear in the isogeny graph; if you just have the j-invariants, you only know them up to twist.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 24, 2015

Changed commit from e5b72e8 to ff48452

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 24, 2015

Branch pushed to git repo; I updated commit sha1. New commits:

ff48452trac #16942 check that l is not equal to p (characteristic)

@fchapoton fchapoton modified the milestones: sage-6.8, sage-6.9 Jul 29, 2015
@fchapoton
Copy link
Contributor

Changed author from sbesnier to Sébastien Besnier

@fchapoton
Copy link
Contributor

comment:20

I am not really the man to give a positive review, given that I am not an expert.

There seems to remain an issue about directed graph versus undirected graph..

@fchapoton fchapoton modified the milestones: sage-6.9, sage-6.10 Nov 10, 2015
@pjbruin
Copy link
Contributor

pjbruin commented Nov 11, 2015

comment:21

Replying to @fchapoton:

There seems to remain an issue about directed graph versus undirected graph..

Yes, and it should also be possible to get the actual elliptic curves in the isogeny class, not just their j-invariants (see comment:17). It would probably be a good idea to look at the method EllipticCurve_rational_field.isogeny_graph() and try to keep the interface consistent with that.

@JohnCremona
Copy link
Member

comment:23

Replying to @pjbruin:

Replying to @fchapoton:

There seems to remain an issue about directed graph versus undirected graph..

Yes, and it should also be possible to get the actual elliptic curves in the isogeny class, not just their j-invariants (see comment:17). It would probably be a good idea to look at the method EllipticCurve_rational_field.isogeny_graph() and try to keep the interface consistent with that.

Agreed. There will be differences: over number fields isogeny classes are finite; in non-CM cases there is a unique degree of cyclic isogeny between any 2 curves in the class, and the graph we draw uses only those of prime degree which is enough to connect the graph. In CM cases, the situation is very similar to ordinary e.c. over finite fields: isogenies come in 2 types, "horizontal" (between curves whose endomorphism rings are the same order), and "vertical" (between curves whose endo rings are different orders in the same imaginary quadratic field). Vertical isogenies are as before, with a unique cyclic degree; but the horizontal ones do not have unique degrees, in the sense that if two curves hace CM by the same imaginary quadratic order, of discriminant D say, then the set of degrees of isogenies between them is the set of positive integers represented by some binary quadratic form of discriminant D. I toyed with the idea of using the quadratic form as a label for the edges in this case, but in the end went for the smallest prime represented by the form.

Over a finite field, in the ordinary case,the curves in an isogeny class all have the same number of points and their endo rings are all orders in the same i.q.field, and one can again distinguish between horizontal and vertical isogenies. The graph is usually referred to as a "volcano" on account of its shape, where "vertical" and "horizontal" now have physical meaning. If coders are up to the challenge we could have some spectacular isogeny graphs! but getting the layout to look good would be a challenge.

And then there are supersingular curves. Here, for any two isogenous curves, all but finitely many positive integers arise as isogeny degrees between them (see the last chapter of Kohel's thesis), and I don't see any reason for not just showing a complete graph.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 26, 2017

Changed commit from ff48452 to 4a943f8

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 26, 2017

Branch pushed to git repo; I updated commit sha1. New commits:

4a943f8Merge branch 'public/ticket/16942' in 8.1.b3

@defeo
Copy link
Member

defeo commented Sep 4, 2017

comment:26

Hello. Thanks @fchapoton for taking the time to rebase this.

But, honestly, shouldn't we say this code is not going to be merged?

I am sure there is a lot of interest right now in isogeny graphs over finite fields (because post-quantum crypto, you know), and it would be good if Sage offered some functionality. However this code hardly covers all interesting cases, and it has design issues.

If we could agree on the correct API, I'd be happy to rewrite the code. Let me summarize the issues:

  • A function on Fq which outputs a graph labeled by j-invariants, VS a function on elliptic curves which outputs a graph labeled by curves. My opinion is that both are useful, so we could implement both.

  • Undirected VS directed graph. The problem is that, although generically these graphs are simple undirected graphs, there are lots of pathological cases:

    • Self-loops,
    • multi-edges,
    • j-invariants 0 and 1728 are especially nasty (same dual for many isogenies).
  • The method: modular polynomials VS isogenies_prime_degree(). As far as I recall, isogenies_prime_degree() factors the division polynomial. It is more expensive, but it requires no optional database (and can in principle cover any degree, although it becomes unrealistic when degrees get large).

Let me add one more issue: fool-proofness. The size of these graphs is polynomial in the field SIZE! I'm sure a lot of beginners would create a cyptographic-size finite field and then ask for an isogeny graph. This would of course take forever. Maybe we could print a warning when the expected size is too large.

@JohnCremona, @pjbruin, comments?

@JohnCremona
Copy link
Member

comment:27

See my comments on #30199.

@JohnCremona
Copy link
Member

comment:28

By the way, Sage's optional database of David Kohel's modular polynomials is tiny. Andrew Sutherland has a very much larger database.

@defeo
Copy link
Member

defeo commented Dec 16, 2020

comment:29

Maybe we can close this ticket, now that #30199 is going to get in?

Re the modular polynomial database, yes, I had been toying with repackaging it for a while. I hope to be able to finish that work some day...

Maybe it would be good to create a meta-ticket summarizing all wanted functionality regarding isogeny classes and graphs? I can do that if you like.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants