Opened 5 years ago

Closed 4 years ago

Last modified 4 years ago

#18086 closed enhancement (fixed)

Twists of newforms

Reported by: pbruin Owned by:
Priority: major Milestone: sage-6.6
Component: modular forms Keywords: twist newform Dirichlet character
Cc: wuthrich Merged in:
Authors: Peter Bruin Reviewers: Chris Wuthrich, David Loeffler
Report Upstream: N/A Work issues:
Branch: 751bf48 (Commits) Commit:
Dependencies: #6837, #18436, #18479 Stopgaps:

Description (last modified by pbruin)

As a follow-up to #6837, twists of newforms by Dirichlet characters should be implemented. This ticket implements a method Newform.twist(), which returns the twisted form as a Newform.

Change History (35)

comment:1 Changed 5 years ago by pbruin

  • Authors set to Peter Bruin

Working on this; the main challenge is determining the level of the twisted form.

comment:2 Changed 5 years ago by pbruin

  • Branch set to u/pbruin/18086-Newform_twist
  • Commit set to dc9a9cc8b08d912974bed3001873f8487903b3ae

A first version; this is still unable to compute the level in many cases.


New commits:

f415597trac 6837: implement twist of modular form by a Dirichlet character
57b7688Trac 6837: improve docstring formatting
20ab273Trac 6837: slightly simplify ModularForm_abstract.twist()
fd741eaTrac 6837: take a lower level for the twisted form
0584707Trac 6837: expand documentation
68354fbTrac 6837: twist of cusp form should be a cusp form
766ef05Trac 6837: update/expand doctests
09d70e8Trac 6837: always use Atkin-Li formula for the level of the twisted form
a53ee1fTrac 18068: move ModularForm_abstract.twist() to ModularFormElement
dc9a9ccTrac 18086: implement twisting of newforms by Dirichlet characters

comment:3 Changed 5 years ago by git

  • Commit changed from dc9a9cc8b08d912974bed3001873f8487903b3ae to 2e66ac6a56894901ef8039135e8e3df0e16e445d

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

44c21d5Trac 18086: move ModularForm_abstract.twist() to ModularFormElement
2e66ac6Trac 18086: implement twisting of newforms by Dirichlet characters

comment:4 Changed 5 years ago by git

  • Commit changed from 2e66ac6a56894901ef8039135e8e3df0e16e445d to 5284dda20fd2c4932e36899f8d3394eebc435030

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

e576ad4Trac 6837: replace Unicode dash by ASCII hyphen
2d3c8a2Trac 6837: move ModularForm_abstract.twist() to ModularFormElement
d9e849aTrac 6837: handle forms without character
5f5694aTrac 6837: use the right base ring for the twisted form
5284ddaTrac 18086: implement twisting of newforms by Dirichlet characters

comment:5 Changed 5 years ago by pbruin

  • Description modified (diff)

comment:6 Changed 5 years ago by git

  • Commit changed from 5284dda20fd2c4932e36899f8d3394eebc435030 to f1533dde29e01cbca26cb57e51a680a3853c6440

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

f1533ddTrac 18086: implement twisting of newforms by Dirichlet characters

comment:7 Changed 5 years ago by pbruin

  • Description modified (diff)
  • Status changed from new to needs_review

Calculating the level of the twisted form is still only implemented in the cases where [Atkin-Li, Theorem 3.1] applies. However, I currently don't have time to find out if this can easily be done in greater generality, and the current implementation suffices for #18061, which depends on this ticket.

comment:8 Changed 5 years ago by pbruin

  • Cc wuthrich added

comment:9 follow-up: Changed 5 years ago by wuthrich

  • Reviewers set to Chris Wuthrich
  • Status changed from needs_review to needs_work

Two issues. One a very small missing second ` after check in the docstring. The other will need more work:

sage: M = CuspForms(Gamma1(13),2)
sage: f = M.newforms('a')[0]
sage: G = DirichletGroup(3)
sage: chi = G.0
sage: f.twist(chi)

returns

TypeError: no common canonical parent for objects with parents: 'Number Field in a0 with defining polynomial x^2 + 3*x + 3' and 'Cyclotomic Field of order 2 and degree 1'

It appears when the form has coefficients outside ZZ.

By the way, this could be another nice example.

sage: D = Newforms(1,12)[0]
sage: G = DirichletGroup(3)
sage: chi = G.0
sage: D.twist(chi)

comment:10 Changed 5 years ago by git

  • Commit changed from f1533dde29e01cbca26cb57e51a680a3853c6440 to dd977f21a88f86e7ecc7712c1b921a23340f6879

Branch pushed to git repo; I updated commit sha1. New commits:

a38e707Trac 18436: fix dimension formulae for modular forms
aa6227fMerge branch 'ticket/18436-dimension_formulae' into ticket/18086-Newform_twist
dd977f2Trac 18086: reviewer comments

comment:11 in reply to: ↑ 9 Changed 5 years ago by pbruin

  • Dependencies changed from #6837 to #6837, #18436

Replying to wuthrich:

Two issues. One a very small missing second ` after check in the docstring.

