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

Add method for computing isogenies between Drinfeld modules #35386

Draft
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

ymusleh
Copy link
Contributor

@ymusleh ymusleh commented Mar 30, 2023

πŸ“š Description

This description assumes the reader is familiar with the general theory of Drinfeld modules. In particular, we will fix our base field $\mathbb{F}_q$, and take the ring of regular functions $A = \mathbb{F}_q[T]$. The characteristic map has codomain a field $K$, and let $\tau$ denote the skew indeterminate such that $\tau z = z^q \tau$ for $z \in K$. Then a Drinfeld module $\phi$ is defined by a choice of $\phi_T \in K[\tau]$ whose constant coefficient is $\gamma(T)$.

Given two Drinfeld $\mathbb{F}_q[T]$-modules $\phi$ and $\psi$, an isogeny $\iota: \phi \to \psi$ is a non-zero $\iota \in K[\tau]$ such that $\iota \phi_a = \psi_a \iota$ for all $a \in \mathbb{F}_q[T]$. The space of all such morphisms is denoted $\textnormal{Hom}(\phi, \psi)$.

A polynomial time algorithm for computing all isogenies of fixed degree between Drinfeld modules was given by Wesolowski. This PR implements the Wesolowski algorithm. Moreover, the approach can be extended to compute a basis for $\textnormal{Hom}(\phi, \psi)$ over $\mathbb{F}_q[\tau^n]$. This PR implements this algorithm as well.

B. Wesolowski. 2022. Computing isogenies between finite Drinfeld modules. Cryptology ePrint Archive, Paper 2022/438. https://eprint.iacr.org/2022/438

πŸ“ 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

None

@kryzar @xcaruso @DavidAyotte @schost

@ymusleh ymusleh marked this pull request as draft March 30, 2023 00:24
@kryzar
Copy link
Contributor

kryzar commented Mar 30, 2023

Implementing the algorithm that broke my dreams. Nice.

@ymusleh
Copy link
Contributor Author

ymusleh commented Mar 30, 2023

I'm thinking it probably makes more sense to have two methods: one that returns a basis for the $\mathbb{F}_q$-space of isogenies and another that just selects one as this method does. Probably doesn't make sense to return it as a vector space object but that too could be a possibility.

@kryzar
Copy link
Contributor

kryzar commented Mar 30, 2023

I'm thinking it probably makes more sense to have two methods: one that returns a basis for the Fq-"space" (excluding 0) of isogenies and another that just selects one as this method does. Probably doesn't make sense to return it as a vector space object but that too could be a possibility.

I will think about this and come back to you!

@xcaruso
Copy link
Contributor

xcaruso commented Mar 30, 2023

Have you thought about implementing this as methods of the homset? I mean:

sage: H = Hom(phi, psi)   # create the Hom space
sage: H.basis()           # return a basis
sage: H.an_element()      # return an isogeny
sage: H.random_element()  # return an random isogeny between phi and psi

@kryzar
Copy link
Contributor

kryzar commented Mar 30, 2023

Have you thought about implementing this as methods of the homset? I mean:

sage: H = Hom(phi, psi)   # create the Hom space
sage: H.basis()           # return a basis
sage: H.an_element()      # return an isogeny
sage: H.random_element()  # return an random isogeny between phi and psi

I was thinking exactly about this. There are many details to figure out, though, and I will definitely think more deeply about the interface.

What's the difference between an_element and random_element?

@kryzar
Copy link
Contributor

kryzar commented Mar 30, 2023

(And of course elements of the basis should be true DrinfeldModuleMorphism.)

@xcaruso
Copy link
Contributor

xcaruso commented Mar 30, 2023

What's the difference between an_element and random_element?

H.random_element() is supposed to be uniformly distributed (or, at least, distributed wrt a reasonable law).

H.an_element() just returns one element, which is in general always the same (often 0, or 1 when it makes sense).

@ymusleh
Copy link
Contributor Author

ymusleh commented Mar 30, 2023

Ok, I can move the method to homset.

For an_element, I will likely just take the first element of the basis unless there are other suggestions.

It might be worthwhile to include a non_zero option as well for random_element.

@ymusleh ymusleh marked this pull request as ready for review March 31, 2023 00:18
for i in range(d + 1)])))
return basis

def random_element(self, degree, seed=None):
Copy link
Contributor

Choose a reason for hiding this comment

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

The usual signature of random_element is

    def random_element(self)

