Opened 13 years ago

Closed 13 years ago

#6606 closed enhancement (fixed)

[with patch, positive review] Add a more efficient implementation of index for Gamma(N).

Reported by: Simon Morris Owned by: Craig Citro
Priority: minor Milestone: sage-4.1.1
Component: modular forms Keywords:
Cc: David Roe Merged in: Sage 4.1.1.alpha1
Authors: Simon Morris Reviewers: John Cremona, David Roed
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

Gamma(N).index used the default implementation which was slow. Attached is a new implementation which works for the specific subgroup.

Attachments (2)

gamma.patch (1.3 KB) - added by Simon Morris 13 years ago.
gamma.2.patch (1.3 KB) - added by Simon Morris 13 years ago.

Download all attachments as: .zip

Change History (7)

Changed 13 years ago by Simon Morris

Attachment: gamma.patch added

comment:1 Changed 13 years ago by John Cremona

Reviewers: John Cremona
Summary: [with patch; needs review] Add a more efficient implementation of index for Gamma(N).[with patch; needs work] Add a more efficient implementation of index for Gamma(N).

The code loooked (though I would have written p(3*e-2)*(p*p-1) ) but after applying to 4.1:

sage: Gamma(19).index()
---------------------------------------------------------------------------
NameError                                 Traceback (most recent call last)

/home/john/.sage/temp/ubuntu/25083/_home_john__sage_init_sage_0.py in <module>()

/home/john/sage-4.1/local/lib/python2.6/site-packages/sage/modular/arithgroup/congroup_gamma.pyc in index(self)
    105             32893086819240
    106         """
--> 107         return prod([p**(3*e) - p**(3*e-2) for (p,e) in self.level().factor()])
    108 

Looks like someone forgot to run sage -t before submitting the patch...

Changed 13 years ago by Simon Morris

Attachment: gamma.2.patch added

comment:2 Changed 13 years ago by Simon Morris

Summary: [with patch; needs work] Add a more efficient implementation of index for Gamma(N).[with patch; needs review] Add a more efficient implementation of index for Gamma(N).

comment:3 Changed 13 years ago by David Roe

Summary: [with patch; needs review] Add a more efficient implementation of index for Gamma(N).[with patch; positive review] Add a more efficient implementation of index for Gamma(N).

I ran sage -t after applying the patch, and all tests pass. Looks good to me.

comment:4 Changed 13 years ago by John Cremona

That's better!

comment:5 Changed 13 years ago by Minh Van Nguyen

Authors: simonSimon Morris
Merged in: Sage 4.1.1.alpha1
Resolution: fixed
Reviewers: John CremonaJohn Cremona, David Roed
Status: newclosed
Summary: [with patch; positive review] Add a more efficient implementation of index for Gamma(N).[with patch, positive review] Add a more efficient implementation of index for Gamma(N).

Simon: The patch gamma.2.patch doesn't contain your username. I've committed it in your name.

Note: See TracTickets for help on using tickets.