Opened 14 years ago

Closed 4 years ago

#2330 closed enhancement (fixed)

modforms -- implement computation of weight 1 forms in Sage

Reported by: was Owned by: craigcitro
Priority: major Milestone: sage-8.2
Component: modular forms Keywords:
Cc: Merged in:
Authors: David Loeffler Reviewers: David Roe
Report Upstream: N/A Work issues:
Branch: 4229170 (Commits, GitHub, GitLab) Commit: 42291700f1c422209abf7862e87f0c65466ab9ec
Dependencies: Stopgaps:

Status badges

Description

Kevin Buzzard and I intend to do this ASAP based on algorithms he designed in 2002.

If you see this and are interested in helping, send me an email (wstein@…).

Change History (16)

comment:1 Changed 14 years ago by was

  • Type changed from defect to enhancement

comment:2 Changed 14 years ago by AlexGhitza

  • Component changed from number theory to modular forms
  • Owner changed from was to craigcitro

comment:3 Changed 8 years ago by jdemeyer

  • Milestone changed from sage-5.11 to sage-5.12

comment:4 Changed 8 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:5 Changed 8 years ago by vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:6 Changed 7 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4

comment:7 Changed 5 years ago by roed

  • Report Upstream set to N/A

comment:8 Changed 4 years ago by davidloeffler

  • Branch set to public/2330-weight1

comment:9 Changed 4 years ago by davidloeffler

  • Authors set to David Loeffler
  • Commit set to 84310cd1580d3d7b4b59ba916da4c71868ffd221
  • Status changed from new to needs_review

New commits:

84310cdImplementation of Schaeffer's algorithm for weight 1 forms

comment:10 Changed 4 years ago by roed

  • Reviewers set to David Roe
  • Status changed from needs_review to needs_work

It's awesome to have weight 1 forms!

Overall, the code looks good. A few issues to be addressed:

  • There's a plugin failure complaining about two lines with non-ASCII characters
  • The definition of R in the docstring of modular_ratio_space is off by one compared to the usage of R in the code.

comment:11 Changed 4 years ago by git

  • Commit changed from 84310cd1580d3d7b4b59ba916da4c71868ffd221 to 42291700f1c422209abf7862e87f0c65466ab9ec

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

4229170Trac 2330: fixes requested by reviewer

comment:12 Changed 4 years ago by davidloeffler

  • Status changed from needs_work to needs_review

This should address those two issues.


New commits:

4229170Trac 2330: fixes requested by reviewer

comment:13 Changed 4 years ago by davidloeffler

I'm a bit puzzled that four out of the five patchbot reports for the first version of the code are reporting test failures, all of which are nothing to do with the ticket as far as I can see -- is there something wrong with the patchbot? Most of them seem to be timeout errors.

comment:14 Changed 4 years ago by roed

  • Status changed from needs_review to positive_review

I'm not sure what's wrong with the other patchbots, but I've seen some of these errors in other places. It's certainly not a problem with your code.

Looks good!

comment:15 Changed 4 years ago by chapoton

  • Milestone changed from sage-6.4 to sage-8.2

comment:16 Changed 4 years ago by vbraun

  • Branch changed from public/2330-weight1 to 42291700f1c422209abf7862e87f0c65466ab9ec
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.