Ticket #12740 (closed enhancement: fixed)

Opened 14 months ago

Last modified 12 months ago

Revamp code for finding generators of modular forms spaces

Reported by: davidloeffler Owned by: craigcitro
Priority: major Milestone: sage-5.1
Component: modular forms Keywords:
Cc: AlexGhitza Work issues:
Report Upstream: N/A Reviewers: Jan Vonk
Authors: David Loeffler Merged in: sage-5.1.beta2
Dependencies: Stopgaps:

Description

The module sage.modular.modform.find_generators contains code which calculates, for a given congruence subgroup, forms which generate the graded ring of modular forms of that level; and code which uses these generators to calculate bases of q-expansions for spaces of very large weight, where modular symbol computations are less efficient.

This code is old and has not been very actively maintained for some while. Motivated by #12043, which uses this code, I decided to rework it a bit. With the attached patch, the new code

  • uses a randomized algorithm in some cases, adapted from Alan Lauder's code at #12043
  • can work over arbitrary base rings (not just the rationals)
  • handles series precision a bit more cleverly, using Sturm's bound
  • can now calculate generators for the ideal of cuspidal forms, and thus bases of q-expansions of large weight cuspidal submodules.

Attachments

trac_12740.patch Download (51.1 KB) - added by davidloeffler 14 months ago.
Patch against 5.0.beta9

Change History

Changed 14 months ago by davidloeffler

Patch against 5.0.beta9

comment:1 Changed 14 months ago by davidloeffler

  • Status changed from new to needs_review

comment:2 Changed 14 months ago by AlexGhitza

  • Cc AlexGhitza added

comment:3 Changed 12 months ago by janv

I just tested files and did long doctests with Sage 5.0 (on Lion!), went perfectly. Documentation builds, and got a 100% doctest coverage.

I have been playing around with some input and everything seems to be working well. Nonetheless, someone with more insight as to where to go look for possible bad output might want to have a look just to be sure. It seems to me though that this patch is impeccably written. I'll continue playing around for a bit and if nobody finds anything in the meanwhile I will review this positively.

comment:4 Changed 12 months ago by janv

  • Status changed from needs_review to positive_review
  • Reviewers set to Jan Vonk

I have been playing around with this for a while, and everything really seems to run smoothly. Very nice code and every small test I did gave the predicted answers. Since nobody more experienced objected in the meanwhile, I'll give this a positive review.

comment:5 Changed 12 months ago by jdemeyer

  • Status changed from positive_review to closed
  • Resolution set to fixed
  • Merged in set to sage-5.1.beta2
Note: See TracTickets for help on using tickets.