Opened 4 years ago

Closed 12 months ago

#26635 closed enhancement (fixed)

Major extensions to local components code

Reported by: davidloeffler Owned by:
Priority: major Milestone: sage-9.4
Component: modular forms Keywords: local-components
Cc: Merged in:
Authors: David Loeffler Reviewers: David Roe
Report Upstream: N/A Work issues:
Branch: c165835 (Commits, GitHub, GitLab) Commit: c165835c65b91c1badecd3aae58fa1b031f74a2b
Dependencies: Stopgaps:

Status badges

Description (last modified by davidloeffler)

Some while ago, I contributed to Sage some code for computing the local component (at a given prime) of the automorphic representation attached to a modular form, based on algorithms from a paper I wrote with Jared Weinstein.

The existing implementation is incomplete in several ways:

  • it throws an error if the given modular form doesn't have minimal level at p among its character twists;
  • if the local component is supercuspidal, induced from a character of a ramified quadratic extension, then it doesn't completely compute the character, it just returns a string saying which extension it comes from;
  • in supercuspidal 2-adic cases where the level is divisible by 64, it fails because a certain quotient group that comes up in the computation isn't cyclic.

I have now reworked the code to fill in all of the above gaps. It should now work for all newforms and all primes, with the exception of certain nasty 2-adic cases where no nice parametrisation exists. In particular, with the new code the following example works, fulfilling a request from Ariel Pacetti:

sage: E = EllipticCurve("575c1")
sage: Pi = E.newform().local_component(5)
sage: Pi.characters()
[
Character of Q_5*, of level 1, mapping 2 |--> 1/2*a, 5 |--> 1/2*a + 2,
Character of Q_5*, of level 1, mapping 2 |--> -1/2*a, 5 |--> -1/2*a + 2
]

Change History (28)

comment:1 Changed 4 years ago by davidloeffler

  • Branch set to u/davidloeffler/minimal_twist_new

comment:2 Changed 4 years ago by davidloeffler

  • Commit set to 37ce61fee64a96db57abeaf5a41ac24ed3c00e98
  • Status changed from new to needs_review

New commits:

1b55638Add a minimal twist function for newform objects
37ce61fLocal components: handle many previously-unimplemented cases

comment:3 Changed 4 years ago by davidloeffler

  • Description modified (diff)

comment:4 Changed 4 years ago by davidloeffler

  • Description modified (diff)

comment:5 Changed 4 years ago by git

  • Commit changed from 37ce61fee64a96db57abeaf5a41ac24ed3c00e98 to 482e00a6c12d5d87fca8ac23aa1daf5edea3f9b9

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

482e00aFix non-ascii chars, non-Py3 code etc picked up by patchbot

comment:6 Changed 4 years ago by davidloeffler

I just committed a fix for some minor coding-style violations picked up by the patchbot.

comment:7 Changed 13 months ago by roed

  • Branch changed from u/davidloeffler/minimal_twist_new to u/roed/minimal_twist_new

comment:8 Changed 13 months ago by git

  • Commit changed from 482e00a6c12d5d87fca8ac23aa1daf5edea3f9b9 to f8160a3c07931345e8c1a8ee33740d75f9c319d8

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

f8160a3Fix trivial test failures

comment:9 Changed 13 months ago by roed

I hopefully fixed the merge failure and a couple of deprecation warnings. There seems to be one actual doctest failure:

File "src/sage/modular/local_comp/local_comp.py", line 704, in sage.modular.local_comp.local_comp.PrimitiveSupercuspidal.characters
Failed example:
    Newforms(DirichletGroup(64, QQ).1, 2, names='a')[0].local_component(2).characters() # long time
Expected:
    [
    Character of unramified extension Q_2(s)* (s^2 + s + 1 = 0), of level 3, mapping s |--> 1, 2*s + 1 |--> -1/4*a0, 4*s + 1 |--> -1, -1 |--> 1, 2 |--> 1,
    Character of unramified extension Q_2(s)* (s^2 + s + 1 = 0), of level 3, mapping s |--> 1, 2*s + 1 |--> -1/4*a0, 4*s + 1 |--> 1, -1 |--> 1, 2 |--> 1
    ]
Got:
    [
    Character of unramified extension Q_2(s)* (s^2 + s + 1 = 0), of level 3, mapping s |--> 1, 2*s + 1 |--> 1/2*a0, 4*s + 1 |--> 1, -1 |--> 1, 2 |--> 1,
    Character of unramified extension Q_2(s)* (s^2 + s + 1 = 0), of level 3, mapping s |--> 1, 2*s + 1 |--> 1/2*a0, 4*s + 1 |--> -1, -1 |--> 1, 2 |--> 1
    ]

comment:10 Changed 13 months ago by roed

