Implement basic j-invariants of Drinfeld modules

[DavidAyotte]

📚 Description

The goal of this PR is to implement basic j-invariants of Drinfeld module as defined by Potemine in

Potemine, I.Y. Minimal Terminal ℚ-Factorial Models of Drinfeld Coarse Moduli Schemes. Mathematical Physics, Analysis and Geometry 1, 171–191 (1998). https://doi.org/10.1023/A:1009724323513

These j-invariants allows to determine whether two Drinfeld Fq[T]-modules of arbitrary ranks are isomorphic or not.

@kryzar @ymusleh @xcaruso

📝 Checklist

• I have made sure that the title is self-explanatory and the description concisely explains the PR.
• [] I have linked an issue or discussion.
• I have created tests covering the changes.
• I have updated the documentation accordingly.

⌛ Dependencies

Depends on #35026 because this PR implements Drinfeld modules.

[codecov-commenter]

Codecov Report

Patch coverage: 96.51% and project coverage change: -0.01 ⚠️

Comparison is base (c00e6c2) 88.62% compared to head (b698d12) 88.61%.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #35057      +/-   ##
===========================================
- Coverage    88.62%   88.61%   -0.01%     
===========================================
  Files         2148     2148              
  Lines       398855   398936      +81     
===========================================
+ Hits        353480   353524      +44     
- Misses       45375    45412      +37     

Impacted Files	Coverage Δ
...function_field/drinfeld_modules/drinfeld_module.py	96.13% <96.51%> (+0.10%)	⬆️

... and 28 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[xcaruso]

Some comments:

• There are many failing doctests (the base field for Drinfeld modules is K, not A)
• I would say that you should recall what is a J-invariant and a J-invariant parameter in the documentation
• In the document of basic_j_invariants_parameters, you should write Return the list of basic j-invariants parameters
• In basic_j_invariant, you should check that parameters has the correct length
• Maybe, we should have a common method for j-invariant in rank 2 and j-invariants in higher rank, e.g. the attribute parameters could be optional when the rank is 2
• There are many copies of lists; maybe some of them could be avoided

[kryzar]

Thank you @DavidAyotte for the beautiful work!

I would propose also having a method basic_j_invariants (plural) that returns the list of all basic j-invariants, and a method j_invariant that returns the j-invariant if there is only one basic j-invariant, and that raises ValueError otherwise.

I agree with @xcaruso that the documentation should give details about basic j-invariants. You could also add an entry in the SageMath master bib. file.

Do you plan on implementing the k-invariants (Theorem 2.2.ii)?

This is by no means urgent. I will review the code in details when you estimate that it is ready for review!

[xcaruso]

I wonder if the methods j_invariant and basic_j_invariant could be merged: if the parameter is just an integer, we return the j-invariant, otherwise the basic j-invariant.

[kryzar]

I wonder if the methods j_invariant and basic_j_invariant could be merged: if the parameter is just an integer, we return the j-invariant, otherwise the basic j-invariant.

You mean basic_j_invariants (plural) and j_invariant, so that this unique method would return a list when there are multiple j-invariants, and just a scalar when the list has length 1? I'm fine with that.

[xcaruso]

No, I really meant j_invariant and basic_j_invariant (singular). But actually, merging all these methods (the three ones) might also be option, I believe.

[kryzar]

Oh OK, thanks for the clarification!

I would insist on keeping both:

• basic_j_invariant(self, parameters ) -> BaseFieldElement;
• and basic_j_invariants(self) -> [BaseFieldElement].

Besides, I agree that it is a bit strange to also have j_invariant(self) -> BaseFieldElement, and that the methods overlap a bit. However, I believe methods signatures need to remain consistent, and that it is quite pedagogical to have the three methods and good documentation.

[kryzar]

Looks like I found a bug:

sage: Fq = GF(25)
sage: A.<T> = Fq[]
sage: K.<z12> = Fq.extension(6)
sage: phi = DrinfeldModule(A, [z12, 0, 1])
sage: psi = DrinfeldModule(A, [z12, 0, 2])
sage: phi.j_invariant()
0
sage: psi.j_invariant()
0
sage: phi.basic_j_invariants_parameters()
[[26, 1]]
sage: phi.basic_j_invariant([26, 1])
1
sage: psi.basic_j_invariant([26, 1])
2
sage: phi.is_isomorphis_to(psi)
False

[xcaruso]

Two additional remarks:

• in basic_j_invariant_parameters, we sometime get parameters where $d_i = 0$ for some $i$; is there a reason for this (given that such parameters correspond to another parameter obtained by just removing the $d_i$ and the corresponding $k_i$)?
• why the method basic_j_invariants does not accept the keyword nonzero?

[xcaruso]

I implemented the changes I requested on my branch: https://github.com/xcaruso/sage/tree/drinfeld-module-jinvariants
Diff here: xcaruso/sage@144acb1...xcaruso:sage:drinfeld-module-jinvariants

Please merge into your branch if you agree with my proposal.

[github-actions]

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

[DavidAyotte]

In the previous commits, I have implemented the latest remarks of @xcaruso.

[xcaruso]

Thanks. I'm happy with your corrections and give again a positive review to this ticket.
(If the other reviewers disagree, please complain.)

[xcaruso]

Btw, are you sure that the doctests do not fail on 32 bit, now?

[DavidAyotte]

Btw, are you sure that the doctests do not fail on 32 bit, now?

I'm not sure, but I used significantly smaller doctests, so I assumed it will pass.

[kryzar]

but I used significantly smaller doctests

In any case, I think that's a good thing for the examples.

[kryzar]

Approved.

[DavidAyotte]

Is the build failures related to this PR?

[xcaruso]

I don't think so.
Maybe you can try to add a commit in order to relaunch the tests.

[kryzar]

(Quick announcement that I will be away until August 21st.)