Opened 14 years ago

Closed 13 years ago

#6071 closed defect (fixed)

[with patch, positive review] Weight 1 Eisenstein series

Reported by: David Loeffler Owned by: David Loeffler
Priority: major Milestone: sage-4.1.1
Component: modular forms Keywords: eisenstein series
Cc: William Stein Merged in: Sage 4.1.1.alpha1
Authors: David Loeffler Reviewers: William Stein, David Loeffler
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

Computing weight 1 cusp forms is hard (cf. #2330), but computing weight 1 Eisenstein series isn't; only very slight modifications are needed to the code we already have.

Attachments (3)

trac_6071.patch (6.7 KB) - added by David Loeffler 14 years ago.
patch against 4.0.alpha0
trac_6071-weight1_eisenstein.patch (12.7 KB) - added by David Loeffler 13 years ago.
Replaces previous patch
trac_6071-referee.patch (1.9 KB) - added by William Stein 13 years ago.

Download all attachments as: .zip

Change History (14)

Changed 14 years ago by David Loeffler

Attachment: trac_6071.patch added

patch against 4.0.alpha0

comment:1 Changed 14 years ago by David Loeffler

Keywords: eisenstein series added
Status: newassigned
Summary: Weight 1 Eisenstein series[with patch, needs review] Weight 1 Eisenstein series

comment:2 Changed 14 years ago by John Cremona

I think I am too ignorant about weight 1 forms to review this honestly. All I can say is that there's a typo on line 285 of the patch (in a comment) -- chi <--> psi?

comment:3 Changed 14 years ago by David Loeffler

Cc: William Stein added

You are right about the comment typo, I will do a micro-patch to fix it when the pile of exam scripts on my desk has decreased far enough.

I am adding William to the CC list, since he certainly knows about weight 1 forms.

comment:4 Changed 13 years ago by William Stein

Summary: [with patch, needs review] Weight 1 Eisenstein series[with patch, needs work] Weight 1 Eisenstein series

REFEREE REPORT:

There is not a single example in this patch of computing an Eisenstein series of weight 1. Can you add some examples?

Also, this seems very very wrong to me:

 	361	        try: 
 	362	            d = self.dimension() 
 	363	        except NotImplementedError: 
 	364	            d = self._dim_eisenstein() 
 	365	        self.__module = free_module.VectorSpace(self.base_ring(), d) 

You've changed the dimension for *ambient* modular forms spaces to return the dimension of the Eisenstein subspace in case the dimension function isn't implemented. What if I take a space with both a cuspidal and eisenstein part -- it'll just say the dimension of the whole space is the dimension of the eisenstein subspace. Somehow I have the feeling you made this change to get things to work in a special case of interest to you, not worrying that you might break other cases.

comment:5 Changed 13 years ago by David Loeffler

Fair point; I will add some more examples.

For your second point: if you install the patch and try it out, you'll see that (for instance) ModularForms?(Gamma1(23), 1) will raise a NotImplementedError?, as it should, but EisensteinForms?(Gamma1(23),1) will work. The thing that you describe as "very very wrong" is forced by the general design we have for modular forms spaces, which insists that Eisenstein forms are always a subspace of an ambient ModularForms? space spanned by the *last* few basis vectors. The point of the workaround above is that when we can't find the dimension of the wt 1 cusp forms, we pretend that there aren't any for the purposes of working with Eisenstein series, but intercept any attempt to create the whole modular forms space (or its cuspidal part) by raising an error.

Even once we have proper code for calculating weight 1 cusp forms this will still be an issue, since for large N calculating dim S_1(Gamma_1(N)) is a serious and time-consuming calculation that we don't want to be forced to do solely in order to know how many zeros to stick at the front of the Eisenstein series.

comment:6 Changed 13 years ago by John Cremona

For what it's worth: that sounds like a good explanation. Perhaps when you put the examples in you can include some which illustrate those points too.

Changed 13 years ago by David Loeffler

Replaces previous patch

comment:7 Changed 13 years ago by David Loeffler

Summary: [with patch, needs work] Weight 1 Eisenstein series[with new patch, needs review] Weight 1 Eisenstein series

comment:8 Changed 13 years ago by William Stein

REFEREE REPORT:

  • This comment must be changed, and I've changed it in the attached referee patch:
File:           /scratch/wstein/build/sage-4.1/local/lib/python2.6/site-packages/sage/modular/modform/ambient.py
Definition:     M.module(self)
Docstring:
    
            Return the underlying free module corresponding to this space of
            modular forms. This is a free module (viewed as a tuple space) of
            the same dimension as this space over the same base ring.

This is because of the following example:

sage: M = ModularForms(Gamma1(23), 1,prec=20); M
Modular Forms space of dimension (unknown) for Congruence Subgroup Gamma1(23) of weight 1 over Rational Field
sage: M.module()
Vector space of dimension 11 over Rational Field

so it is now no longer the case that M.module().dimension() == M.dimension() as is stated in the docstring. The change should just be to state that "If the dimension of M can be computed, then [same as before]. Otherwise, the dimension of M.module() may be smaller. E.g., in the case of weight 1 forms..." Then include an example in the docstring that illustrates this.

This ticket should be changed to "[with patch; positive review]" as soon as somebody else signs off on the referee patch I've attached.

Changed 13 years ago by William Stein

Attachment: trac_6071-referee.patch added

comment:9 Changed 13 years ago by David Loeffler

Summary: [with new patch, needs review] Weight 1 Eisenstein series[with new patch, positive review] Weight 1 Eisenstein series

Fair point. The new patch looks fine to me, so positive review.

comment:10 Changed 13 years ago by Minh Van Nguyen

Authors: David Loeffler
Reviewers: William Stein, David Loeffler
Summary: [with new patch, positive review] Weight 1 Eisenstein series[with patch, positive review] Weight 1 Eisenstein series

comment:11 Changed 13 years ago by Minh Van Nguyen

Merged in: Sage 4.1.1.alpha1
Resolution: fixed
Status: assignedclosed

Merged:

  1. trac_6071-weight1_eisenstein.patch
  2. trac_6071-referee.patch
Note: See TracTickets for help on using tickets.