Done, thanks,

The other will need more work:

sage: M = CuspForms(Gamma1(13),2)
sage: f = M.newforms('a')[0]
sage: G = DirichletGroup(3)
sage: chi = G.0
sage: f.twist(chi)

returns

TypeError: no common canonical parent for objects with parents: 'Number Field in a0 with defining polynomial x^2 + 3*x + 3' and 'Cyclotomic Field of order 2 and degree 1'

It appears when the form has coefficients outside ZZ.

This is because the field of values of chi and the field of coefficients of f are not compatible (this may be a bad example; chi.base_ring() is CyclotomicField(2), which is actually QQ but not recognised as such). In such cases, one could try to take a compositum, but I think it is better to leave it to the user to specify the correct field.

I added youe example, but with chi having coefficients in QQ. Unfortunately, this uncovered a new bug; see #18436.

By the way, this could be another nice example.

sage: D = Newforms(1,12)[0]
sage: G = DirichletGroup(3)
sage: chi = G.0
sage: D.twist(chi)

This is actually the first doctest in a different guise. I added another one to check that twisting by the trivial character (with values in QQ) does not change the form.

comment:12 Changed 5 years ago by pbruin

  • Status changed from needs_work to needs_review

comment:13 follow-up: Changed 5 years ago by davidloeffler

Here's a bug I found while testing your Atkin--Lehner ticket:

sage: f = Newforms(Gamma1(25), names='a')[1]
sage: eps = f.character()
sage: f.twist(eps)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
...
TypeError: No compatible natural embeddings found for Complex Lazy Field and Number Field in a1 with defining polynomial x^8 + 5*x^7 + 11*x^6 + 10*x^5 + x^4 + 10*x^3 + 26*x^2 - 10*x + 1

comment:14 in reply to: ↑ 13 Changed 5 years ago by pbruin

Replying to davidloeffler:

Here's a bug I found while testing your Atkin--Lehner ticket:

sage: f = Newforms(Gamma1(25), names='a')[1]
sage: eps = f.character()
sage: f.twist(eps)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
...
TypeError: No compatible natural embeddings found for Complex Lazy Field and Number Field in a1 with defining polynomial x^8 + 5*x^7 + 11*x^6 + 10*x^5 + x^4 + 10*x^3 + 26*x^2 - 10*x + 1

This is in fact caused by a bug in DirichletCharacter.minimize_base_ring(): in the above setting, typing

sage: eps.minimize_base_ring()

gives the same error. This method is called via the ModularForms constructor, so e.g. ModularForms(eps, 2) also fails.

comment:15 Changed 5 years ago by pbruin

  • Status changed from needs_review to needs_work
  • Work issues set to speed

The following is much slower than it should be (about 6.7 seconds on the system I tested this on):

sage: F = Newforms(299,names='a')[0]
sage: F.twist(F.character())
q + a0*q^2 + a0*q^3 + (-a0 - 1)*q^4 + (-a0 - 1)*q^5 + O(q^6)

This is caused by a lot of unnecessary computations of roots of unity in various number fields; see David's comment:16:ticket:18061.

comment:16 Changed 5 years ago by pbruin

  • Dependencies changed from #6837, #18436 to #6837, #18436, #18479

comment:17 Changed 4 years ago by pbruin

  • Status changed from needs_work to needs_review
  • Work issues speed deleted

