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

Implement Goss polynomials of Drinfeld modules #35991

Merged
merged 10 commits into from Aug 27, 2023

Conversation

DavidAyotte
Copy link
Member

@DavidAyotte DavidAyotte commented Jul 26, 2023

This PR implements Goss polynomials of Drinfeld modules. The interested PR reviewer should read section 3 of this paper. In particular, the recurrence relation (ii) of Proposition 3.4 of op. cit. is used in order to compute the $n$-th Goss polynomial of a Drinfeld module.

CC: @xcaruso @ymusleh @kryzar

📝 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

@DavidAyotte
Copy link
Member Author

Thanks for the comments, I think I have addressed all of them.

Copy link
Contributor

@xcaruso xcaruso left a comment

Choose a reason for hiding this comment

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

Just a small suggestion. Otherwise, LGTM.

Co-authored-by: Xavier Caruso <xcaruso@users.noreply.github.com>
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.

Thanks for this contribution @DavidAyotte. I approve the changes.

@xcaruso
Copy link
Contributor

xcaruso commented Jul 29, 2023

It seems that they are currently problems with automatic checkings.

@xcaruso
Copy link
Contributor

xcaruso commented Aug 1, 2023

Cc @vbraun
I'd like to give a positive review to this ticket but the build test fails for some reason that I did not understand and which is most likely unrelated to this ticket. What should I do?

@fchapoton
Copy link
Contributor

I have re-launched the checks on top of 10.1. The branch has been re-based.

@DavidAyotte
Copy link
Member Author

I have re-launched the checks on top of 10.1. The branch has been re-based.

Thanks Frédéric! The failing doctest does not seem to be related to this branch.

@fchapoton
Copy link
Contributor

Indeed, it is unrelated. Feel free to set to positive if you think this is ready.

@fchapoton
Copy link
Contributor

but note that the added raise is not correctly doctested

@xcaruso
Copy link
Contributor

xcaruso commented Aug 21, 2023

Good point, Frédéric.
Maybe you could fix this by adding a doctest, David.

@DavidAyotte
Copy link
Member Author

Thanks for pointing this out. I added the desired doctest.

@github-actions
Copy link

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

vbraun pushed a commit to vbraun/sage that referenced this pull request Aug 23, 2023
    
<!-- ^^^^^
Please provide a concise, informative and self-explanatory title.
Don't put issue numbers in there, do this in the PR body below.
For example, instead of "Fixes sagemath#1234" use "Introduce new method to
calculate 1+1"
-->
<!-- Describe your changes here in detail -->

<!-- Why is this change required? What problem does it solve? -->
<!-- If this PR resolves an open issue, please link to it here. For
example "Fixes sagemath#12345". -->
<!-- If your change requires a documentation PR, please link it
appropriately. -->

This PR implements Goss polynomials of Drinfeld modules. The interested
PR reviewer should read section 3 of [this
paper](https://link.springer.com/article/10.1007/BF01410204). In
particular, the recurrence relation (ii) of Proposition 3.4 of op. cit.
is used in order to compute the $n$-th Goss polynomial of a Drinfeld
module.

CC: @xcaruso @ymusleh @kryzar

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
<!-- Feel free to remove irrelevant items. -->

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

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on
- sagemath#12345: short description why this is a dependency
- sagemath#34567: ...
-->

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: sagemath#35991
Reported by: David Ayotte
Reviewer(s): Antoine Leudière, Xavier Caruso
@vbraun vbraun merged commit a102fbd into sagemath:develop Aug 27, 2023
12 of 13 checks passed
@mkoeppe mkoeppe added this to the sage-10.2 milestone Aug 27, 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

6 participants