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

Improve morphisms between Drinfeld modules #35527

Merged
merged 34 commits into from Jul 20, 2023

Conversation

xcaruso
Copy link
Contributor

@xcaruso xcaruso commented Apr 16, 2023

We implement several methods for arbitrary morphisms between Drinfeld modules.
Especially, we implement the computation of norms and characteristic polynomials.

@ymusleh @DavidAyotte @kryzar

📚 Description

Here is a list of the most important added methods:

  • hom: fast interface for creating morphisms between Drinfeld modules
  • _composition_/inverse: composition and inverse of morphisms between Drinfeld modules
  • is_isomorphic: check whether two Drinfeld modules are isomorphic either over the base field or over an algebraic closure
  • norm: computation of the norm of a morphism between Drinfeld modules
  • dual_isogeny: computation of the dual isogeny
  • characteristic_polynomial/charpoly: computation of the characteristic polynomial of an endomorphism between Drinfeld modules

We also implement addition of morphisms and the action of the function field $A$; this gives the structure of left $A$-module on the set of morphisms between two given Drinfeld modules.

📝 Checklist

  • The title is concise, informative, and self-explanatory.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation accordingly.

@DavidAyotte
Copy link
Member

DavidAyotte commented Apr 17, 2023

7c8861c: This is indeed much more faster than using basic j-invariants.

@DavidAyotte
Copy link
Member

One feature that could be also implemented is the conjugate:

sage: A = GF(5)['T']
sage: K.<T> = Frac(A)
sage: phi = DrinfeldModule(A, [T, 1, 1])
sage: phi.conjugate(u)  # u nonzero
# return the Drinfeld module T |--> u^(-1) \phi_T u

@xcaruso
Copy link
Contributor Author

xcaruso commented Apr 17, 2023

One feature that could be also implemented is the conjugate:

sage: A = GF(5)['T']
sage: K.<T> = Frac(A)
sage: phi = DrinfeldModule(A, [T, 1, 1])
sage: phi.conjugate(u)  # u nonzero
# return the Drinfeld module T |--> u^(-1) \phi_T u

Sure.
It's the same than phi.velu(u) but it's probably better to have both.

@xcaruso
Copy link
Contributor Author

xcaruso commented May 20, 2023

@kryzar @DavidAyotte @ymusleh
It's time, I think, to start reviewing this ticket. Would you be willing to do this?

@ymusleh
Copy link
Contributor

ymusleh commented May 20, 2023

Not sure if I can act as a formal reviewer since I'm not part of the Sage community and haven't had a PR merged, but I'll look over the code and provide feedback.

@kryzar
Copy link
Contributor

kryzar commented May 22, 2023

@kryzar @DavidAyotte @ymusleh It's time, I think, to start reviewing this ticket. Would you be willing to do this?

Yep.

@ymusleh
Copy link
Contributor

ymusleh commented Jun 19, 2023

Just some minor comments, otherwise I think this is good for a positive review.

Co-authored-by: David Ayotte <34245930+DavidAyotte@users.noreply.github.com>
@DavidAyotte
Copy link
Member

Thanks David for your comments. I addressed all of them. Going through the code, I also noticed that inversions of general isomorphisms (which are not automorphisms) were not implemented. So I fixed this.

Thanks for the update. For my part, I am happy with the current state of this PR.

@ymusleh
Copy link
Contributor

ymusleh commented Jun 21, 2023

I'm also happy with the state of this PR.

Copy link
Contributor

@kryzar kryzar left a comment

Choose a reason for hiding this comment

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

Did my review (thank you for your patience).

xcaruso and others added 11 commits July 6, 2023 13:45
Co-authored-by: Antoine Leudière <clapped.hesitancy332@anonaddy.me>
Co-authored-by: Antoine Leudière <clapped.hesitancy332@anonaddy.me>
…_module.py

Co-authored-by: Antoine Leudière <clapped.hesitancy332@anonaddy.me>
…_module.py

Co-authored-by: Antoine Leudière <clapped.hesitancy332@anonaddy.me>
Co-authored-by: Antoine Leudière <clapped.hesitancy332@anonaddy.me>
Co-authored-by: Antoine Leudière <clapped.hesitancy332@anonaddy.me>
Co-authored-by: Antoine Leudière <clapped.hesitancy332@anonaddy.me>
Co-authored-by: Antoine Leudière <clapped.hesitancy332@anonaddy.me>
Co-authored-by: Antoine Leudière <clapped.hesitancy332@anonaddy.me>
@xcaruso
Copy link
Contributor Author

xcaruso commented Jul 9, 2023

@kryzar
Thanks for your review. I think I adressed all you comments.

Copy link
Contributor

@kryzar kryzar left a comment

Choose a reason for hiding this comment

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

Thank you very much @xcaruso for writing this contribution and taking the tame to address my comments! I approve the changes.

@xcaruso
Copy link
Contributor Author

xcaruso commented Jul 10, 2023

Thanks for your reviews.
I give a positive review to this PR; please revert it if you disagree.

@github-actions
Copy link

Documentation preview for this PR (built with commit 5f76571; changes) is ready! 🎉

@vbraun vbraun merged commit 6da69c7 into sagemath:develop Jul 20, 2023
10 checks passed
@mkoeppe mkoeppe added this to the sage-10.1 milestone Jul 20, 2023
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.

None yet

7 participants