Opened 14 years ago

Closed 13 years ago

#4339 closed enhancement (fixed)

modular forms -- incorporate Nils Skoruppa's code for computing generators for the ring of modular forms of given level

Reported by: was Owned by: craigcitro
Priority: major Milestone: sage-4.3.2
Component: modular forms Keywords:
Cc: NilsSkoruppa Merged in:
Authors: Reviewers:
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

GitHub link to the corresponding issue

Description

Craig has an email from Nils with this code.

Change History (6)

comment:1 Changed 14 years ago by mabshoff

Cc: NilsSkoruppa added

I am adding Nils to the CC here so he is aware of the ticket. IIRC it was also a team effort to write that code, but I could be wrong. Some copy of Nil's code seems to be at

​http://modular.math.washington.edu/home/ljpk/sage-add-ons/nils/

Cheers,

Michael

comment:2 Changed 14 years ago by mabshoff

Craig,

any news on this front? Maybe somebody else would be willing to start the review process if the code was posted?

Cheers,

Michael

comment:3 Changed 14 years ago by craigcitro

Type: defect → enhancement

comment:4 Changed 13 years ago by davidloeffler

Report Upstream: → N/A
Status: new → needs_review

Can I suggest closing this ticket? I independently fixed the code for the ring of modular forms last year, and it got committed as part of #5727 (changeset 11961). I've since looked at Nils' code and it seems that he and I independently fixed the same bug in more or less the same way. Certainly find_generators.py now works, and has extensive doctests and passes them all.

I'm setting this to "needs review" (if you like, I'm asking for a review for the empty patch).

David

comment:5 Changed 13 years ago by was

Status: needs_review → positive_review

Looks good to me. I can't see any bugs, and you didn't lower the coverage score.

comment:6 Changed 13 years ago by craigcitro

Resolution: → fixed
Status: positive_review → closed

I agree. I intended to close this ticket once you (David) fixed up that code, but apparently forgot.

Note: See TracTickets for help on using tickets.