#5262 closed defect (fixed)
[with patch, with positive review] L-series attached to modular forms has a major bug in how it computes the sign of the functional equation
Reported by: | William Stein | Owned by: | David Loeffler |
---|---|---|---|
Priority: | major | Milestone: | sage-4.0 |
Component: | modular forms | Keywords: | |
Cc: | Merged in: | 4.0.alpha0 | |
Authors: | David Loeffler | Reviewers: | Craig Citro |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
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.
Attachments (1)
Change History (10)
comment:1 Changed 14 years ago by
Milestone: | sage-3.4 → sage-3.4.1 |
---|
comment:2 Changed 14 years ago by
Owner: | changed from Craig Citro to David Loeffler |
---|---|
Status: | new → assigned |
comment:3 Changed 14 years ago by
Summary: | L-series attached to modular forms has a major bug in how it computes the sign of the functional equation → [with patch, needs review] L-series attached to modular forms has a major bug in how it computes the sign of the functional equation |
---|
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.
comment:4 Changed 14 years ago by
Summary: | [with patch, needs review] L-series attached to modular forms has a major bug in how it computes the sign of the functional equation → [with patch, needs rebase] L-series attached to modular forms has a major bug in how it computes the sign of the functional equation |
---|
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.)
comment:5 Changed 14 years ago by
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.)
comment:6 Changed 14 years ago by
Summary: | [with patch, needs rebase] L-series attached to modular forms has a major bug in how it computes the sign of the functional equation → [with patch, needs review] L-series attached to modular forms has a major bug in how it computes the sign of the functional equation |
---|
comment:7 Changed 14 years ago by
Summary: | [with patch, needs review] L-series attached to modular forms has a major bug in how it computes the sign of the functional equation → [with patch, with positive review] L-series attached to modular forms has a major bug in how it computes the sign of the functional equation |
---|
Two thumbs up! I don't even see anything to nitpick about. Merge away!
comment:8 Changed 14 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Merged in Sage 4.0.alpha0.
Cheers,
Michael
comment:9 Changed 14 years ago by
Authors: | → David Loeffler |
---|---|
Merged in: | → 4.0.alpha0 |
Reviewers: | → Craig Citro |
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