Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

L-series attached to modular forms has a major bug in how it computes the sign of the functional equation #5262

Closed
williamstein opened this issue Feb 14, 2009 · 10 comments

Comments

@williamstein
Copy link
Contributor

This is wrong:


The following computation should produce identical values in the last
line:

E=EllipticCurve('37b2')
h=E.modular_form()
Lh = h.cuspform_lseries()
LE=E.lseries()
h.elliptic_curve()==E, Lh(1), LE(1)

The output is:

(True, 0, 0.725681061936153)

This is because the Atkin-Lehner sign is computed wrong in sage/modular/modform/element.py. In fact, there one finds the code:

            m = ModularSymbols(N,l,sign=1)
            n = m.cuspidal_subspace().new_subspace()
            e = (-1)**(l/2)*n.atkin_lehner_operator().matrix()[0,0]

Notice that m has absolutely nothing to do with the modular form!

The right fix is to implement an atkin_lehner_eigenvalue(...) function for modularforms, and that should in turn be implemented correctly, and should be called from the cuspform_lseries command.

Component: modular forms

Author: David Loeffler

Reviewer: Craig Citro

Merged: 4.0.alpha0

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

@sagetrac-mabshoff
Copy link
Mannequin

sagetrac-mabshoff mannequin commented Feb 14, 2009

comment:1

I already opened #5247 for this as I mentioned in the email, but I am closing that one as a dupe since this ticket has the better description.

This is not a ReST ticket, so bumping it to 3.4.1.

Cheers,

Michael

@sagetrac-mabshoff sagetrac-mabshoff mannequin modified the milestones: sage-3.4, sage-3.4.1 Feb 14, 2009
@loefflerd loefflerd mannequin assigned loefflerd and unassigned craigcitro May 1, 2009
@loefflerd
Copy link
Mannequin

loefflerd mannequin commented May 1, 2009

patch against 3.4.2.rc0

@loefflerd
Copy link
Mannequin

loefflerd mannequin commented May 1, 2009

comment:3

Attachment: trac_5262.patch.gz

Turns out there were actually two separate bugs: one for level 1 forms (which came up whenever the weight was not a multiple of 4) and one for forms of higher level. I've fixed both, and added doctests to check that they're fixed.

@loefflerd loefflerd mannequin added the s: needs review label May 1, 2009
@craigcitro
Copy link
Member

comment:4

So I think this patch looks pretty good by eye ... but I tried to apply it to a clean 3.4.2.rc0 tree, and I got some merge failures. David, could you just check to make sure you've got the right version posted and I'll go ahead and review this? (The merge failures don't seem too hard to sort out, but it'll probably still be faster for David than me.)

@craigcitro craigcitro changed the title L-series attached to modular forms has a major bug in how it computes the sign of the functional equation [needs rebase] L-series attached to modular forms has a major bug in how it computes the sign of the functional equation May 7, 2009
@loefflerd
Copy link
Mannequin

loefflerd mannequin commented May 7, 2009

comment:5

The merge failures are because the patch depends on my patch for #4357; I forgot to mention this in the description. Sorry. (This was because I independently implemented an "atkin_lehner_eigenvalue" function for newforms as part of fixing 4357, and then realised that this could be used to fix this one as well.)

@loefflerd loefflerd mannequin changed the title [needs rebase] L-series attached to modular forms has a major bug in how it computes the sign of the functional equation L-series attached to modular forms has a major bug in how it computes the sign of the functional equation May 7, 2009
@craigcitro
Copy link
Member

comment:7

Two thumbs up! I don't even see anything to nitpick about. Merge away!

@sagetrac-mabshoff
Copy link
Mannequin

sagetrac-mabshoff mannequin commented May 11, 2009

comment:8

Merged in Sage 4.0.alpha0.

Cheers,

Michael

@sagetrac-mabshoff sagetrac-mabshoff mannequin closed this as completed May 11, 2009
@loefflerd
Copy link
Mannequin

loefflerd mannequin commented Jun 7, 2009

Author: David Loeffler

@loefflerd
Copy link
Mannequin

loefflerd mannequin commented Jun 7, 2009

Reviewer: Craig Citro

@loefflerd
Copy link
Mannequin

loefflerd mannequin commented Jun 7, 2009

Merged: 4.0.alpha0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants