Implement to_polynomial and from_polynomial methods for ModularFormsRing

[DavidAyotte]

The goal of this ticket is to implement two methods: one for the class GradedModularFormElement and one for the class ModularFormsRing.

• The first method is to_polynomial that will convert a graded form F into a polynomial P(X_1, X_2,... X_n) where the X_i correspond to the generators of the graded ring of modular forms in which F lives. Note that the result of this method is not unique in general.

sage: M = ModularFormsRing(1)
sage: f = (M.0)^3 + (M.1)^2;
2 - 288*q + 400032*q^2 + 33473664*q^3 + 796491936*q^4 + 9257371200*q^5 + O(q^6)
sage: f.to_polynomial(f)
X^3 + Y^2

• The second method is from_polynomial that will convert a polynomial into a graded form:

sage: M = ModularFormsRing(1)
sage: M.from_polynomial(X^3 + Y^2, [M.0, M.1])
2 - 288*q + 400032*q^2 + 33473664*q^3 + 796491936*q^4 + 9257371200*q^5 + O(q^6)

This ticket is part of #31560

Depends on #31559

CC: @videlec

Component: modular forms

Keywords: graded modular forms polynomials gsoc 2021

Author: David Ayotte

Branch/Commit: 2cdb21a

Reviewer: Vincent Delecroix

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

[DavidAyotte]

Branch: u/gh-DavidAyotte/modform_to_polynomials_generators

[sagetrac-git]

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

5bbc4f4

initial implementation of the method from_polynomial

[sagetrac-git]

Commit: 5bbc4f4

[sagetrac-git]

Changed commit from 5bbc4f4 to b2517ec

[sagetrac-git]

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

b2517ec

updated _generators_variables_dictionnary

[sagetrac-git]

Changed commit from b2517ec to 461d5f6

[sagetrac-git]

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

461d5f6

implementation of to_polynomial

[sagetrac-git]

Changed commit from 461d5f6 to 1e6a262

[sagetrac-git]

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

1e6a262

make polynomial_ring not private, assigned degrees to the variables

[sagetrac-git]

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

7d8b20a

fix docstring of to_polynomial

[sagetrac-git]

Changed commit from 1e6a262 to 7d8b20a

[videlec]

comment:10

The method _monomials_of_weight is hidden and it does not make much sense to have an EXAMPLES block for this. Furthermore, the code is not properly indented here.

The method _weights_of_generators is used only once in the code. It does not make much sense to have it given that its code is a single line.

In the documentation of polynomial_ring the generators are not unique. You would better change Return the polynomial ring of which ``self`` is a quotient. by Return a polynomial ring of which ``self`` is a quotient..

The method _monomials_of_weight is used nowhere. After :meth: in the documentation you need to add backticks like :meth:`my_method`. This was missing in from_polynomial.

Instead of

iter = 0
poly = 0
for p in monomials.keys():
    poly += soln[iter, 0]*p
    iter += 1

you would better write

poly = 0 # TODO : this should be the zero polynomial, not the integer 0
for iter, p in enumerate(monomials):
    poly += soln[iter, 0]*p

[sagetrac-git]

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

e4a974f

fix docstring, removed _weights_of_gemerators, small code syntax changes

[sagetrac-git]

Changed commit from 7d8b20a to e4a974f

[DavidAyotte]

comment:12

Thank you very much for the comments! Considering the method _monomials_of_weight, it is used in sage/modular/modform/element.py, line 3563 (for the method _homogeneous_to_polynomial). However, it is the only place that it is used. Should I make it not private, or simply erase the method and move the code inside the method _homogeneous_to_polynomial?

[sagetrac-git]

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

1b9332e	moved sage.modular.modform.find_generators.py in sage.rings, attempt at making decrecation work (without success)
68509f9	deprecation, added missing newline
2e78c45	revert to commit 7fdca22
99f8ab5	renamed find_generators.py to ring.py, added deprecation
62a9484	docstring updates, fixed weights_list method for zero element
ed90b5a	fixed deprecation, added deprecation for find_generators and basis_for_modform_space
d489a1f	moved deprecation doctests inside find_generators, fixed docstrings
bf19eec	added example info for GradedModularFormElement class, moved some doctests inside the `__init__` method (for coverage)
2e49907	fix docbuild error
d991d19	merge ticket 31559 'Make ModularFormRings manipulate formal objects' and fix conflicts

