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:

Status badges

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 kedlaya

The short answer here is simple: in hypergeometric_motive.py we have

       if q > 2 ** 31:
            return ValueError("p^f cannot exceed 2^31")

where return should be raise.

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.

comment:2 Changed 2 years ago by kedlaya

  • Branch set to u/kedlaya/error_in_hypergeometric_trace_formula

comment:3 Changed 2 years ago by kedlaya

  • Authors set to Kiran Kedlaya
  • 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:

4334f10Raise correct exception for too large parameters

comment:4 Changed 2 years ago by git

  • Commit changed from 4334f103aaf2634c5bfc18eddb1ee1ab5c2db439 to eb03b0e5b97af048f1562af83537abe4c2e47376

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

eb03b0eAllocate m as defaultdict rather than array

comment:5 Changed 2 years ago by chapoton

  • 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 git

  • Commit changed from eb03b0e5b97af048f1562af83537abe4c2e47376 to a44162166f72f1a74aef86b04cac7aa99fd2e5c2

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

a441621Code cleanup

comment:7 Changed 2 years ago by chapoton

good. Is the import of array still needed in the pxd file ?

comment:8 Changed 2 years ago by git

  • Commit changed from a44162166f72f1a74aef86b04cac7aa99fd2e5c2 to 0940b3f579221645c6a3a2941181961229d37ea6

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

0940b3fMove cimport of array out of hypergeometric_misc.pxd

comment:9 Changed 2 years ago by kedlaya

Now the import can take place in the pyx instead, which I suppose is a bit cleaner.

comment:10 Changed 2 years ago by git

  • Commit changed from 0940b3f579221645c6a3a2941181961229d37ea6 to 3542258fe33f184b33e1963eb94ab87b60271b0e

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

3542258Add error handling to padic_H_value, H_value; utility functions

comment:11 Changed 2 years ago by kedlaya

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 value t (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 and H_value, again with doctests.
  • Add trace as an alias for padic_H_value.
  • Add wild_primes as a method to return the list of wild primes.

comment:12 Changed 2 years ago by chapoton

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 git

  • Commit changed from 3542258fe33f184b33e1963eb94ab87b60271b0e to 295901a638b8e54afabbb7e1bb7ac1b99dd5d226

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

295901aSimplify wild_primes

comment:14 Changed 2 years ago by chapoton

  • 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 vbraun

  • Branch changed from u/kedlaya/error_in_hypergeometric_trace_formula to 295901a638b8e54afabbb7e1bb7ac1b99dd5d226
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.