It turns out that the slowness in comment:15 can be solved by improving the construction of Dirichlet groups so as to avoid computing roots of unity until they are needed. (This basically comes down to allowing Dirichlet groups without a user-specified root of unity.) Since the problem is orthogonal to this ticket, I will create a different ticket to solve it, and I am setting this one back to needs_review. (It probably won't be necessary to make either ticket depend on the other.)

comment:18 Changed 4 years ago by davidloeffler

  • Branch changed from u/pbruin/18086-Newform_twist to u/davidloeffler/18086-Newform_twist

comment:19 Changed 4 years ago by davidloeffler

  • Commit changed from dd977f21a88f86e7ecc7712c1b921a23340f6879 to 2e82464519f002200db7d6fb1bd9ee56f49b67b8

I rebased the trac branch on top of #18479. (I hope I did this right, I'm not very good at git yet!)


New commits:

2358aa1Trac 18086: implement twisting of newforms by Dirichlet characters
2e82464Trac 18086: reviewer comments

comment:20 Changed 4 years ago by davidloeffler

Oops, I seem to have broken something and I can't work out why -- this Git branch won't compile. Sorry, I don't know how to fix this! Any offers?

comment:21 Changed 4 years ago by wuthrich

I won't claim to be an gitexpert, but I guess a "merge" is better than a "rebase" when other people are involved. The commit at which the two branches merge then tells you exactly what went on.

In this case the two involve disjoint set of files so it should be straight forward. So I am surprised this does not work.

Your branch (merged into the 6.8.beta1) compiles fine for me. I will try on it own later. Where does your compilation get a problem ?

comment:22 Changed 4 years ago by davidloeffler

When I try compiling this branch, it raises an error at sage/ext/interrupt/interrupt.pxd and refuses to continue.

comment:23 Changed 4 years ago by wuthrich

No, I can't reproduce this. It all works fine for me on that branch. That file should not have been touched and should not compile. Probably you best abandon your branch and start a new one pulling from the trac and try compiling again.

comment:24 Changed 4 years ago by davidloeffler

I worked out the problem. The point is that the current "develop" branch doesn't actually build on my machine -- as is sometimes the case with unreleased betas -- and the branch at #18479 hangs off develop, not master.

comment:25 Changed 4 years ago by davidloeffler

  • Branch changed from u/davidloeffler/18086-Newform_twist to u/davidloeffler/18086-twists-merged

comment:26 Changed 4 years ago by git

  • Commit changed from 2e82464519f002200db7d6fb1bd9ee56f49b67b8 to d57edf416adaace77e013e71223b1b5bae7d686a

Branch pushed to git repo; I updated commit sha1. New commits:

d57edf4Trac 18086: fix two small bugs involving Dirichlet characters revealed in testing

comment:27 Changed 4 years ago by davidloeffler

  • Reviewers changed from Chris Wuthrich to Chris Wuthrich, David Loeffler

Right, I created a new branch which has only the relevant commits on top of current "master", using the rather useful "git cherry-pick" command, and now I'm testing that.

But I don't think the example from comment 14 is fixed yet. With #18479 in place, calling eps.minimize_base_ring() works fine, but still f.twist(eps) returns exactly the same error. I've traced this to sloppy coding in some dimension counting functions. I also spotted another error in minimize_base_ring(), unrelated to #18479, when the coefficient field is a relative number field.

With these two changes the code looks sound. Peter: if you're happy with these changes, please set the ticket to positive review.

comment:28 Changed 4 years ago by davidloeffler

Aargh! Now it won't merge and I can't work out why! This is giving me a serious headache.

comment:29 Changed 4 years ago by wuthrich

David

I got a merge conflict with #18436. But relax, probably Peter is better than all of us and can fix all of it in no time.

a small doc string thingy:

Check a bug spotted at #18086 is fixed::

should use :trac:18086 instead.

Last edited 4 years ago by wuthrich (previous) (diff)

comment:30 Changed 4 years ago by davidloeffler

  • Branch changed from u/davidloeffler/18086-twists-merged to u/davidloeffler/18086-rebased

comment:31 Changed 4 years ago by git

  • Commit changed from d57edf416adaace77e013e71223b1b5bae7d686a to 751bf48f8c2f65fd4a7478e0b22fbe76c6edf3bb

Branch pushed to git repo; I updated commit sha1. New commits:

751bf48Trac 18086: fixed a small Dirichlet character bug

comment:32 Changed 4 years ago by davidloeffler

OK, so it turns out that:

  • the reason I couldn't get the develop branch to compile was that I was using "sage -b" to recompile after switching between branches, which is not robust when there are changes to packages; it seems to be safer to use "make";
  • two of the three changes in the patch I uploaded earlier had already been fixed by other patches of Peter's, which was the cause of the conflict.

I've now uploaded a patch with the one extra change that Peter hadn't already fixed -- minimize_base_ring returns an error when the base is a relative number field, which is relevant if you create a newform in a space with a specified character, so the Hecke eigenvalue field is a relative field with the coefficient field of the character as its base. Now I think we're now good to go.

comment:33 Changed 4 years ago by pbruin

  • Status changed from needs_review to positive_review

I saw the git-related messages come by but didn't have time to look into it. I think the way you solved it is good, although the branch at #18061 is now less tidy than it would have been if you had used merging instead of rebasing. (The situation might have been clearer if I had merged #18479; I don't always find it easy to decide if and when dependencies should be merged into a branch.)

Thanks to both of you for the review!

comment:34 Changed 4 years ago by vbraun

  • Branch changed from u/davidloeffler/18086-rebased to 751bf48f8c2f65fd4a7478e0b22fbe76c6edf3bb
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:35 Changed 4 years ago by davidloeffler

  • Commit 751bf48f8c2f65fd4a7478e0b22fbe76c6edf3bb deleted

See #18672 for a follow-up.

Note: See TracTickets for help on using tickets.