Changes between Version 9 and Version 12 of Ticket #13615

08/22/13 18:22:26 (7 years ago)

Thanks for the link, and congratulations to Kimi. I quickly read it through. I obviously didn't check line by line, but no doubt that you got the maths and the coding right. So, I'm ready to give a positive review.

I agree that the filename is not wonderful, but we did want to split off the general isogeny code now in ell_curve_isogeny from the special cases. Perhaps you are right that the "general" one belongs in the other file.

I agree, it is best to keep specific algorithms separate from the isogeny class. On second thought, all the algorithms in that file have something in common: they are generic algorithms only practical for small prime degrees (as you suggest in the title of the doc page). Maybe it's just the filename that needs to be changed. What about

least_semi_primitive is indeed internal. If I thught there would be other applications I might have suggested putting it with primitive roots and similar functions.

I had the same feeling about the variable isog_table and the function hyperelliptic_isogeny_data. Hence, I renamed all the three by adding an underscore in front of them.

I also

  • Edited the copyright banner by adding your names
  • Fixed the docstring of _least_semi_primitive()
  • Added ALGORITHM:: sections to some functions, pointing to the relevant chapter in Kimi's thesis.

I'm attaching my reviewer's patch. I'm working with the git repo, and I'm confused by this directory renaming stuff, so I guess it'll be easier if I link to my git branch on trac.

I'm not giving a positive review right away, so that you can review my patch, and give your opinion on renaming the file. Enjoy vacation!


  • Ticket #13615

    • Property Branch changed from to u/defeo/trac13615
    • Property Reviewers changed from John Cremona, Jenny Cooley, Samuele Anni to John Cremona, Jenny Cooley, Samuele Anni, Luca De Feo
  • Ticket #13615 – Description

    v9 v12  
    33In this ticket we will add extra functionality to be able to compute l-isogenies for arbitrary prime degrees.
    5 Apply: ell_curve_isogeny_split.patch, isogeny_genus_0.patch, trac_13615_review.patch