Implement characteristic polynomial computation for Drinfeld modules of any Rank #35269
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## develop #35269 +/- ##
===========================================
- Coverage 88.62% 88.61% -0.02%
===========================================
Files 2148 2148
Lines 398855 398915 +60
===========================================
+ Hits 353480 353493 +13
- Misses 45375 45422 +47
... and 29 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. |
|
Thanks Joseph! I will read the code and docs this week, and I'd be happy to be a reviewer! |
src/sage/rings/function_field/drinfeld_modules/finite_drinfeld_module.py
Outdated
Show resolved
Hide resolved
|
Thank you very much Joseph, that's a wonderful contribution. I made a couple of minor docstring typesetting suggestions that follows the Sage development guidelines, but overall from a first glance the code looks very good! |
src/sage/rings/function_field/drinfeld_modules/finite_drinfeld_module.py
Outdated
Show resolved
Hide resolved
src/sage/rings/function_field/drinfeld_modules/finite_drinfeld_module.py
Outdated
Show resolved
Hide resolved
| algorithms = {'gekeler', 'crystalline'} | ||
| if algorithm in algorithms: | ||
| return getattr(self, \ | ||
| f'{self.frobenius_charpoly.__name__}_{algorithm}')(var) |
There was a problem hiding this comment.
| f'{self.frobenius_charpoly.__name__}_{algorithm}')(var) | |
| f'{self._frobenius_charpoly.__name__}_{algorithm}')(var) |
There was a problem hiding this comment.
Ugh... I have trouble adding the necessary spaces, be careful here!
| moduli = [S([c**(q**(-i*nstar % n)) for c in mu_coeffs]) \ | ||
| for i in range(1, n1) ] | ||
|
|
||
| def reduce_and_frobenius(order, modulus): |
There was a problem hiding this comment.
This nested function is only called once; do we really need to have it?
| ALGORITHM: | ||
| def _frobenius_charpoly_crystalline(self, var='X'): | ||
| r""" | ||
| Method to compute the characteristic polynomial of the |
There was a problem hiding this comment.
I think that the first line of the doctest has to be of the form Return [...]
| sage: K.<z> = Fq.extension(8) | ||
| sage: phi = DrinfeldModule(A, [z, 4, 1, z, z+1, 2, z+2, 1, 1, 3, 1]) | ||
| sage: phi._frobenius_charpoly_crystalline() | ||
| X^10 + X^9 + (3*T + z2 + 1)*X^8 + (4*T^2 + z2*T + 2*z2 + 1)*X^7 + (4*T^3 + (z2 + 2)*T^2 + (4*z2 + 2)*T + 4)*X^6 + (3*T^4 + T^3 + 3*z2*T + 3*z2 + 3)*X^5 + ((4*z2 + 2)*T^4 + (3*z2 + 4)*T^3 + 3*T^2 + (2*z2 + 4)*T + 4*z2 + 1)*X^4 + (3*T^5 + (2*z2 + 3)*T^4 + 4*T^3 + (2*z2 + 2)*T^2 + (z2 + 3)*T + 4*z2)*X^3 + (3*T^6 + (3*z2 + 2)*T^5 + (4*z2 + 1)*T^4 + z2*T^3 + (4*z2 + 4)*T^2 + 4*z2*T)*X^2 + (2*T^7 + 3*T^6 + 2*z2*T^5 + (2*z2 + 3)*T^4 + (4*z2 + 3)*T^3 + (z2 + 2)*T^2 + (z2 + 4)*T + 2*z2 + 2)*X + T^8 + (4*z2 + 3)*T^6 + (4*z2 + 4)*T^4 + 4*z2*T^2 + (z2 + 2)*T + z2 |
There was a problem hiding this comment.
IMO, the complete output is not really meaningful; maybe, you should cut it using Ellipsis (...);
| sage: K.<z> = Fq.extension(10) | ||
| sage: phi = DrinfeldModule(A, [z, z^2 + z, 2, 1, z, z+1, 2, z+2, 0, 1, 1, z^2 + z]) | ||
| sage: phi._frobenius_charpoly_crystalline() | ||
| X^11 + (z3^2 + 2*z3)*X^10 + ((z3 + 1)*T + z3)*X^9 + ((2*z3^2 + z3 + 2)*T^2 + (2*z3^2 + z3 + 1)*T + 2*z3^2 + z3 + 2)*X^8 + ((z3^2 + z3)*T^3 + 2*z3^2*T^2 + (2*z3^2 + z3 + 2)*T + 1)*X^7 + ((z3^2 + 2*z3)*T^4 + (2*z3^2 + 2*z3)*T^3 + (z3^2 + 1)*T^2 + (z3^2 + 1)*T + z3^2 + z3 + 2)*X^6 + ((z3^2 + z3)*T^5 + (z3^2 + 1)*T^4 + (z3^2 + 1)*T^3 + (z3^2 + 2*z3 + 1)*T^2 + (z3^2 + 2)*T + z3^2 + 2*z3 + 1)*X^5 + ((z3^2 + z3 + 1)*T^6 + (z3^2 + 2*z3 + 1)*T^5 + (2*z3^2 + z3)*T^4 + (z3^2 + 2)*T^2 + (2*z3^2 + 2*z3)*T + z3^2 + 1)*X^4 + (2*z3*T^7 + 2*z3^2*T^6 + (z3^2 + z3)*T^5 + (2*z3 + 2)*T^4 + 2*z3*T^3 + (z3 + 2)*T^2 + z3^2*T + z3^2 + 2*z3 + 1)*X^3 + (2*z3*T^8 + (z3^2 + z3)*T^7 + (2*z3^2 + z3 + 2)*T^6 + (z3^2 + 2)*T^5 + T^4 + (2*z3^2 + 2)*T^3 + (z3^2 + z3 + 1)*T^2 + (z3 + 2)*T + 2*z3^2 + z3 + 1)*X^2 + (2*z3*T^9 + z3*T^8 + (z3^2 + z3 + 1)*T^7 + T^6 + (z3^2 + 2*z3 + 2)*T^5 + (z3^2 + 2*z3 + 1)*T^4 + T^3 + (2*z3^2 + z3 + 2)*T^2 + T + 2*z3^2 + z3)*X + z3*T^10 + (z3^2 + z3)*T^9 + (2*z3^2 + 1)*T^8 + (2*z3^2 + 2*z3)*T^7 + z3*T^6 + (z3^2 + 2*z3 + 2)*T^5 + T^4 + z3*T^3 + (z3^2 + 1)*T^2 + (2*z3^2 + 2*z3 + 2)*T + z3^2 |
| WARNING: This algorithm only works in the "generic" case | ||
| when the corresponding linear system is invertible. | ||
| Notable cases where this fails include Drinfeld | ||
| modules whose minimal polynomial is not equal to | ||
| the characteristic polynomial, and rank 2 Drinfeld | ||
| modules where the degree 1 coefficient of \phi_T is | ||
| 0. |
There was a problem hiding this comment.
Formatting is not correct. It should be:
.. WARNING:
This algorithm only works in the "generic" case
when the corresponding linear system is invertible.
Notable cases where this fails include Drinfeld
modules whose minimal polynomial is not equal to
the characteristic polynomial, and rank 2 Drinfeld
modules where the degree 1 coefficient of `\phi_T` is 0.
| sage: K.<z> = Fq.extension(6) | ||
| sage: phi = DrinfeldModule(A, [z, 4, 1, z]) | ||
| sage: phi._frobenius_charpoly_gekeler() | ||
| X^3 + ((z2 + 2)*T^2 + (z2 + 2)*T + 4*z2 + 4)*X^2 + (4*z2*T^3 + (2*z2 + 3)*T^2 + (2*z2 + 2)*T + z2 + 3)*X + (3*z2 + 2)*T^6 + (4*z2 + 2)*T^5 + (3*z2 + 2)*T^4 + (3*z2 + 4)*T^3 + (3*z2 + 2)*T^2 + (3*z2 + 3)*T + 4 |
There was a problem hiding this comment.
Again, maybe truncate output.
| sage: phi = DrinfeldModule(A, [z, 0, z]) | ||
| sage: phi._frobenius_charpoly_gekeler() | ||
| Traceback (most recent call last): | ||
| ValueError: Can't solve system for characteristic polynomial |
There was a problem hiding this comment.
Maybe NotImplementedError instead of ValueError
|
I would like to add the algorithm |
A separate PR would be better, no? |
Either approach is fine with me, but it seems separate PRs are preferred so I think creating one and just referencing this as a dependency is probably best. |
|
OK, I will open a new PR soon. |
There was a problem hiding this comment.
I did my review! Thank you Joseph for this contribution, which seems quite technical. The core is very good. Yet, I think the code could definitely be cleaner.
Also, a few general remarks:
- Generally, variable names — especially those that are only one character — can be a little cryptic.
- I would remove the default variable name on the
_frobenius_charpoly_...methods. - I would suggest adding comments that outline / split the rough different steps in your algorithms. You may use blank lines, sporadically and wisely, if necessary.
- Be careful that your paragraphs are correctly flowed (I use
gqiporgwipon vim to achieve the result).
For the Gekeler algorithm, should we check from the start the the constant coefficient generates the base field? If not, we should highlight in the doctest that the computation may fail, and raise an exception. @DavidAyotte, @xcaruso, what do you think?
Can you update methods frobenius_trace and frobenius_norm?
Also, you can use a linter to check that your code respects code-writing conventions. For Python, the convention is PEP8, and flake8 is the most used linter. In the terminal, you can run flake8 finite_drinfeld_module.py. Of course, there are some errors that you have to ignore (typically, sage: lines often exceed the character limit).
| from sage.modules.free_module_element import vector | ||
| from sage.rings.polynomial.polynomial_ring_constructor import PolynomialRing | ||
| from sage.rings.function_field.drinfeld_modules.drinfeld_module import DrinfeldModule | ||
|
|
||
| from sage.functions.other import ceil, sqrt | ||
| from sage.all import prod | ||
|
|
There was a problem hiding this comment.
Can you add a blank line here? I forgot to do this in the first place (PEP8)...
| is defined in [Gek1991]_. An important feature of this | ||
| polynomial is that it is a monic univariate polynomial with | ||
| coefficients in the function ring. As in our case the function |
There was a problem hiding this comment.
What about:
An important feature of this polynomial is that it is monic, univariate, and his coefficients live in the function ring.
src/sage/rings/function_field/drinfeld_modules/finite_drinfeld_module.py
Outdated
Show resolved
Hide resolved
src/sage/rings/function_field/drinfeld_modules/finite_drinfeld_module.py
Outdated
Show resolved
Hide resolved
|
|
||
| ALGORITHM: | ||
| """ | ||
| algorithms = {'gekeler', 'crystalline'} |
There was a problem hiding this comment.
Isn't there a way to automatically create this set from the list of methods?
src/sage/rings/function_field/drinfeld_modules/finite_drinfeld_module.py
Outdated
Show resolved
Hide resolved
src/sage/rings/function_field/drinfeld_modules/finite_drinfeld_module.py
Outdated
Show resolved
Hide resolved
src/sage/rings/function_field/drinfeld_modules/finite_drinfeld_module.py
Outdated
Show resolved
Hide resolved
|
|
||
| """ | ||
| if not isinstance(psi, DrinfeldModule): | ||
| raise TypeError("Input must be a Drinfeld module") |
| raise TypeError("Input must be a Drinfeld module") | ||
| if self.category() != psi.category(): | ||
| raise TypeError("Drinfeld modules are not in the same category") | ||
| return self.rank() == psi.rank() and\ |
|
Thanks for the very detailed and thorough review; I will try to address the requested changes shortly. To reply to a couple of your points.
I'm thinking for q and n in particular there isn't a great alternative and their usage should hopefully be clear from the documentation.
The Gekeler algorithm can return the correct result in the non-prime field case, so I think it should be fine as is. |
No rush!
For those, I agree. I was thinking about
I'm okay with this! |
I'm mostly done with the updates, and I think this is the last major point to address. I'm unsure what the best approach is here, as I find it a bit excessive to have methods merely to access a single coefficient of the characteristic polynomial. However, for the Frobenius norm at least, there is some justification for such a method as it has a simple formula for the prime field case. I think there are 3 possibilities, and am happy for input on which makes the most sense:
|
|
IMO, it's better to keep these methods, even if they look redundant. |
I would keep them both. Removing them would require a formal deprecation. Obviously the third option is the best. I haven't looked at algorithms that compute the trace without computing the full characteristic polynomial. And I will not have time in the short term. For the time being, we can compute the full polynomial and return the coefficient, while insisting in the documentation that this is inefficient. EDIT. Let In that case, we could use it for the method self._frobenius_norm = None
self._frobenius_trace = Nonefrom the constructor. If we implement |
For the motivic algorithm (and I would say that it's the same for the crystalline algorithm), I think it suffices to return the trace/determinant of the intermediate matrix we compute, instead of its characteristic polynomial. |
| methods = [method for method in dir(self) | ||
| if method.startswith('_frobenius_charpoly_')] | ||
| method_name = f'_frobenius_charpoly_{algorithm}' |
| sage: phi = DrinfeldModule(A, [p_root, z12^3, z12^5]) | ||
| sage: phi.frobenius_charpoly() | ||
| X^2 + ((4*z2 + 4)*T^3 + (z2 + 3)*T^2 + 3*T + 2*z2 + 3)*X + 3*z2*T^6 + (4*z2 + 3)*T^5 + (4*z2 + 4)*T^4 + 2*T^3 + (3*z2 + 3)*T^2 + (z2 + 2)*T + 4*z2 | ||
|
|
There was a problem hiding this comment.
Missing :::
| :: | |
| if self._frobenius_charpoly is not None: | ||
| return self._frobenius_charpoly | ||
| self._frobenius_charpoly = getattr(self, method_name)(var) | ||
| return self._frobenius_charpoly | ||
| raise NotImplementedError(f'Algorithm \"{algorithm}\" not implemented') | ||
|
|
||
| def _frobenius_charpoly_crystalline(self, var='X'): |
There was a problem hiding this comment.
My only comment for this method is the following: I think you did not address my previous comment on the var='X' for _frobenius_charpoly_... methods. Shouldn't we remove it here? It seems redundant.
The rest of the method is perfect, thank you for the changes!
| any rank. | ||
|
|
||
| This method is private and should not be directly called. | ||
| Instead, use :meth:`frobenius_charpoly`. |
There was a problem hiding this comment.
| Instead, use :meth:`frobenius_charpoly`. | |
| Instead, use :meth:`frobenius_charpoly` with the option | |
| `algorithm='crystalline'`. |
| companion_initial = prod([companion(i) | ||
| for i in range(nrem, 0, -1)]) | ||
| companion_step = prod([companion(i) | ||
| for i in range(nstar + nrem, nrem, -1)]) |
There was a problem hiding this comment.
| companion_initial = prod([companion(i) | |
| for i in range(nrem, 0, -1)]) | |
| companion_step = prod([companion(i) | |
| for i in range(nstar + nrem, nrem, -1)]) | |
| companion_initial = prod([companion(i) for i in range(nrem, 0, -1)]) | |
| companion_step = prod([companion(i) | |
| for i in range(nstar + nrem, nrem, -1)]) |
| of the base field. | ||
|
|
||
| This method is private and should not be directly called. | ||
| Instead, use :meth:`frobenius_charpoly`. | ||
|
|
||
| .. WARNING: |
There was a problem hiding this comment.
I don't know why I can't select the whole block, but here is a suggestion:
This algorithm only works in the generic case when the
corresponding linear system is invertible. Notable cases
where this fails include Drinfeld modules whose minimal
polynomial is not equal to the characteristic polynomial,
and rank 2 Drinfeld modules where the degree 1 coefficient
of `\phi_T` is 0. In that case, an exception is raised.
| @@ -369,18 +413,19 @@ def _frobenius_charpoly_gekeler(self, var='X'): | |||
| sage: phi = DrinfeldModule(A, [z, 0, z]) | |||
There was a problem hiding this comment.
Can you replace Line 109 with
::
There was a problem hiding this comment.
The Rubric line? Not sure if this is what you want so I'll leave this for now.
There was a problem hiding this comment.
I apologize, this was not clear at all by me. Apparently, what I meant was Line 418. I tagued you in the relevant comment.
|
|
||
| Let `n` be the degree of the base field over `\mathbb{F}_q` Then the | ||
| Frobenius norm has degree `n`. | ||
| Let `C(X) = \sum_{i=0}^r a_iX^{r-i}` denote the characteristic |
There was a problem hiding this comment.
Whare are you using r-i instead of i?
Also, I would use is defined as instead of simply is.
Same remark for frobenius_trace.
There was a problem hiding this comment.
Notationally it can actually be a bit cleaner to do it this way, but to ensure consistency with documentation elsewhere I'll revert it.
There was a problem hiding this comment.
Ok. I don't have a strong opinion on this. At first glance it looks like i is a bit easier for the user that won't go into details. That being said, I believe you. @xcaruso, @DavidAyotte: any recommendation?
EDIT. That being said, it makes more sense to me that a norm would be the degree-$0$ coefficient.
There was a problem hiding this comment.
The main gain is that the degree constraint looks a little nicer, and the sum ranges from 1 to r instead of 0 to r - 1, and reflects the "order" of the term, with the norm being an order r object and the trace being an order 1 object. Not really a huge issue so I'll leave it with the standard convention.
| return self.rank() == psi.rank() and \ | ||
| self.frobenius_charpoly() == psi.frobenius_charpoly() |
There was a problem hiding this comment.
After all it looks better with the and at the begining of the line:
| return self.rank() == psi.rank() and \ | |
| self.frobenius_charpoly() == psi.frobenius_charpoly() | |
| return self.rank() == psi.rank() \ | |
| and self.frobenius_charpoly() == psi.frobenius_charpoly() |
| @@ -706,7 +720,6 @@ def is_ordinary(self): | |||
| `\mathbb{F}_q[T]`-characteristic under the Drinfeld module | |||
| is purely inseparable; see [Gek1991]_, Proposition 4.1. | |||
| """ | |||
| self._check_rank_two() | |||
There was a problem hiding this comment.
Thank you, I had forgotten about this!
|
This request should be ready for review now. |
| Traceback (most recent call last): | ||
| TypeError: Drinfeld modules are not in the same category |
| Traceback (most recent call last): | ||
| TypeError: input must be a Drinfeld module |
| K = self.base_over_constants_field() | ||
| n = K.degree(self._Fq) | ||
| char = self.characteristic() | ||
| norm = K(self.coefficients()[-1]).norm() |
There was a problem hiding this comment.
So, should we implement it instead?
|
Hi Joseph, most of this PR looks good for me. If you want, you can add your name to the author list at the top of the file, and mention what you implemented. |
|
Thanks. |
Sorry, I meant to reply to the previous comment directly but forgot. So I believe it is computed via the formula in |
|
Oh sorry, you're right, I misread the code. |
| if algorithm in algorithms: | ||
| return getattr(self, \ | ||
| f'{self.frobenius_charpoly.__name__}_{algorithm}')(var) | ||
| raise NotImplementedError(f'Algorithm \"{algorithm}\" not implemented') |
There was a problem hiding this comment.
| raise NotImplementedError(f'Algorithm \"{algorithm}\" not implemented') | |
| raise NotImplementedError(f'algorithm \"{algorithm}\" not implemented') |
| Return the characteristic polynomial of the Frobenius | ||
| endomorphism for any rank if the minimal polynomial is | ||
| equal to the characteristic polynomial. Currently only | ||
| works for Drinfeld modules defined over Fq[T]. |
There was a problem hiding this comment.
The description is usually formatted in this way: a one sentence description of the form "Return..." then, on a second paragraph, a longer description.
| Return the characteristic polynomial of the Frobenius | |
| endomorphism for any rank if the minimal polynomial is | |
| equal to the characteristic polynomial. Currently only | |
| works for Drinfeld modules defined over Fq[T]. | |
| Return the characteristic polynomial of the Frobenius | |
| endomorphism for any rank if the minimal polynomial is | |
| equal to the characteristic polynomial. | |
| Currently only works for Drinfeld `\mathbb{F}_q[T]`-modules. |
| WARNING: This algorithm only works in the "generic" case | ||
| when the corresponding linear system is invertible. | ||
| Notable cases where this fails include Drinfeld | ||
| modules whose minimal polynomial is not equal to | ||
| the characteristic polynomial, and rank 2 Drinfeld | ||
| modules where the degree 1 coefficient of \phi_T is | ||
| 0. |
There was a problem hiding this comment.
| WARNING: This algorithm only works in the "generic" case | |
| when the corresponding linear system is invertible. | |
| Notable cases where this fails include Drinfeld | |
| modules whose minimal polynomial is not equal to | |
| the characteristic polynomial, and rank 2 Drinfeld | |
| modules where the degree 1 coefficient of \phi_T is | |
| 0. | |
| .. WARNING: | |
| This algorithm only works in the "generic" case | |
| when the corresponding linear system is invertible. | |
| Notable cases where this fails include Drinfeld | |
| modules whose minimal polynomial is not equal to | |
| the characteristic polynomial, and rank 2 Drinfeld | |
| modules where the degree 1 coefficient of \phi_T is 0. |
|
|
||
| INPUT: | ||
|
|
||
| - ``var`` (default: ``'X'``) -- the name of the second variable |
There was a problem hiding this comment.
| - ``var`` (default: ``'X'``) -- the name of the second variable | |
| - ``var`` (string, default: ``'X'``) -- the name of the second variable |
Also, I would change var to name.
| Notable cases where this fails include Drinfeld | ||
| modules whose minimal polynomial is not equal to | ||
| the characteristic polynomial, and rank 2 Drinfeld | ||
| modules where the degree 1 coefficient of \phi_T is |
There was a problem hiding this comment.
| modules where the degree 1 coefficient of \phi_T is | |
| modules where the degree 1 coefficient of `\phi_T` is |
| if sys.right_nullity() == 0: | ||
| sol = list(sys.solve_right(rhs)) | ||
| else: | ||
| raise ValueError("Can't solve system for characteristic polynomial") |
There was a problem hiding this comment.
| raise ValueError("Can't solve system for characteristic polynomial") | |
| raise ValueError("can't solve system for characteristic polynomial") |
| Method to compute the characteristic polynomial of the | ||
| Frobenius endomorphism using Crystalline cohomology. | ||
| Currently only works for Drinfeld modules defined over | ||
| Fq[T], but otherwise does not impose any other constraints, | ||
| including on the rank, minimal polynomial, or that the Drinfeld | ||
| module is defined over the prime field. |
There was a problem hiding this comment.
| Method to compute the characteristic polynomial of the | |
| Frobenius endomorphism using Crystalline cohomology. | |
| Currently only works for Drinfeld modules defined over | |
| Fq[T], but otherwise does not impose any other constraints, | |
| including on the rank, minimal polynomial, or that the Drinfeld | |
| module is defined over the prime field. | |
| Return the characteristic polynomial of the | |
| Frobenius endomorphism using Crystalline cohomology. | |
| Currently only works for Drinfeld `\mathbb{F}_q[T]`-modules, | |
| but otherwise does not impose any other constraints, | |
| including on the rank, minimal polynomial, or that the Drinfeld | |
| module is defined over the prime field. |
| ALGORITHM: | ||
|
|
||
| Compute the characteristic polynomial of the endomorphism | ||
| acting on the crystalline cohomology of a Drinfeld module. | ||
| A recurrence on elements of the cohomology allows us to | ||
| compute a matrix representation of the Frobenius endomorphism | ||
| efficiently using a companion matrix method. |
There was a problem hiding this comment.
| ALGORITHM: | |
| Compute the characteristic polynomial of the endomorphism | |
| acting on the crystalline cohomology of a Drinfeld module. | |
| A recurrence on elements of the cohomology allows us to | |
| compute a matrix representation of the Frobenius endomorphism | |
| efficiently using a companion matrix method. | |
| ALGORITHM: | |
| Compute the characteristic polynomial of the endomorphism | |
| acting on the crystalline cohomology of a Drinfeld module. | |
| A recurrence on elements of the cohomology allows us to | |
| compute a matrix representation of the Frobenius endomorphism | |
| efficiently using a companion matrix method. |
| raise NotImplementedError(\ | ||
| "Can't solve system for characteristic polynomial") |
There was a problem hiding this comment.
I prefer the first error message which is more explicative. I would either keep what Joseph wrote or merge the two:
raise NotImplementedError(\
"Algorithm failed: cannot solve system for characteristic polynomial")
| @@ -470,6 +632,61 @@ def invert(self, ore_pol): | |||
| raise ValueError('input must be in the image of the Drinfeld ' | |||
| 'module') | |||
|
|
|||
| def is_isogenous(self, psi): | |||
| r""" | |||
| Return ``True`` whethere ``self`` is isogenous to the other | |||
There was a problem hiding this comment.
There is a small typo: "whethere" should be "whether"
|
I'm also ok for a positive review. Maybe @kryzar wants to go over the changes one last time. |
src/sage/rings/function_field/drinfeld_modules/finite_drinfeld_module.py
Outdated
Show resolved
Hide resolved
src/sage/rings/function_field/drinfeld_modules/finite_drinfeld_module.py
Outdated
Show resolved
Hide resolved
| @@ -183,46 +257,62 @@ def frobenius_endomorphism(self): | |||
| deg = self.base_over_constants_field().degree_over() | |||
| return self._Hom_(self, category=self.category())(t**deg) | |||
|
|
|||
| def frobenius_charpoly(self, var='X'): | |||
| def frobenius_charpoly(self, var='X', algorithm='crystalline'): | |||
There was a problem hiding this comment.
Shouldn't 'motive' be an available option for the algorithm flag? This is implemented in DrinfeldModuleMorphism.
There was a problem hiding this comment.
I assumed that would get added when you and Xavier implement the other algorithms specialized for the frobenius case, but I can add a wrapper for the morphism method if you'd like.
There was a problem hiding this comment.
I'm actually fine with both solutions: adding it here or in a separate PR.
However, I think that if you add it, you should document it a bit in the generic method frobenius_charpoly.
There was a problem hiding this comment.
I think this should be implemented. However, I don't really care if it is here or in a separate PR. Maybe a separate PR would be easier for Joseph tho, so we can do that!
EDIT.: Well Joseph did it. Thanks a lot! But I agree that in that case the documentation needs to explain that we can use algorithm to choose between different algorithms, etc.
| K = self.base_over_constants_field() | ||
| n = K.degree(self._Fq) | ||
| char = self.characteristic() | ||
| norm = K(self.coefficients()[-1]).norm() |
There was a problem hiding this comment.
I think it would be a good idea!
…_module.py Co-authored-by: Antoine Leudière <clapped.hesitancy332@anonaddy.me>
…_module.py Co-authored-by: Antoine Leudière <clapped.hesitancy332@anonaddy.me>
src/sage/rings/function_field/drinfeld_modules/finite_drinfeld_module.py
Outdated
Show resolved
Hide resolved
…_module.py Co-authored-by: Antoine Leudière <clapped.hesitancy332@anonaddy.me>
|
Documentation preview for this PR (built with commit 28b0705; changes) is ready! 🎉 |
|
Thanks a lot @ymusleh! |
📚 Description
The goal of this PR is to implement methods for computing the characteristic polynomial (of the Frobenius endomorphism) of a Drinfeld Fq[t]-module. This largely closes #35028, though opportunities to implement other algorithms, as well as the question of computing characteristic polynomials for arbitrary morphisms, remain open.
Some further reading on characteristic polynomials of Drinfeld modules:
Gekeler, E.-U. (2008). Frobenius Distributions of Drinfeld Modules over Finite Fields. Transactions of the American Mathematical Society, 360(4), 1695–1721. http://www.jstor.org/stable/20161942
Angles, B. On some characteristic polynomials attached to finite Drinfeld modules. Manuscripta Math 93, 369–379 (1997). https://doi.org/10.1007/BF02677478
@kryzar @xcaruso @DavidAyotte @schost
📝 Checklist
⌛ Dependencies
None as it only requires the base implementation of Drinfeld modules which has been merged.