Opened 2 years ago
Closed 2 years ago
#29778 closed defect (fixed)
Error in hypergeometric trace formula
Reported by: | kedlaya | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-9.2 |
Component: | modular forms | Keywords: | hypergeometric trace formula |
Cc: | Merged in: | ||
Authors: | Kiran Kedlaya | Reviewers: | Frédéric Chapoton |
Report Upstream: | N/A | Work issues: | |
Branch: | 295901a (Commits, GitHub, GitLab) | Commit: | 295901a638b8e54afabbb7e1bb7ac1b99dd5d226 |
Dependencies: | Stopgaps: |
Description
Reported to me by Thomas Grubb. (It takes a minute or two to run before returning the error, but that is reasonable given the size of the calculation.)
sage: from sage.modular.hypergeometric_motive import HypergeometricData as HGData sage: H = HGData(alpha_beta=([1/5,2/5,3/5,4/5, 1/5,2/5,3/5,4/5], [1/4,3/4,1/7,2/7,3/7,4/7, 5/7,6/7])) sage: print(H.euler_factor(2, 373)) --------------------------------------------------------------------------- KeyError Traceback (most recent call last) /home/kedlaya/sage/local/lib/python3.7/site-packages/sage/misc/cachefunc.pyx in sage.misc.cachefunc.CachedMethodCaller.__call__ (build/cythonized/sage/misc/cachefunc.c:10139)() 1942 try: -> 1943 return cache[k] 1944 except TypeError: # k is not hashable KeyError: ((2, 373, False), ()) During handling of the above exception, another exception occurred: TypeError Traceback (most recent call last) <ipython-input-9-cb7d337b8f9a> in <module>() ----> 1 print(H.euler_factor(Integer(2), Integer(373))) /home/kedlaya/sage/local/lib/python3.7/site-packages/sage/misc/cachefunc.pyx in sage.misc.cachefunc.CachedMethodCaller.__call__ (build/cythonized/sage/misc/cachefunc.c:10273)() 1946 return cache[k] 1947 except KeyError: -> 1948 w = self._instance_call(*args, **kwds) 1949 cache[k] = w 1950 return w /home/kedlaya/sage/local/lib/python3.7/site-packages/sage/misc/cachefunc.pyx in sage.misc.cachefunc.CachedMethodCaller._instance_call (build/cythonized/sage/misc/cachefunc.c:9758)() 1822 True 1823 """ -> 1824 return self.f(self._instance, *args, **kwds) 1825 1826 cdef fix_args_kwds(self, tuple args, dict kwds): /home/kedlaya/sage/local/lib/python3.7/site-packages/sage/modular/hypergeometric_motive.py in euler_factor(self, t, p, cache_p) 1519 w = self.weight() 1520 sign = self.sign(t, p) -> 1521 return characteristic_polynomial_from_traces(traces, d, p, w, sign) /home/kedlaya/sage/local/lib/python3.7/site-packages/sage/modular/hypergeometric_motive.py in characteristic_polynomial_from_traces(traces, d, q, i, sign) 157 ring = PolynomialRing(ZZ, 'T') 158 --> 159 series = sum(- api * t**(i + 1) / (i + 1) for i, api in enumerate(traces)) 160 series = series.O(d // 2 + 1).exp() 161 coeffs = list(series) /home/kedlaya/sage/local/lib/python3.7/site-packages/sage/modular/hypergeometric_motive.py in <genexpr>(.0) 157 ring = PolynomialRing(ZZ, 'T') 158 --> 159 series = sum(- api * t**(i + 1) / (i + 1) for i, api in enumerate(traces)) 160 series = series.O(d // 2 + 1).exp() 161 coeffs = list(series) TypeError: bad operand type for unary -: 'ValueError'
Change History (15)
comment:1 Changed 2 years ago by
comment:2 Changed 2 years ago by
- Branch set to u/kedlaya/error_in_hypergeometric_trace_formula
comment:3 Changed 2 years ago by
- Commit set to 4334f103aaf2634c5bfc18eddb1ee1ab5c2db439
- Status changed from new to needs_review
This commit fixes the error in padic_H_value
and also propagates the exception-raising to euler_factor
, so that it happens instantly rather than wasting time on computing lower-order coefficients.
New commits:
4334f10 | Raise correct exception for too large parameters
|
comment:4 Changed 2 years ago by
- Commit changed from 4334f103aaf2634c5bfc18eddb1ee1ab5c2db439 to eb03b0e5b97af048f1562af83537abe4c2e47376
Branch pushed to git repo; I updated commit sha1. New commits:
eb03b0e | Allocate m as defaultdict rather than array
|
comment:5 Changed 2 years ago by
- The line
+ ....:
is not needed
- array is no longer used, its import should be removed
+src/sage/modular/hypergeometric_motive.py:64: 'array' imported but unused
- multiple things on one line are no longer kosher:
+src/sage/modular/hypergeometric_motive.py:1101:28: E701 multiple statements on one line (colon) +src/sage/modular/hypergeometric_motive.py:1239:30: E701 multiple statements on one line (colon)
comment:6 Changed 2 years ago by
- Commit changed from eb03b0e5b97af048f1562af83537abe4c2e47376 to a44162166f72f1a74aef86b04cac7aa99fd2e5c2
Branch pushed to git repo; I updated commit sha1. New commits:
a441621 | Code cleanup
|
comment:7 Changed 2 years ago by
good. Is the import of array still needed in the pxd file ?
comment:8 Changed 2 years ago by
- Commit changed from a44162166f72f1a74aef86b04cac7aa99fd2e5c2 to 0940b3f579221645c6a3a2941181961229d37ea6
Branch pushed to git repo; I updated commit sha1. New commits:
0940b3f | Move cimport of array out of hypergeometric_misc.pxd
|
comment:9 Changed 2 years ago by
Now the import can take place in the pyx instead, which I suppose is a bit cleaner.
comment:10 Changed 2 years ago by
- Commit changed from 0940b3f579221645c6a3a2941181961229d37ea6 to 3542258fe33f184b33e1963eb94ab87b60271b0e
Branch pushed to git repo; I updated commit sha1. New commits:
3542258 | Add error handling to padic_H_value, H_value; utility functions
|
comment:11 Changed 2 years ago by
Since this code is getting a lot of use at this week's (virtual) ICERM workshop, Edgar Costa suggested some easy improvements that I bundled into a commit.
- Improve the error handling in
euler_factor
so that it does not factor the parameter valuet
(as this could be slow in some use cases). - Add doctests for said error handling. (I also modified the doctest I added earlier on this ticket, so that it confirms not just the exception type but also the message.)
- Handle errors the same way in
padic_H_value
andH_value
, again with doctests. - Add
trace
as an alias forpadic_H_value
. - Add
wild_primes
as a method to return the list of wild primes.
comment:12 Changed 2 years ago by
This
+ return sorted(list(set([p for n in gamma.keys() for (p, _) in n.factor()])))
could be simplified to
+ return sorted(set(p for n in gamma for (p, _) in n.factor()))
comment:13 Changed 2 years ago by
- Commit changed from 3542258fe33f184b33e1963eb94ab87b60271b0e to 295901a638b8e54afabbb7e1bb7ac1b99dd5d226
Branch pushed to git repo; I updated commit sha1. New commits:
295901a | Simplify wild_primes
|
comment:14 Changed 2 years ago by
- Reviewers set to Frédéric Chapoton
- Status changed from needs_review to positive_review
not quite as simplified as it could be, but ok
I am setting to positive
comment:15 Changed 2 years ago by
- Branch changed from u/kedlaya/error_in_hypergeometric_trace_formula to 295901a638b8e54afabbb7e1bb7ac1b99dd5d226
- Resolution set to fixed
- Status changed from positive_review to closed
The short answer here is simple: in
hypergeometric_motive.py
we havewhere
return
should beraise
.A more satisfying answer would be to relax the restriction on
q
, which was imposed to avoid integer overflows in the Cython code. I had thought this was beyond the range where these calculations would still be practical, but I appear to have been mistaken.