Major extensions to local components code
loefflerd opened this issue · 37 comments
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
]
Component: modular forms
Keywords: local-components
Author: David Loeffler
Branch/Commit: c165835
Reviewer: David Roe
Issue created by migration from https://trac.sagemath.org/ticket/26635
Description changed:
---
+++
@@ -5,4 +5,14 @@
- 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.
+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, the following example now 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
+]
+```Description changed:
---
+++
@@ -5,7 +5,7 @@
- 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, the following example now works, fulfilling a request from Ariel Pacetti:
+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")Branch pushed to git repo; I updated commit sha1. New commits:
482e00a | Fix non-ascii chars, non-Py3 code etc picked up by patchbot |
I just committed a fix for some minor coding-style violations picked up by the patchbot.
Changed branch from u/davidloeffler/minimal_twist_new to u/roed/minimal_twist_new
Branch pushed to git repo; I updated commit sha1. New commits:
f8160a3 | Fix trivial test failures |
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
]
Nope, merge still failed. I'll try a more recent version of Sage soon.
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).
Branch pushed to git repo; I updated commit sha1. New commits:
0cef4b0 | Merge branch 'u/roed/minimal_twist_new' of git://trac.sagemath.org/sage into t/26635/minimal_twist_new |
Branch pushed to git repo; I updated commit sha1. New commits:
1681b79 | Fix doctest failure |
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!
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?
Branch pushed to git repo; I updated commit sha1. New commits:
c2267ce | Mostly style changes, one potential speedup |
Branch pushed to git repo; I updated commit sha1. New commits:
d0c9d58 | Mark fragile doctest as random |
Branch pushed to git repo; I updated commit sha1. New commits:
7d4dd1b | Some more style fixes |
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!
Reviewer: David Roe
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.
PDF docs don't build
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?
Wait, I found the error: exactly two characters missing in a single docstring in smoothchar.py. Testing, will upload shortly.
Changed branch from u/roed/minimal_twist_new to u/davidloeffler/minimal_twist_new
Right, that should fix it. Given the microscopic size of the change, I presume it is valid to reinstate the positive review.
New commits:
c165835 | Fix broken latex in docstring |
Yep, I agree with the positive review.
Changed branch from u/davidloeffler/minimal_twist_new to c165835