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

allow supplying a value of q for special_supersingular_curve() #38483

Conversation

yyyyx4
Copy link
Member

@yyyyx4 yyyyx4 commented Aug 7, 2024

This is a follow-up to #36665: There, a CM endomorphism is chosen automatically to construct a "special" supersingular elliptic curve. In some applications, it can be useful to explicitly supply the desired degree of such an endomorphism to Bröker's algorithm, which is made possible by this patch.

Note that some of the subroutines used in the new code paths are relatively fragile, which can cause the functionality added here to fail for some inputs, but this has been documented in the appropriate places.

Copy link

github-actions bot commented Aug 7, 2024

Documentation preview for this PR (built with commit b2cdaa0; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

@yyyyx4 yyyyx4 force-pushed the public/special_supersingular_curve_with_prescribed_q branch 2 times, most recently from d2257c1 to aa3c5e8 Compare August 10, 2024 21:57
@yyyyx4 yyyyx4 force-pushed the public/special_supersingular_curve_with_prescribed_q branch 2 times, most recently from 3f02366 to 72b99f9 Compare September 16, 2024 19:25
@yyyyx4 yyyyx4 force-pushed the public/special_supersingular_curve_with_prescribed_q branch from 72b99f9 to b2cdaa0 Compare October 6, 2024 13:49
Copy link
Contributor

@GiacomoPope GiacomoPope left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one tiny comment otherwise this patch seems nice.

assert False # should never happen
from sage.arith.misc import legendre_symbol
for q in map(ZZ, range(3,p,4)):
if not q.is_prime():
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it work just iterating through primes() here instead of checking primality?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think primes() supports the "step" argument (4). What could work is iterating through the primes and skipping the ones that aren't congruent to 3 modulo 4, but it's the same amount of code, and this part should be negligible in terms of computation anyway.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah my thinking was it's maybe faster to ask for the primes and check if it's the right congruence versus ensuring the right congruence and checking primality, but i agree that the majority of CPU time is not spent here (hence the positive review either way :) )

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the review!

vbraun pushed a commit to vbraun/sage that referenced this pull request Oct 9, 2024
…lar_curve()

    
This is a follow-up to sagemath#36665: There, a CM endomorphism is chosen
automatically to construct a "special" supersingular elliptic curve. In
some applications, it can be useful to explicitly supply the desired
degree of such an endomorphism to Bröker's algorithm, which is made
possible by this patch.

Note that some of the subroutines used in the new code paths are
relatively fragile, which can cause the functionality added here to fail
for some inputs, but this has been documented in the appropriate places.
    
URL: sagemath#38483
Reported by: Lorenz Panny
Reviewer(s): Giacomo Pope, Lorenz Panny
@vbraun vbraun merged commit b8bae02 into sagemath:develop Oct 12, 2024
22 of 23 checks passed
@yyyyx4 yyyyx4 deleted the public/special_supersingular_curve_with_prescribed_q branch October 12, 2024 11:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants