Make ModularFormRings manipulate formal objects

[videlec]

Currently, ModularFormRings is manipulating truncated q-series. It is desirable to be able to manipulate the ring of modular forms. In other words, being able to define E4 + E6 and access its homogeneous components.

This ticket is part of #31560

Depends on #32168

CC: @DavidAyotte

Component: modular forms

Keywords: gsoc2021, modular forms rings

Author: David Ayotte

Branch/Commit: 088139a

Reviewer: Vincent Delecroix

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

[mkoeppe]

comment:1

Sage development has entered the release candidate phase for 9.3. Setting a new milestone for this ticket based on a cursory review of ticket status, priority, and last modification date.

[DavidAyotte]

Branch: u/gh-DavidAyotte/make_modularformrings_manipulate_formal_objects

[DavidAyotte]

Commit: 5f23e4d

[DavidAyotte]

New commits:

5f23e4d

Initial implementation of GradedModularFormElement. The class ModularFormsRing now inherit from Parent and UniqueRepresentation

[sagetrac-git]

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

33a8cd5

Added the methods `_neq_` and is_homogeneous.

[sagetrac-git]

Changed commit from 5f23e4d to 33a8cd5

[sagetrac-git]

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

6faa4ba

Implemented coercion. Added documentation. Fixed a bug in `_add_` method.

[sagetrac-git]

Changed commit from 33a8cd5 to 6faa4ba

[sagetrac-git]

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

9216db9

Moved the class GradedModularFormElement into modular/modform/element.py. Fixed some issues with coercion.

[sagetrac-git]

Changed commit from 6faa4ba to 9216db9

[videlec]

Author: David Ayotte

[videlec]

Reviewer: Vincent Delecroix

[videlec]

comment:9

In the constructor I would either allow a list of modular forms or a dictionary. In both situations there should be a check that the data is consistent. And in the case it is a dictionary, you would better not write

    self.__mf_dict = mf_dict

because this uses the dictionary provided in the input. This dictionary can be modified outside of the scope of the class and corrupt the data in a scenario such as

sage: d = {}
sage: f = M(d)
sage: d[3] = 'hello'  # data corruption

Why do you have a method _dictionary and not use directly __mf_dict. This add one level of indirection for no purpose that I can see.

When you iterate over the keys and values of a dictionary it is best to use the items() method as in

def __neg__(self):
    GM = self.__class__
    minus_self = {k:-v for k,v in self.__mf_dict.items()}
    return GM(self.parent(), minus_self)

You should always take care of removing the entries in the dictionary whose values are zero (in the constructor and after a binary operation). Otherwise a method such as is_homogeneous gets completely wrong.

Use __neg__ and not _neg_ (this method does not go through corecion).

In __getitem__ you would better return the zero of the base ring and not the python integer 0.

Don't write x.__getitem__(k) but x[k]. The latter is much faster in general.

Rather than

def weights(self):
    wts = list(self._dictionnary().keys())
    wts.sort()
    return wts

you could use

def weights(self):
    return sorted(self.__mf_dict)

Instead of

def is_homogeneous(self):
    if len(self._dictionnary())>1:
        return False
    return True

you could use

def is_homogeneous(self):
    return len(self.__mf_dict) <= 1

For the documentation, it is INPUT and not INPUTS. And the items afterwards should not be indented.

It would be better to inherit from ModuleElement (not doing this causes trouble with the coercion model). Once done, implement the method _lmul_ for scalar multiplications (it should be possible to do integer x (element of the ring of modular forms).

Another method that is lacking is __bool__ (for the zero test). Once the dictionary is clean (ie no zero value) you could simply do

def __bool__(self):
    return bool(self.__mf_dict)

Also write methods is_zero and is_one that are standard ones for rings.

[videlec]

comment:10

If qexp is simply an alias of q_expansion write qexp = q_expansion. The documentation of q_expansion could mention the fact that such an alias exists.

The method _repr_ could be simplified to

def _repr_(self):
    return str(self.q_expansion())

Since now ModularFormsRing inherit from Parent you could use the test suite. You should add the following test in the constructor ModularFormsRing.__init__

TESTS::

    sage: M = ModularFormsRing(1)
    sage: TestSuite(M).run()

This test suite should also be run with other groups (especially non-congruence once) and other base rings.

The coercion is not tested, right? You should add various tests for

(modular form) + (element of ModularFormsRing)
(element of ModularFormsRing) + (modular form)
(scalar) + (element of ModularFormsRing)
(element of ModularFormsRing) + (scalar)

To determine coercions, one alternative could be to rely on what is implemented for modular forms. Namely the ModularFormsRing should have a method to obtain the modular forms of a given weight (something like M.graded_component(self, weight). It would then be possible to use coercions with respect to whatever is the result of this method.

[sagetrac-git]

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

b631370

updates: verifications in the constructor of GradedModularFormElement, changed some code syntax, fixed some bugs, updated documentation

[sagetrac-git]

Changed commit from 9216db9 to b631370

[sagetrac-git]

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

b036435

Implemented `_richcmp_` and `_bool_` for elements, fixed some typos in the documentation, updated q_expansion method

[sagetrac-git]

Changed commit from b631370 to b036435

[sagetrac-git]

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

71d7354

Merge branch 'develop' into t/31559/make_modularformrings_manipulate_formal_objects

[sagetrac-git]

Changed commit from b036435 to 71d7354

[sagetrac-git]

Changed commit from 71d7354 to 33de73a

[sagetrac-git]

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

33de73a

implemented `_pushout_`, fixed some docstrings, added doctests, fixed bugs

[videlec]

comment:75

Replying to @DavidAyotte:

Considering the "same doctest", in the first one I am testing the deprecated function name (as you suggested in comment 49) and in the second one I am testing the deprecation import warning:
<...>

Indeed. I read too quickly! Sorry.

[sagetrac-git]

Changed commit from d489a1f to bf19eec

[sagetrac-git]

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

bf19eec

added example info for GradedModularFormElement class, moved some doctests inside the `__init__` method (for coverage)

[DavidAyotte]

comment:77

In last commit, I moved some doctests inside the __init__ method of GradedModularFormElement class because it was lacking doctests.

[sagetrac-git]

Changed commit from bf19eec to 2e49907

[sagetrac-git]

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

2e49907

fix docbuild error

[videlec]

comment:79

All right. Let us move forward.

[DavidAyotte]

comment:81

The index.rst file was not updated after renaming find_generator.py into ring.py and so the documentation does not show up in the reference manual. I'm fixing this.

[sagetrac-git]

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

088139a

reference manual fix

[sagetrac-git]

Changed commit from 2e49907 to 088139a

[videlec]

comment:84

thanks

[vbraun]

Changed branch from u/gh-DavidAyotte/make_modularformrings_manipulate_formal_objects to 088139a