-
-
Notifications
You must be signed in to change notification settings - Fork 480
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
Fix some quasimodular forms rings methods for congruence subgroups #34569
Comments
This comment has been minimized.
This comment has been minimized.
Commit: |
Branch pushed to git repo; I updated commit sha1. New commits:
|
comment:5
Nice!
- def polynomial_ring(self, names='E2, E4, E6'):
+ def polynomial_ring(self, names='g'): This is a backward incompatible change (maybe not so dramatic?). One possibility would be to use the signature
|
Reviewer: Vincent Delecroix |
comment:7
Replying to Vincent Delecroix:
Hi Vincent, thanks for your comment! I don't think there is any standard naming in the literature (even for Eisenstein series I've seen the same name for different normalizations). I agree however for a better naming. In line with your suggestion, I propose that we use
What do you think? |
Branch pushed to git repo; I updated commit sha1. New commits:
|
Branch pushed to git repo; I updated commit sha1. New commits:
|
comment:10
Sounds good. Maybe better to not mix lower and upper case. Also your current suggestion has collision: degree Maybe a more readable choice is
|
comment:12
Replying to Vincent Delecroix:
Well spotted!
That's a good suggestion, thanks! I implemented it and fixed some documentation strings. I don't know if it's the most optimal way as I use a couple of |
Branch pushed to git repo; I updated commit sha1. New commits:
|
comment:15
Sorry, I find it a bit confusing to have Also, it could happen that |
comment:16
Replying to Vincent Delecroix:
Don't be sorry, any comment and suggestion is more than appreciated. Moreover, your comment makes a lot of sense. Concerning the weights, it can happens that we have (relatively) a lot of generators of the same weights. As an example, for the group
Personally, I would be okay with this. Replying to Vincent Delecroix:
This is a good question. Yes I think that it would be possible to recognize when it is the case. However, according to my experiments (up to level 30 for Here are the computations I did: |
comment:17
Replying to David Ayotte:
Ok for me but not extremly nice. What about using letters after
If these |
comment:18
Replying to Vincent Delecroix:
Indeed. Old forms are precisely these
|
comment:19
Replying to Vincent Delecroix:
I think that it is a good suggestion (which is definitely possible to implement)! I also have a suggestion: instead of distinguishing the old/new forms, we could simply distinguish whether a generator is an element of a basis for the Eisenstein subspace or the cuspidal subspace. For example, we would have
In other words, we would use:
Note here the Also, currently the method |
comment:20
Indeed, distinguishing Eisenstein and cuspidal forms is a more sensible choice. I am a bit confused though. Why the ring generators are not chosen to be basis elements? Isn't it always possible? I agree that the names for |
comment:21
Replying to Vincent Delecroix:
I think that you are right and it would be possible to only choose basis elements, however, from what I understand, Sage looks for a minimal set of generators. The algorithm iterates over the spaces of weight up to 8 (which I think could be reduced) and at each step looks for new generators, by multiplying together generators found during previous steps. The idea behind algorithm 1 of this paper (see page 12) seems to describe relatively well what Sage currently does (up to some details).
Ok lets leave this for an other ticket. Also, I'm a bit curious: what are the advantages of making the polynomials live in |
Branch pushed to git repo; I updated commit sha1. New commits:
|
comment:24
The code looks good. From pyflakes
|
Branch pushed to git repo; I updated commit sha1. New commits:
|
Changed branch from u/gh-DavidAyotte/fix_to_polynomial_quasimodform to |
The implementation currently assumes that the congruence subgroup is
SL_2(Z)
and that the ring is generated byE2
,E4
andE6
. However, it is possible to create a ring for a congruence subgroup, which leads to some errors like this:The goal of this ticket is to make the code for quasimodular forms ring coherent for congruence subgroup and add more doctests.
CC: @videlec
Component: modular forms
Keywords: quasiforms to_polynomial
Author: David Ayotte
Branch/Commit:
51412f3
Reviewer: Vincent Delecroix
Issue created by migration from https://trac.sagemath.org/ticket/34569
The text was updated successfully, but these errors were encountered: