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

Refactor Drinfeld modules subclasses #36325

Merged
merged 18 commits into from Oct 14, 2023

Conversation

DavidAyotte
Copy link
Member

@DavidAyotte DavidAyotte commented Sep 25, 2023

The goal of this PR is to refactor the Drinfeld modules subclasses. In particular, we rename FiniteDrinfeldModule into DrinfeldModule_finite and we create a new class named DrinfeldModule_charzero. The second class denotes a Drinfeld module which is defined over a ring of $\mathbb{F}_q[T]$-characteristic zero. More conceptually, we propose the following structure:

    DrinfeldModule
        - DrinfeldModule_finite
        - DrinfeldModule_charzero

The user will only have to interact with the metaclass DrinfeldModule.

This refactorisation ensures that the exponential, the logarithm and the Goss polynomials methods always works as these are not defined in the finite case (and thus removes the required check).

In the near future, we plan to implement DrinfeldModule_charzero_rank_one as a subclass of DrinfeldModule_charzero and implement their associated $A$-lattice.

CC: @kryzar @xcaruso @ymusleh

📝 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.

⌛ Dependencies

@xcaruso
Copy link
Contributor

xcaruso commented Sep 25, 2023

I agree that this organization sounds better!

I just have a little concern about the name DrinfeldModule_complex since your definition allows Drinfeld modules defined over $\mathbb F_q(T)$ itself (for example) for which the terminology complex is not well suited, IMHO. Why not just DrinfeldModule_char_zero?

In the case of Drinfeld modules defined over finite extensions of $\mathbb F_q(T)$, I think than more methods could be available (e.g. has_good_reduction, Lfunction, etc.), so that we could eventually have a special class for this setting.

@DavidAyotte
Copy link
Member Author

The idea was to denote any Drinfeld module whose base field can be embedded in $\mathbb{C}_{\infty}$ and thus all the methods related to the analytic side make sense. However, I don't think that simply checking the characteristic is enough in that case, but then I'm not sure which check to use in SageMath. I could maybe restrict to finite extension of $\mathbb{F}_q(T)$, but this would not take into account Drinfeld modules over infinite extension, for example over $\mathbb{F}_q((1/T))$ the completion of $\mathbb{F}_q(T)$ w.r.t $1/T$:

sage: A = GF(3)['T']
sage: L.<s> = LaurentSeriesRing(GF(3))
sage: phi = DrinfeldModule(A, [1/s, 1])
sage: phi
Drinfeld module defined by T |--> t + s^-1
sage: phi.base()
Laurent Series Ring in s over Finite Field of size 3 over its base

For Drinfeld modules over finite extension of $\mathbb{F}_q(T)$, which name do you suggest?

@kryzar
Copy link
Contributor

kryzar commented Sep 25, 2023

So we can't just copy elliptic curves, because they have the following modules:

ell_local_data.py
ell_torsion.py
ell_wp.py
ell_generic.py
ell_rational_field.py
ell_curve_isogeny.py
ell_tate_curve.py
ell_modular_symbols.py
ell_finite_field.py
ell_padic_field.py
ell_field.py
ell_number_field.py
ell_point.py
ell_egros.py

The situation is tricky because clearly our methods work, in theory, only for Drinfeld modules defined on a subfield of the "complex functions" $\mathbb C_\infty$. However, we do not even know how to represent elements in this field. The same way we have a class for Drinfeld modules over finite fields (and not over a field of positive characteristic!), we would want a class for Drinfeld modules over the complex functions, and as such, I would find DrinfeldModule_complex to be suitable. I agree however that it is a bit misleading when the coefficients leave in $\mathbb F_q(T)$, the equivalent of the rational numbers. In conclusion, I don't have a strong preference between the two possibilities. It would be nice to have DrinfeldModule_complex, but realistically we can only ensure DrinfeldModule_zero_characteristic (or DrinfeldModule_generic_characteristic as I think David had proposed at some point).

@ymusleh
Copy link
Contributor

ymusleh commented Sep 27, 2023

I do agree that this naming scheme seems better and more extensible, though I'm not sure about the example you proposed regarding implementing DrinfeldModule_rank_one as a subclass since that could cause confusion. Perhaps DrinfeldModule_complex_rank_one.

I wouldn't hate the name DrinfeldModule_analytic over DrinfeldModule_complex (as I agree with Xavier that the use of complex here is a bit misleading) but perhaps that is even more confusing.

Also, I will get back to the characteristic polynomial next week, and the endomorphism ring shortly after; my apologies for the delay.

@xcaruso
Copy link
Contributor

xcaruso commented Sep 27, 2023

Although I definitely agree that the name of the methods really needs to be mathematically relevant, I think that it's more important that the names of the classes are programmatically relevant given that they are completely invisible to the user.