So, I think that degree should have a default here (I don't know nonetheless what it should be) and also, it's probably not the right place to give the possibility to define a seed.

Copy link
Contributor

Choose a reason for hiding this comment

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

In fact, after my next comment, probably we can just forget about degree.

@xcaruso
Copy link
Contributor

xcaruso commented Apr 3, 2023

In the case of endomorphisms, the set $\text{End}(\phi)$ has a rich structure: it is a simple central algebra over $\mathbb{F}_q(\tau^d)$ and we know all its Hasse invariants. In particular, it is naturally a finite-dimensional vector space over $\mathbb{F}_q(\tau^d)$; I think it would make sense to return a basis over this larger field (in order, firstly, to speed up computations and, secondly, to avoid this artificial bound on the degree).

Actually, I think that what precedes also apply for general isogenies: composing by $\tau^d$ (on the left or on the right, it commutes) defines a structure of $\mathbb{F}_q(\tau^d)$-vector space on $\text{Hom}(\phi,\psi)$ (it is even a module over $\text{End}(\phi) \simeq \text{End}(\psi)$, over which it is probably locally free of rank 1) which we can use to have shorter bases.

@ymusleh
Copy link
Contributor Author

ymusleh commented Apr 4, 2023

In light of some discussion, I think the best approach here is to drop seed from random_element and keep degree in all methods but set its default to n - 1.

@ymusleh
Copy link
Contributor Author

ymusleh commented Apr 4, 2023

Further to my previous point, I would like to add a method to make it easier to access $[K: \mathbb{F}_q]$. Currently, to access it as a default parameter, I need to write
def random_element(self, degree=self.domain().base_over_constants_field().degree(self.domain()._Fq)-1):

Which is quite a lot for a simple and rather important constant, and I don't believe there is an easier way to do this (please inform me if there is!). If someone else wishes to do this that would be fine with me, otherwise I'm happy to either open a separate PR or add it onto this one.

@kryzar
Copy link
Contributor

kryzar commented Apr 4, 2023

Further to my previous point, I would like to add a method to make it easier to access [K:Fq]. Currently, to access it as a default parameter, I need to write def random_element(self, degree=self.domain().base_over_constants_field().degree(self.domain()._Fq)-1):

Which is quite a lot for a simple and rather important constant, and I don't believe there is an easier way to do this (please inform me if there is!). If someone else wishes to do this that would be fine with me, otherwise I'm happy to either open a separate PR or add it onto this one.

Good idea! I would suggest only creating a private attribute, e.g. self._base_degree_over_constants rather than creating a method. What do you think?

@ymusleh
Copy link
Contributor Author

ymusleh commented Apr 4, 2023

Further to my previous point, I would like to add a method to make it easier to access [K:Fq]. Currently, to access it as a default parameter, I need to write def random_element(self, degree=self.domain().base_over_constants_field().degree(self.domain()._Fq)-1):
Which is quite a lot for a simple and rather important constant, and I don't believe there is an easier way to do this (please inform me if there is!). If someone else wishes to do this that would be fine with me, otherwise I'm happy to either open a separate PR or add it onto this one.

Good idea! I would suggest only creating a private attribute, e.g. self._base_degree_over_constants rather than creating a method. What do you think?

That is fine with me. I think it's small enough that I might just tack it onto the char poly pull request, but if there is another preferred strategy let me know.

@kryzar
Copy link
Contributor

kryzar commented Apr 4, 2023

Further to my previous point, I would like to add a method to make it easier to access [K:Fq]. Currently, to access it as a default parameter, I need to write def random_element(self, degree=self.domain().base_over_constants_field().degree(self.domain()._Fq)-1):
Which is quite a lot for a simple and rather important constant, and I don't believe there is an easier way to do this (please inform me if there is!). If someone else wishes to do this that would be fine with me, otherwise I'm happy to either open a separate PR or add it onto this one.

Good idea! I would suggest only creating a private attribute, e.g. self._base_degree_over_constants rather than creating a method. What do you think?

That is fine with me. I think it's small enough that I might just tack it onto the char poly pull request, but if there is another preferred strategy let me know.

That's perfectly reasonable to me. Thank you very much for this initiative.

@xcaruso
Copy link
Contributor

xcaruso commented Apr 5, 2023

In light of some discussion, I think the best approach here is to drop seed from random_element and keep degree in all methods but set its default to n - 1.

As I said, I would argue for just removing this attribute degree (which is justified if we return a basis over $\mathbb F_q(\tau^n)$). What are your arguments for keeping it?

@ymusleh
Copy link
Contributor Author

ymusleh commented Apr 5, 2023

In light of some discussion, I think the best approach here is to drop seed from random_element and keep degree in all methods but set its default to n - 1.

As I said, I would argue for just removing this attribute degree (which is justified if we return a basis over $\mathbb F_q(\tau^n)$). What are your arguments for keeping it?

To me at least, it seems reasonable that a user might be interested in morphisms up to a particular degree (for example, they may wish to only compute isomorphisms). It doesn't seem to be a huge loss to give users the option, particularly for large n, though I think it should be documented that you degree n-1 gives you a basis over $\mathbb F_q(\tau^n)$ and this is the preferred way to construct high-degree isogenies.

@kryzar
Copy link
Contributor

kryzar commented Apr 5, 2023

Why not do both? The method could, by default, return a basis over $\mathbb{F}_q[\tau^n]$ (somehow curly brackets do not print), and, if a parameter degree is given, return a basis over $\mathbb{F}_q$ for the morphisms of fixed input degree.

sage: Hom(phi, psi).basis()  # Basis over Fq{tau^n}
...
sage: Hom(phi, psi).basis(degree=3)  # Basis over Fq for morphsism with tau-degree 3
...

@ymusleh
Copy link
Contributor Author

ymusleh commented Apr 5, 2023

Why not do both? The method could, by default, return a basis over Fq[Ο„n] (somehow curly brackets do not print), and, if a parameter degree is given, return a basis over Fq for the morphisms of fixed input degree.

sage: Hom(phi, psi).basis()  # Basis over Fq{tau^n}
...
sage: Hom(phi, psi).basis(degree=3)  # Basis over Fq for morphsism with tau-degree 3
...

I think this is more or less what the current setup does with the default degree being n-1.

@kryzar
Copy link
Contributor

kryzar commented Apr 5, 2023

Why not do both? The method could, by default, return a basis over Fq[Ο„n] (somehow curly brackets do not print), and, if a parameter degree is given, return a basis over Fq for the morphisms of fixed input degree.

sage: Hom(phi, psi).basis()  # Basis over Fq{tau^n}
...
sage: Hom(phi, psi).basis(degree=3)  # Basis over Fq for morphsism with tau-degree 3
...

I think this is more or less what the current setup does with the default degree being n-1.

Default set to n - 1 seems more arbitrary than returning a basis over $\mathbb{F}_q[\tau^n]$, I believe.

@ymusleh
Copy link
Contributor Author

ymusleh commented Apr 5, 2023

Why not do both? The method could, by default, return a basis over Fq[Ο„n] (somehow curly brackets do not print), and, if a parameter degree is given, return a basis over Fq for the morphisms of fixed input degree.

sage: Hom(phi, psi).basis()  # Basis over Fq{tau^n}
...
sage: Hom(phi, psi).basis(degree=3)  # Basis over Fq for morphsism with tau-degree 3
...

I think this is more or less what the current setup does with the default degree being n-1.

Default set to n - 1 seems more arbitrary than returning a basis over Fq[Ο„n], I believe.

But in that case what you are returning is a basis over $\mathbb{F}_q(\tau^n)$, no?

@xcaruso
Copy link
Contributor

xcaruso commented Apr 5, 2023

But in that case what you are returning is a basis over Fq(Ο„n), no?

I don't think it's equivalent in full generality.

@ymusleh
Copy link
Contributor Author

ymusleh commented Apr 5, 2023

I would say then that it might make sense to have two distinct methods for the $\mathbb{F}_q(\tau^n)$ basis and $\mathbb{F}_q$ bases up to a certain degree.

@xcaruso
Copy link
Contributor

xcaruso commented Apr 5, 2023

I would say then that it might make sense to have two distinct methods for the Fq(Ο„n) basis and Fq bases up to a certain degree.

Why?

@ymusleh
Copy link
Contributor Author

ymusleh commented Apr 5, 2023

I would say then that it might make sense to have two distinct methods for the Fq(Ο„n) basis and Fq bases up to a certain degree.

Why?

For essentially the same reasons I wanted to keep the default parameter: I suspect computing isogenies up to a fixed degree may be a useful computation, and it would be nice to be able to do so without having to compute a potentially very large basis.

@xcaruso
Copy link
Contributor

xcaruso commented Apr 6, 2023

I don't really see your point: in the implementation, the algorithm could be different depending on whether degree is passed or not (or even on its value), no?

@ymusleh
Copy link
Contributor Author

ymusleh commented Apr 6, 2023

I don't really see your point: in the implementation, the algorithm could be different depending on whether degree is passed or not (or even on its value), no?

My issue is that we are returning what are fundamentally two different objects. One is a basis for a vector space over $\mathbb{F}_q$ of isogenies up to a fixed degree, and the other is a basis for the module of all isogenies over the ring $\mathbb{F}_q[\tau^n]$. Perhaps we are fine with this, but my thinking is that it is preferable to not conflate these by having them returned by the same method.

Also, the algorithm to use here is not so clear to me yet. I have admittedly not had much time to think about it so perhaps there is a fairly simple way to compute it.

@xcaruso
Copy link
Contributor

xcaruso commented Apr 6, 2023

My issue is that we are returning what are fundamentally two different objects. One is a basis for a vector space over Fq of isogenies up to a fixed degree, and the other is a basis for the module of all isogenies over the ring Fq[Ο„n]. Perhaps we are fine with this, but my thinking is that it is preferable to not conflate these by having them returned by the same method.

OK, I see. I think it's not a problem but if you dislike this, I'm also fine with having two separated methods. Which names would you suggest?

Also, the algorithm to use here is not so clear to me yet. I have admittedly not had much time to think about it so perhaps there is a fairly simple way to compute it.

I think that the algorithm is basically the same: you write the Ore polynomial defining the isogeny as a linear combinaison of the form $\sum_{0 \leq i,j < r} a_{i,j}(T^n) e_i T^j$ where the $a_{i,j}$'s are unknown polynomials over $\mathbb F_q$ and the $e_i$'s form a basis of $K$ over $\mathbb F_q$. This results in a linear system (over $\mathbb F_q[\tau^n]$) on the $a_{i,j}$'s.

@kryzar
Copy link
Contributor

kryzar commented Apr 12, 2023

I was thinking about implementing a method is_trivial that returns True whenever the only morphism is $0$. What do you think?

@xcaruso
Copy link
Contributor

xcaruso commented Apr 12, 2023

Why not is_zero?

@kryzar
Copy link
Contributor

kryzar commented Apr 12, 2023

Why not is_zero?

Why not!

@xcaruso
Copy link
Contributor

xcaruso commented Apr 17, 2023

While working on #35527, I realized that the homset between Drinfeld modules is also a module over \mathbb{F}_q[T].
So, it's probably better to keep your implementation as is, and discuss possible improvements in a separate issue/PR.

@ymusleh
Copy link
Contributor Author

ymusleh commented Apr 17, 2023

While working on #35527, I realized that the homset between Drinfeld modules is also a module over \mathbb{F}_q[T]. So, it's probably better to keep your implementation as is, and discuss possible improvements in a separate issue/PR.

Indeed, $\textnormal{Hom}(\phi, \psi)$ is a left $\textnormal{End}(\psi)$-module and a right $\textnormal{End}(\phi)$-module in the natural way, and I think the details of how to approach the computation in this case are actually a bit more technical and probably go beyond what should be discussed on a software development platform. I agree that unless anyone feels this computation is deprecated by this alternate view, this PR should focus exclusively on the $\mathbb{F}_q$-basis. This is also the name I suggest for the method i.e. Fq_basis

@ymusleh ymusleh reopened this Nov 18, 2023
@ymusleh
Copy link
Contributor Author

ymusleh commented Nov 18, 2023

The implementation now provides the method Fq_basis for computing the basis of fixed degree morphisms over $\mathbb{F}_q$ and basis for morphisms over $\mathbb{F}_q[\tau^n]$. I'm not sure if you wanted to do $\mathbb{F}_q[T]$ or other structures in this PR or another so I've left them for now.

Copy link

Documentation preview for this PR (built with commit b48d29d; changes) is ready! πŸŽ‰

@kryzar kryzar assigned kryzar and unassigned kryzar Apr 18, 2024
@kryzar kryzar self-requested a review April 18, 2024 21:39
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 for your patience.

I think there are issues with the testing and error messages. For example, those methods all require the Drinfeld modules to be defined over a finite field, but this is never tested. Some more specific comments are also included.

Comment on lines +6445 to +6447
.. [Wes2022] \B. Wesolowski. 2022. *Computing isogenies between finite Drinfeld modules*.
Cryptology ePrint Archive, Paper 2022/438. https://eprint.iacr.org/2022/438

Copy link
Contributor

Choose a reason for hiding this comment

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

At some point you'll probably want to add your own thesis. (Don't forget to ping us when it's available btw, and congrats on defending it!)

@@ -421,3 +425,264 @@ def _element_constructor_(self, *args, **kwds):
# would call __init__ instead of __classcall_private__. This
# seems to work, but I don't know what I'm doing.
return DrinfeldModuleMorphism(self, *args, **kwds)

def element(self, degree):
Copy link
Contributor

Choose a reason for hiding this comment

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

Generally, I believe the preferred name for this method is an_element, which is implemented in Parent and takes no argument:

sage: Parent.an_element?
Docstring:     
   Returns a (preferably typical) element of this parent.

   This is used both for illustration and testing purposes. If the set
   "self" is empty, "an_element()" raises the exception
   "EmptySetError".

   This calls "_an_element_()" (which see), and caches the result.
   Parent are thus encouraged to override "_an_element_()".

   EXAMPLES:

      sage: CDF.an_element()
      1.0*I
      sage: ZZ[['t']].an_element()
      t

   In case the set is empty, an "EmptySetError" is raised:

      sage: Set([]).an_element()
      Traceback (most recent call last):
      ...
      EmptySetError
Init docstring: Initialize self.  See help(type(self)) for accurate signature.
File:           /usr/lib/python3.11/site-packages/sage/structure/parent.pyx
Type:           method_descriptor
sage: 

You can still have the degree parameter, maybe setting it to None by default.

You may also want to implement random_element, as suggested by Xavier a bit below.


OUTPUT: a univariate ore polynomials with coefficients in `K`

EXAMPLES::
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you ought to add some tests, in order to have more meaningful results or error messages:

Example 1

Two Drinfeld modules with different ranks.

sage: Fq = GF(4)
sage: A.<T> = Fq[]
sage: K.<z> = Fq.extension(2)
sage: phi = DrinfeldModule(A, [z, 1])
sage: psi = DrinfeldModule(A, [z, 0, 1])
sage: hom = Hom(phi, psi)
sage: hom.element(3)
---------------------------------------------------------------------------
IndexError                                Traceback (most recent call last)
Cell In[30], line 1
----> 1 hom.element(Integer(3))

File ~/sage/src/sage/rings/function_field/drinfeld_modules/homset.py:460, in DrinfeldModuleHomset.element(self, degree)
    430 r"""
    431 Return an element of the space of morphisms between the domain and
    432 codomain. By default, chooses an element of largest degree less than
   (...)
    457     and return it.
    458 """
    459 basis = self.Fq_basis(degree)
--> 460 elem = basis[0]
    461 max_deg = elem.ore_polynomial().degree()
    462 for basis_elem in basis:

IndexError: list index out of range
sage: 

Example 2

Similar kind of situation, but different error message.

sage: Fq = GF(2)
sage: A.<T> = Fq[]
sage: K.<z> = Fq.extension(2)
sage: phi = DrinfeldModule(A, [z, 1])
sage: psi = DrinfeldModule(A, [z, 1, 1])
sage: hom = Hom(phi, psi)
sage: hom.element(5)
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
Cell In[46], line 1
----> 1 hom.element(Integer(5))

File ~/sage/src/sage/rings/function_field/drinfeld_modules/homset.py:459, in DrinfeldModuleHomset.element(self, degree)
    429 def element(self, degree):
    430     r"""
    431     Return an element of the space of morphisms between the domain and
    432     codomain. By default, chooses an element of largest degree less than
   (...)
    457         and return it.
    458     """
--> 459     basis = self.Fq_basis(degree)
    460     elem = basis[0]
    461     max_deg = elem.ore_polynomial().degree()

File ~/sage/src/sage/rings/function_field/drinfeld_modules/homset.py:536, in DrinfeldModuleHomset.Fq_basis(self, degree)
    534 tau = domain.ore_polring().gen()
    535 for basis_elem in sol:
--> 536     basis.append(self(sum([sum([K_basis[j]*basis_elem[i*n + j]
    537                        for j in range(n)])*(tau**i)
    538                        for i in range(d + 1)])))
    539 return basis

File ~/sage/src/sage/structure/parent.pyx:901, in sage.structure.parent.Parent.__call__()
    899 if mor is not None:
    900     if no_extra_args:
--> 901         return mor._call_(x)
    902     else:
    903         return mor._call_with_args(x, args, kwds)

File ~/sage/src/sage/structure/coerce_maps.pyx:163, in sage.structure.coerce_maps.DefaultConvertMap_unique._call_()
    161             print(type(C), C)
    162             print(type(C._element_constructor), C._element_constructor)
--> 163         raise
    164 
    165 cpdef Element _call_with_args(self, x, args=(), kwds={}) noexcept:

File ~/sage/src/sage/structure/coerce_maps.pyx:158, in sage.structure.coerce_maps.DefaultConvertMap_unique._call_()
    156 cdef Parent C = self._codomain
    157 try:
--> 158     return C._element_constructor(x)
    159 except Exception:
    160     if print_warnings:

File ~/sage/src/sage/rings/function_field/drinfeld_modules/homset.py:427, in DrinfeldModuleHomset._element_constructor_(self, *args, **kwds)
    383 r"""
    384 Return the Drinfeld module morphism defined by the given Ore
    385 polynomial.
   (...)
    422       Defn: t + 1
    423 """
    424 # NOTE: This used to be self.element_class(self, ...), but this
    425 # would call __init__ instead of __classcall_private__. This
    426 # seems to work, but I don't know what I'm doing.
--> 427 return DrinfeldModuleMorphism(self, *args, **kwds)

File ~/sage/src/sage/misc/classcall_metaclass.pyx:320, in sage.misc.classcall_metaclass.ClasscallMetaclass.__call__()
    318 """
    319 if cls.classcall is not None:
--> 320     return cls.classcall(cls, *args, **kwds)
    321 else:
    322     # Fast version of type.__call__(cls, *args, **kwds)

File ~/sage/src/sage/rings/function_field/drinfeld_modules/morphism.py:187, in DrinfeldModuleMorphism.__classcall_private__(cls, parent, x)
    185     ore_pol = domain.ore_polring()(x)
    186 if ore_pol * domain.gen() != codomain.gen() * ore_pol:
--> 187     raise ValueError('Ore polynomial does not define a morphism')
    188 return cls.__classcall__(cls, parent, ore_pol)

ValueError: Ore polynomial does not define a morphism
sage: 

Example 3

Drinfeld modules that are not isogeneous.

sage: phi = DrinfeldModule(A, [z, z, 1])
sage: psi = DrinfeldModule(A, [z, 0, 1])
sage: phi.frobenius_charpoly()
X^2 + T^2 + T + 1
sage: psi.frobenius_charpoly()
X^2 + X + T^2 + T + 1
sage: hom = Hom(phi, psi)
sage: hom.element(5)
---------------------------------------------------------------------------
IndexError                                Traceback (most recent call last)
Cell In[73], line 1
----> 1 hom.element(Integer(5))

File ~/sage/src/sage/rings/function_field/drinfeld_modules/homset.py:460, in DrinfeldModuleHomset.element(self, degree)
    430 r"""
    431 Return an element of the space of morphisms between the domain and
    432 codomain. By default, chooses an element of largest degree less than
   (...)
    457     and return it.
    458 """
    459 basis = self.Fq_basis(degree)
--> 460 elem = basis[0]
    461 max_deg = elem.ore_polynomial().degree()
    462 for basis_elem in basis:

IndexError: list index out of range
sage: 

If the Drinfeld modules are isogeneous and no isogeny of degree degree exists, no problem: return a lower degree one, as you propose in the docstring, be it zero if necessary. However, I am not sure what the best approach is when the two Drinfeld modules are not isogeneous: raise an exception, or return zero? As far as I'm concerned, as the definition allows it, I would be tempted to return zero.

max_deg = elem.ore_polynomial().degree()
return elem

def Fq_basis(self, degree):
Copy link
Contributor

Choose a reason for hiding this comment

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

As we have discussed in the past, I'm not so sure about the name. I like the idea of having a single method, using an argument basis. However, if you insist on having different methods, then I would at least suggest to have the methods named with the pattern basis_*. That is, all methods that compute a basis can be accessed through autocompletion of "bas...", and are grouped together in the alphabetically-sorted list of methods (which one accesses from the shell via dir(hom).

Comment on lines +470 to +474
Return a basis for the `\mathbb{F}_q`-space of morphisms from `phi` to
a Drinfeld module `\psi` of degree at most `degree`. A morphism
`\iota: \phi \to psi` is an element `\iota \in K\{\tau\}` such that
`iota \phi_T = \psi_T \iota`. The degree of a morphism is the
`\tau`-degree of `\iota`.
Copy link
Contributor

Choose a reason for hiding this comment

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

In the doc of DrinfeldModuleMorphism, f is used rather than \iota. You may also add a reference to that.

# K_basis.
c_tik = (dom_coeffs[i].frobenius(qorder*k)*K_basis[j] \
- cod_coeffs[i]*K_basis[j].frobenius(qorder*i)) \
.polynomial().coefficients(sparse=False)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm a bit surprised that this works, or that it does what you think it does. My understanding is the following:

  • You aim at working over Fq (as a basis of K), and because K has degree n over Fq, c_tik should at the end have n elements.
  • It looks like the method polynomial returns the polynomial whose coefficients are that on the input on the prime field, i.e. $\mathbb F_p$, in which case, its degree may be strictly larger than n.

In the below example, the polynomial has degree $3$ while $n$ equals $2$:

sage: Fq = GF(3^2)
sage: A.<T> = Fq[]
sage: K.<z> = Fq.extension(2)
sage: x = K.random_element()
sage: x.polynomial?
Docstring:     
   Return self viewed as a polynomial over
   "self.parent().prime_subfield()".

   EXAMPLES:

      sage: k.<b> = GF(5^2); k
      Finite Field in b of size 5^2
      sage: f = (b^2+1).polynomial(); f
      b + 4
      sage: type(f)
      <class 'sage.rings.polynomial.polynomial_zmod_flint.Polynomial_zmod_flint'>
      sage: parent(f)
      Univariate Polynomial Ring in b over Finite Field of size 5
Init docstring: Initialize self.  See help(type(self)) for accurate signature.
File:           ~/sage/src/sage/rings/finite_rings/element_givaro.pyx
Type:           builtin_function_or_method
sage: x
2*z^2 + 2*z + 1
sage: x.polynomial().coefficients(sparse=False)
[1, 2, 2]

And this is more like your code:

sage: from sage.functions.log import logb
sage: q = 4
sage: Fq = GF(q)
sage: A.<T> = Fq[]
sage: _K_prime = Fq.extension(2)
sage: _K.<z> = _K_prime.extension(3)
sage: dom = DrinfeldModule(A, [z, 0, 1])
sage: cod = DrinfeldModule(A, [z, 1, 1])
sage: dom_coeffs = dom.gen().coefficients(sparse=False)
sage: cod_coeffs = cod.gen().coefficients(sparse=False)
sage: K = dom.base_over_constants_field()
sage: n = K.degree(Fq)  # 6
sage: qorder = logb(q, Fq.characteristic())
sage: K_basis = [K.gen()**i for i in range(n)]
sage: 
sage: (i, j, k) = (2, 3, 4)
sage: c_tik = (dom_coeffs[i].frobenius(qorder*k)*K_basis[j] \
sage:          - cod_coeffs[i]*K_basis[j].frobenius(qorder*i)
sage:         ).polynomial().coefficients(sparse=False)
sage: c_tik
[1, 1, 0, 0, 0, 1, 1, 0, 1, 1, 0, 1]

At the end, c_tik has length $12$, while n equals $6$. So while your method seems to output correct results, it may be working over $\mathbb F_p$. This not at all a problem, but I believe the code and its comments should reflect this situation. Also, if I am not mistaken, then the following line (606) is never entered in.

Copy link
Contributor Author

@ymusleh ymusleh Apr 20, 2024

Choose a reason for hiding this comment

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

I'm a bit surprised that this works, or that it does what you think it does. My understanding is the following:

  • You aim at working over Fq (as a basis of K), and because K has degree n over Fq, c_tik should at the end have n elements.
  • It looks like the method polynomial returns the polynomial whose coefficients are that on the input on the prime field, i.e. Fp, in which case, its degree may be strictly larger than n.

In the below example, the polynomial has degree 3 while n equals 2:

sage: Fq = GF(3^2)
sage: A.<T> = Fq[]
sage: K.<z> = Fq.extension(2)
sage: x = K.random_element()
sage: x.polynomial?
Docstring:     
   Return self viewed as a polynomial over
   "self.parent().prime_subfield()".

   EXAMPLES:

      sage: k.<b> = GF(5^2); k
      Finite Field in b of size 5^2
      sage: f = (b^2+1).polynomial(); f
      b + 4
      sage: type(f)
      <class 'sage.rings.polynomial.polynomial_zmod_flint.Polynomial_zmod_flint'>
      sage: parent(f)
      Univariate Polynomial Ring in b over Finite Field of size 5
Init docstring: Initialize self.  See help(type(self)) for accurate signature.
File:           ~/sage/src/sage/rings/finite_rings/element_givaro.pyx
Type:           builtin_function_or_method
sage: x
2*z^2 + 2*z + 1
sage: x.polynomial().coefficients(sparse=False)
[1, 2, 2]

And this is more like your code:

sage: from sage.functions.log import logb
sage: q = 4
sage: Fq = GF(q)
sage: A.<T> = Fq[]
sage: _K_prime = Fq.extension(2)
sage: _K.<z> = _K_prime.extension(3)
sage: dom = DrinfeldModule(A, [z, 0, 1])
sage: cod = DrinfeldModule(A, [z, 1, 1])
sage: dom_coeffs = dom.gen().coefficients(sparse=False)
sage: cod_coeffs = cod.gen().coefficients(sparse=False)
sage: K = dom.base_over_constants_field()
sage: n = K.degree(Fq)  # 6
sage: qorder = logb(q, Fq.characteristic())
sage: K_basis = [K.gen()**i for i in range(n)]
sage: 
sage: (i, j, k) = (2, 3, 4)
sage: c_tik = (dom_coeffs[i].frobenius(qorder*k)*K_basis[j] \
sage:          - cod_coeffs[i]*K_basis[j].frobenius(qorder*i)
sage:         ).polynomial().coefficients(sparse=False)
sage: c_tik
[1, 1, 0, 0, 0, 1, 1, 0, 1, 1, 0, 1]

At the end, c_tik has length 12, while n equals 6. So while your method seems to output correct results, it may be working over Fp. This not at all a problem, but I believe the code and its comments should reflect this situation. Also, if I am not mistaken, then the following line (606) is never entered in.

Perhaps this is my fault for not marking this PR as a draft. At some point I realized that Fq.extension(n) returns a field constructed as a flat extension over $\mathbb{F}_p$ and not as a direct extension of $\mathbb{F}_q$, which made some of the computations I wanted to perform a bit more awkward to implement. As you may have noticed, all the tests are currently over the prime field case only, and this code will indeed fail for non-prime $q$. My plan was to research the finite field implementation a bit more to see what the most elegant way around this would be, ideally to have this work both for the flat extension setting and possibly for fields constructed as quotients of $\mathbb{F}_q[x]$. With my thesis largely done with I'll likely have more time to do this soon.

Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps this is my fault for not marking this PR as a draft.

Don't worry.

With my thesis largely done with I'll likely have more time to do this soon.

Great. Indeed, it seems like basis kinda works, while Fq_basis simply does not (or not in all cases):

sage: Fq = GF(4)
sage: A.<T> = Fq[]
sage: K.<z> = Fq.extension(2)
sage: phi = DrinfeldModule(A, [z, z^2 + z + 1, 0, z^3 + z^2 + z + 1])
sage: psi = DrinfeldModule(A, [z, z^2 + 1, 1, z^3 + z^2 + z + 1])
sage: hom = Hom(phi, psi)
sage: hom.basis()
[Drinfeld Module morphism:
   From: Drinfeld module defined by T |--> (z^3 + z^2 + z + 1)*t^3 + (z^2 + z + 1)*t + z
   To:   Drinfeld module defined by T |--> (z^3 + z^2 + z + 1)*t^3 + t^2 + (z^2 + 1)*t + z
   Defn: t^2 + z*t + 1]
sage: hom.Fq_basis(2)
[]

In any case, any limitation to the prime field should be specified in the docs.

Copy link
Contributor Author

@ymusleh ymusleh Apr 23, 2024

Choose a reason for hiding this comment

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

In any case, any limitation to the prime field should be specified in the docs.

I intend to remove this limitation, and there are several ways to do this (such as, for example, doing linear algebra over $\mathbb{F}_p$ or using an explicit field embedding), though this raised the question of whether the implementation of Drinfeld modules should be able to handle fields constructed as explicit towers. For example, consider the following:

sage: Fq = GF(4)
sage: A.<T> = Fq[]
sage: ip = A.irreducible_element(2)
sage: K.<z> = Fq.extension(ip)

Currently, K as constructed above can't be used to create a Drinfeld module using the implementation, and I had planned to ask what opinions were on adding support for this.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, thanks for pointing this out. I was not aware of this limitation; I think there is no reason to prevent the user from using this construction, and would definitely support the addition of this feature...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, thanks for pointing this out. I was not aware of this limitation; I think there is no reason to prevent the user from using this construction, and would definitely support the addition of this feature...

Ok I'll have a look into it then.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you. I'm happy to help (I won't have much time before summer tho, I'm focused on writing my thesis, and I will be away in July).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's fine, I don't think it's necessary going to be too difficult. The easiest approach would probably be to extend or add certain methods to work for quotient rings (such as in_base and frobenius).

`iota`.
"""
domain, codomain = self.domain(), self.codomain()
Fq = domain._Fq
Copy link
Contributor

Choose a reason for hiding this comment

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

You should not rely on the internal (and ultimately private) representation of objects.


OUTPUT: a list of Drinfeld module morphisms.

EXAMPLES::
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as before: very few tests.

Comment on lines +629 to +630
we provide a helper method here. This should probably be part of the
Finite field implementation.
Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed. Eventually, it will probably also will have to be implemented for Xavier's motives.

Comment on lines +524 to +527
oper = K(dom_coeffs[k-i]
.frobenius(qorder*i)).matrix().transpose() \
- K(cod_coeffs[k-i]).matrix().transpose() \
* self._frobenius_matrix(k - i)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
oper = K(dom_coeffs[k-i]
.frobenius(qorder*i)).matrix().transpose() \
- K(cod_coeffs[k-i]).matrix().transpose() \
* self._frobenius_matrix(k - i)
oper = K(dom_coeffs[k-i].frobenius(qorder*i)
).matrix().transpose() \
- K(cod_coeffs[k-i]).matrix().transpose() \
* self._frobenius_matrix(k - i)

@ymusleh ymusleh marked this pull request as draft April 20, 2024 14:48
Comment on lines +431 to +433
Return an element of the space of morphisms between the domain and
codomain. By default, chooses an element of largest degree less than
or equal to the parameter `degree`.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Return an element of the space of morphisms between the domain and
codomain. By default, chooses an element of largest degree less than
or equal to the parameter `degree`.
Return an element of the space of morphisms between the domain and codomain.
By default, chooses an element of largest degree less than
or equal to the parameter `degree`.

This should follow docstring conventions in the Sage developers guide


def Fq_basis(self, degree):
r"""
Return a basis for the `\mathbb{F}_q`-space of morphisms from `phi` to
Copy link
Member

Choose a reason for hiding this comment

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

Same comment as above. This description should be a one-sentence description followed by a blank line.

n = K.degree(Fq)
# shorten name for readability
d = degree
qorder = logb(q, char)
Copy link
Member

Choose a reason for hiding this comment

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

Here, you don't need to import the function logb, you can simply do:

Suggested change
qorder = logb(q, char)
qorder = char.log(q)

(this also applies to the method basis below)

Copy link
Contributor

Choose a reason for hiding this comment

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

TIL

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

5 participants