Nope, merge still failed. I'll try a more recent version of Sage soon.

comment:11 Changed 13 months ago by davidloeffler

Nice to see some movement on this at long last, thanks for taking a look at it! The doctest failure looks harmless to me, my guess is that Sage has changed its mind about which generator to use for the coeff field of the newform. Let me know if you have trouble getting the merge to work, I can have a go myself (if I still remember how).

comment:12 Changed 13 months ago by git

  • Commit changed from f8160a3c07931345e8c1a8ee33740d75f9c319d8 to 0cef4b09472bc5f69e21eed99ec46f9b36a913e3

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

0cef4b0Merge branch 'u/roed/minimal_twist_new' of git://trac.sagemath.org/sage into t/26635/minimal_twist_new

comment:13 Changed 13 months ago by git

  • Commit changed from 0cef4b09472bc5f69e21eed99ec46f9b36a913e3 to 1681b79a2a889c9a9d21eed4d4eea406ed06196c

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

1681b79Fix doctest failure

comment:14 Changed 13 months ago by roed

I got the merge to work (and have adjusted the doctest to what's currently returned). I'll try to read through the code soon, and let you know if I have questions!

comment:15 Changed 13 months ago by davidloeffler

I checked about the failing long doctest. It is indeed unrelated to the code at hand, and arises because sage is using a random algorithm to choose a generator of the coeff field of the newform. Shall we just flag it as # random?

comment:16 Changed 13 months ago by git

  • Commit changed from 1681b79a2a889c9a9d21eed4d4eea406ed06196c to c2267ce939fdf1ffdfe0908025e8b3253487fe74

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

c2267ceMostly style changes, one potential speedup

comment:17 Changed 13 months ago by git

  • Commit changed from c2267ce939fdf1ffdfe0908025e8b3253487fe74 to d0c9d58f3323d86c5344b57fcd74f469ecef78d9

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

d0c9d58Mark fragile doctest as random

comment:18 Changed 13 months ago by git

  • Commit changed from d0c9d58f3323d86c5344b57fcd74f469ecef78d9 to 7d4dd1bc2c9899b2c1261676f858e1eae9916d7a

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

7d4dd1bSome more style fixes

comment:19 Changed 13 months ago by roed

  • Reviewers set to David Roe
  • Status changed from needs_review to positive_review

I read through the code and it looks good. I made various minor changes to better conform to Sage's style guide. The one substantive change I made was to move the line t = self.discrete_log(n, p) outside a loop.

I'm going to give this a positive review now, but we should keep an eye on the patchbot results. I'm also hopeful that I can help with improving speed eventually using #26849 (which still needs work). Finally, what's the theoretical status on the missing 2-adic cases?

I'm sorry this took so long to get reviewed!

comment:20 Changed 13 months ago by davidloeffler

Thanks for the review!

Your ticket for p-adic unit groups looks interesting & obviously it would be nice to unify it with the local-component code; but I doubt it would lead to much of a speed increase. In local-component computations, the computation time is overwhelmingly dominated by computing modular-symbol spaces and Hecke operators on them. The smoothchar.py stuff is just there to give a user-friendly way of interacting with the results of the modsym computation, it is computationally trivial in itself.

comment:21 Changed 13 months ago by chapoton

  • Milestone changed from sage-8.5 to sage-9.4

comment:22 follow-up: Changed 13 months ago by vbraun

  • Status changed from positive_review to needs_work

PDF docs don't build

comment:23 in reply to: ↑ 22 Changed 13 months ago by davidloeffler

Replying to vbraun:

PDF docs don't build

Volker: I cannot reproduce this failure on my machine (since pdf doc building doesn't work on my system for unrelated reasons). Can you post the relevant log file?

comment:24 Changed 13 months ago by davidloeffler

Wait, I found the error: exactly two characters missing in a single docstring in smoothchar.py. Testing, will upload shortly.

comment:25 Changed 13 months ago by davidloeffler

  • Branch changed from u/roed/minimal_twist_new to u/davidloeffler/minimal_twist_new

comment:26 Changed 13 months ago by davidloeffler

  • Commit changed from 7d4dd1bc2c9899b2c1261676f858e1eae9916d7a to c165835c65b91c1badecd3aae58fa1b031f74a2b
  • Status changed from needs_work to positive_review

Right, that should fix it. Given the microscopic size of the change, I presume it is valid to reinstate the positive review.


New commits:

c165835Fix broken latex in docstring

comment:27 Changed 13 months ago by roed

Yep, I agree with the positive review.

comment:28 Changed 12 months ago by vbraun

  • Branch changed from u/davidloeffler/minimal_twist_new to c165835c65b91c1badecd3aae58fa1b031f74a2b
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.