#4062 closed defect (fixed)
[with patch, positive review] Problems with Eisenstein series code?
Reported by: | craigcitro | Owned by: | craigcitro |
---|---|---|---|
Priority: | major | Milestone: | sage-3.2 |
Component: | modular forms | Keywords: | |
Cc: | Merged in: | 3.2.alpha3 | |
Authors: | Craig Citro, Peter Bruin | Reviewers: | David Loeffler |
Report Upstream: | Work issues: | ||
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
This was reported to sage-support
:
Hi, When computing Eisenstein series with a given character, Sage may return some forms with a wrong character. The following lines show an example of this: sage: G = DirichletGroup(7) sage: E = EisensteinForms(G[4]).eisenstein_series() sage: E[0].character() == G[4] False The problem appears to be caused by the condition if chi*psi == eps: in the function __find_eisen_chars in modular/modform/eis_series.py. According to Miyake, _Modular Forms_, Lemma 7.1.1 (cited in a comment in this function), it should be if chi == eps*psi: Another bug is that Sage uses an incorrect formula to compute q- expansions of Eisenstein series. Here the origin of the problem seems to be formula (5.3.1) in Stein, _Modular Forms: A Computational Approach_, where the psi(n) should be replaced by its complex conjugate (cf. Miyake, _Modular Forms_, Theorem 4.7.1 and the first three lines of page 271). The method __compute_general_case of the class EisensteinSeries in modular/modform/element.py reproduces this formula in the form v.append(sum([psi(n)*chi(m/n)*n**(k-1) for n in rings.divisors(m)])) Here psi should be ~psi. Thanks, Peter Bruin
Attachments (1)
Change History (6)
Changed 8 years ago by
comment:1 Changed 8 years ago by
- Milestone changed from sage-3.2.1 to sage-3.2
- Status changed from new to assigned
- Summary changed from Problems with Eisenstein series code? to [with patch, needs review] Problems with Eisenstein series code?
comment:2 Changed 8 years ago by
Further remarks from craig:
I posted a fix for the Eisenstein series bug that was reported a little bit ago. (Kevin, I'm cc'ing you because William said he mentioned this bug to you, too.) The original poster was right: his snippet of code takes a character chi, asks for a weight two Eisenstein series f in M_k(Gamma_1(N), chi), and then asks for f.character() -- and Sage says that it isn't chi! So that was the bug. The fix he starts detailing in his original post is completely the wrong direction -- he's somehow trying to correct the series to match the character that's getting returned. He later realized that the right fix was actually to change the character returned.
and from Kevin:
The only comment I had about the report was that it sounded to me that the poster had perhaps misunderstood the port of the theorem in Miyake to Sage---he seemed to be saying "this character should be replaced by its conjugate in several places", not realising that the notation in the sage code was that the character in the code sounded to me like it was by definition the conjugate of the character in Miyake.
comment:3 Changed 8 years ago by
- Summary changed from [with patch, needs review] Problems with Eisenstein series code? to [with patch, positive review] Problems with Eisenstein series code?
Patch applies fine to 3.2.alpha1 and all doctests in sage/modular/modform pass. I've also evaluated some Eisenstein series numerically at various points in the upper half-plane to check that the forms that are being returned really do have the characters they're supposed to, and that does check out, which is reassuring.
comment:4 Changed 8 years ago by
- Resolution set to fixed
- Status changed from assigned to closed
Merged in Sage 3.2.alpha3
comment:5 Changed 8 years ago by
- Merged in set to 3.2.alpha3
- Reviewers set to David Loeffler
Note: See
TracTickets for help on using
tickets.
This is a fix for the above problem. In fact, the fix was suggested by Peter Bruin, who originally reported the fix. Here's what he had to say:
The attached patch fixes this, and adds a few doctests to catch it in the future. Credit for the patch should also go to Peter.