[sagetrac-git]

Changed commit from e4a974f to d991d19

[sagetrac-git]

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

b23e726

fix constant polynomials bug

[sagetrac-git]

Changed commit from 3539dbb to 182b16e

[DavidAyotte]

comment:23

Currently polynomial conversion does not work over base ring other than Q mostly because of the method gens_form (which only returns forms over Q no matter what the base ring is). There is also the method generators which only returns q-expansion, so I think that there might be more than just one change to do here. Hence, I think that this should be a topic of a new ticket (I was planning to open one soon) and I simply decided to add an if statement in order to check the base ring. What do you think?

[videlec]

comment:24

It is fine for now.

[DavidAyotte]

comment:25

I'm changing this to needs work because I will add input parsing for the element constructor (in order to check if the polynomial is a q-expansion).

[sagetrac-git]

Changed commit from 182b16e to b77460b

[sagetrac-git]

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

b77460b

added input parsing to _element_constructor_

[videlec]

comment:28

Is this the expected behavior

sage: M = ModularFormsRing(1)
sage: q,t = polygens(QQ, 'q,t')
sage: M(q^2+t^3)
Traceback (most recent call last):
...
NotImplementedError: conversion from q-expansion not yet implemented

[sagetrac-git]

Changed commit from b77460b to 7d10c2d

[sagetrac-git]

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

7d10c2d

fixed parsing for variable name in from_polynomial

[DavidAyotte]

comment:30

Hello Vincent,

Thank you for your comment, I changed a little bit the behavior of the code. What do you think? I'm all ears to any other ideas also.

[videlec]

comment:31

Hi David,

I don't like the fact that we rely on variable names to parse user input. What about using univariate (for q-expansion) versus multivariate (for generators)?

Also, you sometimes use defaults.DEFAULT_VARIABLE and sometimes the string 'q'. The usage could be cleaner.

[videlec]

comment:32

(cleaner and tested)

[sagetrac-git]

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

2cdb21a

fixed polynomial parsing, added doctest

[sagetrac-git]

Changed commit from 7d10c2d to 2cdb21a

[DavidAyotte]

comment:34

Replying to @videlec:

Hi David,

I don't like the fact that we rely on variable names to parse user input. What about using univariate (for q-expansion) versus multivariate (for generators)?

Also, you sometimes use defaults.DEFAULT_VARIABLE and sometimes the string 'q'. The usage could be cleaner.

Thank you for your feedback! I decided to drop completely the case for a univariate polynomial. At first, I wanted the code to work even if the number of variables was less or equal to the number of generators, but it turned out to be confusing with q-expansion, so as you said it is better to stick with multivariate polynomials. I also added some details in the documentation in order to make it clearer for the user (is it?).

For q-expansion, I think it will be better to stick with power series (and not univariate polynomial) instead in order to be consistent with the spaces of weighted modular forms:

sage: M = ModularForms(1, 4)
sage: q = polygen(QQ, 'q')
sage: E4 = 1 + 240*q + 2160*q^2 + 6720*q^3 + 17520*q^4 + 30240*q^5 # Univariate polynomial
sage: M(E4)
Traceback (most recent call last):
...
TypeError: can't initialize vector from nonzero non-list
sage: P.<q> = PowerSeriesRing(QQ)
sage: E4 = 1 + 240*q + 2160*q^2 + 6720*q^3 + 17520*q^4 + 30240*q^5 + O(q^6) # Power series
sage: M(E4)
1 + 240*q + 2160*q^2 + 6720*q^3 + 17520*q^4 + 30240*q^5 + O(q^6)

Moreover, observe that we have the following behavior:

sage: P.<q> = PowerSeriesRing(QQ)
sage: E4 = 1 + 240*q + 2160*q^2 + 6720*q^3 + 17520*q^4 + 30240*q^5 # Exact power series
sage: M(E4)
Traceback (most recent call last):
...
TypeError: unable to create modular form from exact non-zero polynomial

However, I do find the error message "can't initialize vector from nonzero non-list" not very explicit for the user (but that's an issue for an other ticket).

[videlec]

comment:35

Good point. I agree that compatibility with modular forms come first. The interface there is reasonable, to specify zero leading coefficients you need the O(q^n).

[vbraun]

Changed branch from u/gh-DavidAyotte/modform_to_polynomials_generators to 2cdb21a