So far, all analytic methods we have apply to any $\mathbb F_q[T]$-field of characteristic zero and I would say moreover that all reasonable $\mathbb F_q[T]$-fields the user will be constructing in Sage will always embed canonically into $\mathbb C_\infty$. Therefore, I think it's okay (at least for now) to make the confusion between "analytic"/"complex" and "characteristic zero" for the name of the classes. The second option sounds a little bit better to me because it's more generic.

As a summary, the hierarchy I propose is:

DrinfeldModule
    DrinfeldModule_finite
    DrinfeldModule_charzero
        DrinfeldModule_function_field   # for later

@DavidAyotte
Copy link
Member Author

Thank you both for your suggestions. They make a lot of sense.

I do agree that this naming scheme seems better and more extensible, though I'm not sure about the example you proposed regarding implementing DrinfeldModule_rank_one as a subclass since that could cause confusion. Perhaps DrinfeldModule_complex_rank_one.

Yes that's a good idea to distinguish between generic characteristic and finite characteristic! I think I will follow Xavier's naming convention and use DrinfeldModule_charzero_rank_one

Also, let me extend a bit on my plans for the rank 1 case. More precisely, I plan (in a following PR) to add methods for computing a period (i.e. a generator of the associated $\mathbb{F}_q[T]$-lattice) of that Drinfeld module. So it would be possible to do something like:

    sage: A = GF(3)['T']
    sage: K.<T> = Frac(A)
    sage: rho = DrinfeldModule(A, [T, 1])  # Carlitz module
    sage: rho.period()
    # Return a Carlitz period generating the associated rank one A-lattice.

The rank two case would also be possible in some cases I think, but I would need to read a bit more on the subject.

@kryzar
Copy link
Contributor

kryzar commented Sep 29, 2023

Although I definitely agree that the name of the methods really needs to be mathematically relevant, I think that it's more important that the names of the classes are programmatically relevant given that they are completely invisible to the user.

So far, all analytic methods we have apply to any Fq[T]-field of characteristic zero and I would say moreover that all reasonable Fq[T]-fields the user will be constructing in Sage will always embed canonically into C∞. Therefore, I think it's okay (at least for now) to make the confusion between "analytic"/"complex" and "characteristic zero" for the name of the classes. The second option sounds a little bit better to me because it's more generic.

As a summary, the hierarchy I propose is:

DrinfeldModule
    DrinfeldModule_finite
    DrinfeldModule_charzero
        DrinfeldModule_function_field   # for later

So a Drinfeld module over $\mathbb F_q(T)$ would be a DrinfeldModule_function_field and a Drinfeld module over $\mathbb F_q((\frac 1 T))$ would be a DrinfeldModule_charzero? That's fine by me.

@xcaruso
Copy link
Contributor

xcaruso commented Sep 30, 2023

So a Drinfeld module over Fq(T) would be a DrinfeldModule_function_field and a Drinfeld module over Fq((1T)) would be a DrinfeldModule_charzero? That's fine by me.

Yes.
Maybe later, if it becomes useful, we can create another subclass DrinfeldModule_analytic of DrinfeldModule_charzero for specific methods that will apply only to Drinfeld modules over $\mathbb C_\infty$.

@kryzar
Copy link
Contributor

kryzar commented Oct 2, 2023

Thanks! Works for me.

@github-actions
Copy link

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

@DavidAyotte DavidAyotte marked this pull request as ready for review October 10, 2023 19:20
DavidAyotte and others added 3 commits October 11, 2023 09:51
…ld_module.py

Co-authored-by: Xavier Caruso <xcaruso@users.noreply.github.com>
…ld_module.py

Co-authored-by: Xavier Caruso <xcaruso@users.noreply.github.com>
…ld_module.py

Co-authored-by: Xavier Caruso <xcaruso@users.noreply.github.com>
@xcaruso
Copy link
Contributor

xcaruso commented Oct 11, 2023

There are failing doctests but I think that there are not related to this ticket.
So I give a positive review.

@DavidAyotte
Copy link
Member Author

No the failing doctests are not related. Thanks for the review!

@vbraun vbraun merged commit 6ab9c83 into sagemath:develop Oct 14, 2023
18 of 26 checks passed
@mkoeppe mkoeppe added this to the sage-10.2 milestone Oct 14, 2023
@kryzar
Copy link
Contributor

kryzar commented Oct 16, 2023

So I give a positive review.

Hi Xavier. Next time could you wait a little for changing the label to s: positive review? Even though I did not review the PR as soon as it was marked ready, I would still have wanted to do so!

@xcaruso
Copy link
Contributor

xcaruso commented Oct 17, 2023

Well, feel free to open a new PR if you want to do some modifications.

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

6 participants