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 to_polynomial and from_polynomial for quasimodular forms #32336

Closed
DavidAyotte opened this issue Aug 5, 2021 · 26 comments
Closed

Implement to_polynomial and from_polynomial for quasimodular forms #32336

DavidAyotte opened this issue Aug 5, 2021 · 26 comments

Comments

@DavidAyotte
Copy link
Member

The goal of this ticket is like the ticket #32135: implement the methods to_polynomial and from_polynomial for the class QuasiModularFormElement (see #31560, #31512)

In particular, we want to be able to write a quasimodular form as a polynomial in terms of the generators of the graded algebra of quasimodular forms. Examples:

sage: QM = QuasiModularForms(1)
sage: f = QM.0 + QM.1
sage: f.to_polynomial()
E2 + E4
sage: P.<E2, E4, E6> = QQ[]
sage: g = QM.from_polynomial(E2 + E4); g
2 + 216*q + 2088*q^2 + 6624*q^3 + 17352*q^4 + 30096*q^5 + O(q^6)
sage: f == g
True

Depends on #31512

CC: @videlec

Component: modular forms

Author: David Ayotte

Branch/Commit: 6b729bc

Reviewer: Marc Mezzarobba, Travis Scrimshaw

Issue created by migration from https://trac.sagemath.org/ticket/32336

@DavidAyotte
Copy link
Member Author

@DavidAyotte
Copy link
Member Author

Author: David Ayotte

@DavidAyotte
Copy link
Member Author

Commit: 094d64a

@DavidAyotte
Copy link
Member Author

Last 10 new commits:

b2517ecupdated _generators_variables_dictionnary
461d5f6implementation of to_polynomial
1e6a262make polynomial_ring not private, assigned degrees to the variables
7d8b20afix docstring of to_polynomial
e4a974ffix docstring, removed _weights_of_gemerators, small code syntax changes
1038d68rename find_generator.py into ring.py
d991d19merge ticket 31559 'Make ModularFormRings manipulate formal objects' and fix conflicts
9304a66Merge ticket 32135 'Implement to_polynomial and from_polynomial methods for ModularFormsRing' into t/31512/implementation_of_the_graded_quasimodular_forms_ring
526c884fix constant polynomials bug
094d64aNew methods in ring.py: ngens, polynomial_ring, from_polynomial. New methods in element.py: to_polynomial, weights_list, is_homogeneous, weight, homogeneous_components.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 6, 2021

Branch pushed to git repo; I updated commit sha1. New commits:

c8fb9d1fix some docstring. small fix.
6057a6ffix failing doctests, pyflakes, block, tiple colon

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 6, 2021

Changed commit from 094d64a to 6057a6f

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 8, 2021

Changed commit from 6057a6f to 71ba9f0

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 8, 2021

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

6984a26fix failing doctests, pyflakes, block, tiple colon
5a373a3remove __base_ring attributes and method, changed some syntax
600fdfffix typos
77e8a3dadd congruence subgroups support
6884d6dadd input parsing for q-expansions in _element_constructor_
31432aefixed input parsing for q-expansion, added documentation to reference manual, added a bibliographical reference
5ef7767Merge branch 't/31559/make_modularformrings_manipulate_formal_objects' into t/31512/implementation_of_the_graded_quasimodular_forms_ring
377d88bresolved merge conflicts
100f079added missing newline, fix capitalization of title
71ba9f0fixed merge conflicts

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 14, 2021

Branch pushed to git repo; I updated commit sha1. New commits:

936b3b6fix docbuild errors

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 14, 2021

Changed commit from 71ba9f0 to 936b3b6

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 27, 2021

Branch pushed to git repo; I updated commit sha1. New commits:

782d722Merge branch 'develop' into quasiform_to_from_polynomial

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 27, 2021

Changed commit from 936b3b6 to 782d722

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 27, 2021

Branch pushed to git repo; I updated commit sha1. New commits:

d4eb6cdfix small error

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 27, 2021

Changed commit from 782d722 to d4eb6cd

@mezzarobba
Copy link
Member

comment:8

Just a minor remark: almost all other methods that convert something to a polynomial are called just polynomial, not to_polynomial. (I have no idea if there is a good reason to deviate from that here.)

@DavidAyotte
Copy link
Member Author

comment:9

Thank you very much for bringing up this remark! I was not aware of this fact, as I never used polynomial conversion methods before. I agree with you: everything should be consistent. What do you think about adding an alias polynomial = to_polynomial?

I will also fix the work in #32135 to be consistent with the option we decide.

@mezzarobba
Copy link
Member

comment:10

Replying to @DavidAyotte:

Thank you very much for bringing up this remark! I was not aware of this fact, as I never used polynomial conversion methods before. I agree with you: everything should be consistent. What do you think about adding an alias polynomial = to_polynomial?

I am not sure that the alias is necessary, but I have nothing against it!

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 1, 2021

Branch pushed to git repo; I updated commit sha1. New commits:

a3245b8fix failing doctests

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 1, 2021

Changed commit from d4eb6cd to a3245b8

@tscrim
Copy link
Collaborator

tscrim commented Oct 3, 2021

comment:12

It seems like you also have some stuff from #32357 merged in here. While I am not strictly opposed to that, I feel it would be better to have this ticket be "clean" in the sense it only has the code going to/from a polynomial ring.

I agree with Marc about calling the method polynomial. I might want to avoid the alias because it clutters things a bit. I am not that opposed to it, but I would want to call the method polynomial with to_polynomial = polynomial to subtly indicate which is the "main" method.

@DavidAyotte
Copy link
Member Author

comment:13

It seems like you also have some stuff from #32357 merged in here.

Thank you very much for finding this. I think that you are talking about the method differentiation_operator (are you?). In fact, this method was written at the beginning of my GSOC project (in #31512) and it happens that I removed it in #31512, but it must have slipped through the cracks when I resolved the merge conflicts. It is indeed important to clean this!

Concerning the to_polynomial method, I will rename it as you and Marc suggested. For now, I will also leave an alias to_polynomial = polynomial.

Again, thank you very much for taking the time to look at this.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 6, 2021

Branch pushed to git repo; I updated commit sha1. New commits:

6b729bcrenamed to_polynomial into polynomial, removed differentiation_operator

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 6, 2021

Changed commit from a3245b8 to 6b729bc

@tscrim
Copy link
Collaborator

tscrim commented Oct 6, 2021

Reviewer: Marc Mezzarobba, Travis Scrimshaw

@tscrim
Copy link
Collaborator

tscrim commented Oct 6, 2021

comment:15

Thanks. LGTM.

@vbraun
Copy link
Member

vbraun commented Oct 10, 2021

Changed branch from u/gh-DavidAyotte/quasiform_to_from_polynomial to 6b729bc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants