#23671 closed enhancement (fixed)

hypergeometric motives

Reported by: chapoton Owned by:
Priority: major Milestone: sage-8.1
Component: modular forms Keywords: hypergeometric motives, Euler factors, sd91
Cc: kedlaya, jvoight, tornaria Merged in:
Authors: Frédéric Chapoton, Kiran Kedlaya Reviewers: David Roe
Report Upstream: N/A Work issues:
Branch: d8c961b (Commits) Commit: d8c961bfe6d67c265a5c615557ad7744cdf42e20
Dependencies: Stopgaps:

Description (last modified by chapoton)

For the moment, a first sketch.

The computation of Euler factors (via traces of Frobenius) is only for good primes.

Change History (102)

comment:1 Changed 22 months ago by chapoton

  • Branch set to u/chapoton/23671
  • Commit set to 8ce1dc8c6e40300c1929d814048d79e19e146745
  • Dependencies set to #23585
  • Status changed from new to needs_info

New commits:

8ce1dc8first sketch of implementation of hypergeometric motives

comment:2 Changed 22 months ago by git

  • Commit changed from 8ce1dc8c6e40300c1929d814048d79e19e146745 to 9a281db9be4ca4bfda004aff12ea1610cfb04135

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

60e138fsome details
9a281dbdetails

comment:3 Changed 22 months ago by git

  • Commit changed from 9a281db9be4ca4bfda004aff12ea1610cfb04135 to c123aeae45c2e428d49d9f6a2a60adaf51cd6053

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

e0a0462removing hold keyword (deprecated in matplotlib 2.0)
6ddffbbMerge branch 'u/chapoton/23671' in 8.1.b3
c123aeatrac 23671 adaptation

comment:4 Changed 22 months ago by git

  • Commit changed from c123aeae45c2e428d49d9f6a2a60adaf51cd6053 to b8428bc7a8f27ad680497fb7c8c0b41bd1fd20cb

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

dcea249first sketch of implementation of hypergeometric motives
da411ebsome details
d6c66dcdetails
b8428bctrac 23671 adaptation

comment:5 Changed 22 months ago by git

  • Commit changed from b8428bc7a8f27ad680497fb7c8c0b41bd1fd20cb to 09bf0d0c94747950f312d28ba79845b161c8eaa1

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

09bf0d0trac 23671 insertion in the doc

comment:6 Changed 22 months ago by cremona

I'm alerting the LMFDB people who work on hypergeometric motives to the existence of this code.

comment:7 Changed 22 months ago by chapoton

Thanks. Maybe they can help me to fix the lurking bug.

comment:8 Changed 22 months ago by git

  • Commit changed from 09bf0d0c94747950f312d28ba79845b161c8eaa1 to bec14e0c19e91137d0d5fbec9e821d9a274c29c5

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

bec14e0trac 23671 one detail and more doctests

comment:9 Changed 22 months ago by kedlaya

  • Cc kedlaya added

comment:10 Changed 22 months ago by git

  • Commit changed from bec14e0c19e91137d0d5fbec9e821d9a274c29c5 to b2bba5d0227ff70175745ccda638add7a7f629cf

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

b2bba5dtrac 23585 many details

comment:11 Changed 22 months ago by git

  • Commit changed from b2bba5d0227ff70175745ccda638add7a7f629cf to 1005e0aff6f1d873b21c7b5882a1557612f09b56

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

1005e0atrac 23671 detail

comment:12 Changed 22 months ago by git

  • Commit changed from 1005e0aff6f1d873b21c7b5882a1557612f09b56 to b40dd7d1db23f975baf054802d61f5d34b1f5164

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

b40dd7dtrac 23671 method swap_alpha_beta

comment:13 Changed 22 months ago by git

  • Commit changed from b40dd7d1db23f975baf054802d61f5d34b1f5164 to 6dc9bc1b4b8b94b90c6c238c957378072f5c45f1

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

6dc9bc1trac 23671 more helpful message

comment:14 Changed 22 months ago by git

  • Commit changed from 6dc9bc1b4b8b94b90c6c238c957378072f5c45f1 to f4190ab33e2802adefd6f078153dbeba9b99d672

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

f4190abtrac 23671 detail (comment)

comment:15 Changed 22 months ago by git

  • Commit changed from f4190ab33e2802adefd6f078153dbeba9b99d672 to 7d8590c6b9e2f260997c939c8f624b6e5b0a23cd

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

7d8590ccare for doc

comment:16 Changed 22 months ago by git

  • Commit changed from 7d8590c6b9e2f260997c939c8f624b6e5b0a23cd to 5f1be177550319a13402d928c2605facd4e78c64

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

5f1be17details

comment:17 Changed 22 months ago by kedlaya

Is Hyp a temporary name? Magma's analogue is HypergeometricData, which would be reasonable to use here also.

comment:18 Changed 22 months ago by chapoton

yes, indeed. But as long as the euler_factor method is not working correctly, all this has little value..

comment:19 Changed 22 months ago by kedlaya

Is this a correct illustration of the issue with Gauss sums?

sage: H = Hyp(gamma_list=[-6,-1,4,3])
sage: t = 189/125
sage: H.H_value(19,1,t)
0
sage: H.padic_H_value(19,1,t) # should give the same answer?
-3

comment:20 Changed 22 months ago by chapoton

Yes, this is typical. Either our p-adic Gauss sum has a problem, or my code has a problem. I wrote very parallel code for H_value and padic_H_value, but this does not prove that they are both correct. The H_value method seems to be working rather well, compared to the literature.

comment:21 Changed 22 months ago by kedlaya

I don't see any problem with the p-adic Gauss sums. Here is a test:

sage: def f(z,p):
....:    return z - (z^p-1)/(p*z^(p-1))

sage: from sage.arith.misc import gauss_sum
sage: from sage.rings.padics.misc import gauss_sum as padic_gauss_sum

sage: p = 19
sage: R = padic_gauss_sum(1, p, 1, 10).parent()
sage: z = 1 - R.gen()
sage: for i in range(10): z = f(z,p)
sage: z *= R.teichmuller(GF(p).multiplicative_generator())
sage: S.<a> = CyclotomicField(p*(p-1))

sage: for i in range(p-1):
....:    t = gauss_sum(S.zeta(p-1)^i, GF(p)).polynomial()(z)
....:    u = padic_gauss_sum(-i, p, 1, 10)
....:    print (t - u).is_zero()

returns all True.

I'm guessing the issue is the normalization of the comparison between abstract and p-adic roots of unity, which is critical to the Gauss-Koblitz formula. I haven't traced this fully, but it looks like in the lines:

                    prod(padic_gauss_sum(-xp * r, p, f, prec)
                         for xp in gamma_pos) /
                    prod(padic_gauss_sum(-xq * r, p, f, prec)
                         for xq in gamma_neg) *

if one changes -xp * r to xp * r and -xq * r to xq * r, then the most egregious doctest failures in euler_factor go away. (There remain cases where there is an unresolved sign ambiguity, which I need to stare at some more.)

Last edited 22 months ago by kedlaya (previous) (diff)

comment:22 follow-up: Changed 22 months ago by kedlaya

Regarding the sign ambiguity: according to the Magma documentation, they are using the local functional equation to expedite the computation. That suggests that they have a formula for the sign of the functional equation, but I'm not sure where that is writen down.

In other news, I think the statement of the Gross-Koblitz formula in the docstring of the p-adic Gauss sum is missing an overall minus sign, which is in Robert's paper and also in the code.

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

Replying to kedlaya:

Regarding the sign ambiguity: according to the Magma documentation, they are using the local functional equation to expedite the computation. That suggests that they have a formula for the sign of the functional equation, but I'm not sure where that is writen down.

According to Mark Watkins, Magma uses the formulas given in 11.1.1 of http://magma.maths.usyd.edu.au/~watkins/papers/known.pdf for the sign of the local functional equation at good primes. (This is for even weight; for odd weight the sign is always +1.)

Last edited 22 months ago by kedlaya (previous) (diff)

comment:24 Changed 22 months ago by chapoton

Great ! Thank you for investigating. I will now correct that.

comment:25 Changed 22 months ago by git

  • Commit changed from 5f1be177550319a13402d928c2605facd4e78c64 to 445de286f6f8c64eaf0e9debd53440fc800c60bc

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

4daf486first step of fixing euler_factor
445de28tweak one doctest

comment:26 Changed 22 months ago by git

  • Commit changed from 445de286f6f8c64eaf0e9debd53440fc800c60bc to 9012cc56cb8eeb9ff1a04acb157e45f24879e607

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

9012cc5first sketch of implementation of hypergeometric motives

comment:27 Changed 22 months ago by chapoton

Humm, it seems that this does not fix all the doctests of padic_H_value. Some of the doctests for (p,2,t) for the first HGM example are still wrong (by a factor of p).

comment:28 Changed 22 months ago by git

  • Commit changed from 9012cc56cb8eeb9ff1a04acb157e45f24879e607 to fd2f2efad4f61132a471bc0161e0701554a19b23

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

fd2f2efremove comment

comment:29 Changed 22 months ago by git

  • Commit changed from fd2f2efad4f61132a471bc0161e0701554a19b23 to e1b5570f8384ef3f993f603d1c818bc8aa3c5666

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

e1b5570trac 23671 first step for ambiguity-resolving

comment:30 Changed 22 months ago by kedlaya

Suggestion: rather than defining Hyp as as alias for HypergeometricMotive, it might be safer to just change import Hyp to import HypergeometricMotive as Hyp in the doctests. (The abbreviation Hyp could be misinterpreted in various ways, so I'm not sold on the wisdom of putting it into the global namespace.)

comment:31 Changed 22 months ago by chapoton

yes, for sure. But much more importantly we need to (1) fix the computation of padic_H_value and (2) resolve the ambiguity of Euler factors. I have no idea for (1) and plan to work on (2).

EDIT: And so far, nothing is added to the global namespace..

Last edited 22 months ago by chapoton (previous) (diff)

comment:32 Changed 22 months ago by git

  • Commit changed from e1b5570f8384ef3f993f603d1c818bc8aa3c5666 to 8e35f9a19b124179845183b012f00d2cdf5eebae

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

8e35f9atrac 23671 not setting Hyp as an alias

comment:33 Changed 22 months ago by git

  • Commit changed from 8e35f9a19b124179845183b012f00d2cdf5eebae to e76c51d92a4f2dc99ed64298d70d6eeb26a71b7b

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

e76c51dfirst sketch of implementation of hypergeometric motives

comment:34 Changed 22 months ago by git

  • Commit changed from e76c51d92a4f2dc99ed64298d70d6eeb26a71b7b to 140dc5fb2662b90ca8a7d5c3f8eb6d3b85c581ef

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

140dc5f+ one new method

comment:35 Changed 22 months ago by kedlaya

I am going to take a look at the Magma code sometime soon to see if it sheds some light on the remaining doctest failures...

comment:36 Changed 22 months ago by git

  • Commit changed from 140dc5fb2662b90ca8a7d5c3f8eb6d3b85c581ef to 585257b6ab549a6de522f413067d14effbd6453e

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

585257btrac 23671 fixing Hyp calls

comment:37 Changed 22 months ago by chapoton

My only reference for the formula used in padic_H_value is page 4 of http://magma.maths.usyd.edu.au/~watkins/papers/HGM-chapter.pdf

comment:38 Changed 22 months ago by git

  • Commit changed from 585257b6ab549a6de522f413067d14effbd6453e to a6ac27e4fc508cf464bd7baeda6ec1a5ea09d3ff

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

f18fc10trac 23671 work on H_value
a6ac27ework on padic_H_value

comment:39 Changed 22 months ago by git

  • Commit changed from a6ac27e4fc508cf464bd7baeda6ec1a5ea09d3ff to babe4dbba281bd25c5d3341509d9c94e48e4f912

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

babe4dbforgotten in change

comment:40 Changed 22 months ago by git

  • Commit changed from babe4dbba281bd25c5d3341509d9c94e48e4f912 to 916ec11f240223c46516c83902c6ea162036c39b

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

916ec11better handling of ComplexField H_value

comment:41 Changed 22 months ago by kedlaya

In padic_H_value, you might want to change

        p_ring = Zp(p)

to

        p_ring = Zp(p,prec=prec)

as otherwise the value of prec doesn't really make a difference in the internal computation. (This shouldn't matter to any of the current doctests, though.)

comment:42 Changed 22 months ago by git

  • Commit changed from 916ec11f240223c46516c83902c6ea162036c39b to 9d4e8398d3fcf633d59d58193320749108673aac

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

9d4e839trac 23671 detail

comment:43 follow-up: Changed 22 months ago by kedlaya

I think I found the remaining issue with padic_H_value. The problem is actually in convert_to_Z:

    p_ring = x.parent().base_ring()
    p = p_ring.prime()
    L = x.list()
    print(p_ring, p, L)
    return ZZ.sum(L[(p - 1) * i] * (-p) ** i
                  for i in range(1 + len(L) // (p - 1)))

The trouble is that x.list() behaves differently for an element of a p-adic ring (in which case it starts from 0) and an element of a p-adic field (in which case it starts from the valuation).

There are various ways to fix this. One would be to replace L = x.list() with:

    # Pass from field to ring so that list() starts from p^0.
    y = x.parent().integer_ring()(x)
    L = y.list()

to force the ring behavior.

comment:44 follow-up: Changed 22 months ago by kedlaya

In other news, there are some mathematical errors in the docstring for the p-adic Gauss sum. Maybe we should go ahead and fix them on this ticket:

    Let `p` be a prime, `f` a positive integer, `q=p^f`, and `\pi` be
    the unique root of `f(x) = x^{p-1}+p` congruent to `\zeta_p-1` modulo
    `(\zeta_p-1)^2`. Let `0\leq a < q-1`. Then the Gross-Koblitz formula 
    gives us the value of the Gauss sum `g_q(a)` as a product of p-adic 
    Gamma functions as follows:

    .. MATH::

        g_q(a) = -\pi^s \prod_{0\leq i < f} \Gamma_p(a^{(i)}/(q-1)),

    where `s` is the sum of the digits of `a` in base `p` and the
    `a^{(i)}` have `p`-adic expansions obtained from cyclic
    permutations of that of `a`.

comment:45 Changed 22 months ago by git

  • Commit changed from 9d4e8398d3fcf633d59d58193320749108673aac to 4f23442f652455ae381c7f923574a629746959be

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

4f23442trac 23671 fixing convert_to_Z

comment:46 Changed 22 months ago by git

  • Commit changed from 4f23442f652455ae381c7f923574a629746959be to dee34a4bd6fb9da44ddef80d92107019ea3b5592

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

dee34a4trac 23671 fixing doc of padic Gauss sums

comment:47 in reply to: ↑ 43 Changed 22 months ago by chapoton

Great ! Thanks a lot for finding that ! Now we can at last compute Euler factors for good primes. All tests pass.

Then we have 2 options. Trying to get his in sage in its present state, and enhance later. Or trying to go further before put this inside sage. I am not really planning to spend a lot more time on that, so I would prefer the first option.

Replying to kedlaya:

I think I found the remaining issue with padic_H_value. The problem is actually in convert_to_Z:

    p_ring = x.parent().base_ring()
    p = p_ring.prime()
    L = x.list()
    print(p_ring, p, L)
    return ZZ.sum(L[(p - 1) * i] * (-p) ** i
                  for i in range(1 + len(L) // (p - 1)))

The trouble is that x.list() behaves differently for an element of a p-adic ring (in which case it starts from 0) and an element of a p-adic field (in which case it starts from the valuation).

There are various ways to fix this. One would be to replace L = x.list() with:

    # Pass from field to ring so that list() starts from p^0.
    y = x.parent().integer_ring()(x)
    L = y.list()

to force the ring behavior.

comment:48 Changed 22 months ago by kedlaya

Definitely get this feature set into Sage first, then create separate tickets for additional features (like the Euler factors at bad reduction primes). This progress might serve as the basis for organizing a Sage Days on HGM sooner or later.

As soon as this is ready to go to needs_review I will start looking more critically (particularly at the documentation, which so far I have mostly ignored).

comment:49 Changed 22 months ago by chapoton

  • Description modified (diff)

comment:50 Changed 22 months ago by git

  • Commit changed from dee34a4bd6fb9da44ddef80d92107019ea3b5592 to 4856761ac805418cc7620ba688361f8e6e4cb206

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

4856761first sketch of implementation of hypergeometric motives

comment:51 Changed 22 months ago by git

  • Commit changed from 4856761ac805418cc7620ba688361f8e6e4cb206 to d5aae64066afbcb4edbc35d867b1122da8413c39

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

d5aae64trac 23671 adding tests

comment:52 in reply to: ↑ 44 Changed 22 months ago by kedlaya

Replying to kedlaya:

In other news, there are some mathematical errors in the docstring for the p-adic Gauss sum. Maybe we should go ahead and fix them on this ticket:

That change has been made on #23780. The primary purpose of that ticket is to modify the p-adic Gauss sum computation to allow the caller to avoid instantiating the ramified extension of Q_p. With that, one can modify padic_H_value to also avoid constructing the ramified extension, which in principle saves a factor of p in complexity.

comment:53 Changed 22 months ago by kedlaya

  • Branch changed from u/chapoton/23671 to u/kedlaya/23671

comment:54 Changed 22 months ago by kedlaya

  • Authors changed from Frédéric Chapoton to Frédéric Chapoton, Kiran Kedlaya
  • Commit changed from d5aae64066afbcb4edbc35d867b1122da8413c39 to db3363b4a150d7345e6ac1a8b5b72a426ccbf297
  • Dependencies changed from #23585 to #23585, #23780
  • Keywords hypergeometric motives Euler factors added

This commit implements my suggestion to do the computation entirely in Z_p by using the Gauss-Koblitz formula in factored form. Even saving a factor of p, it still seems to be much slower than Magma; not sure whether there is a single reason for that.


Last 10 new commits:

f5be269trac 23585 doc detail
c1ca8datrac 23585 allow to choose a character by its value on the mult. generator.
adeaa96trac 23585 having finite_field as input
aea2daetrac 23585 more doc (definition of Gauss sum)
67145d2trac 23585 doc details, fine tuning
6fb031dMerge branch 't/23585/gauss_sum_v2' into t/23671/23671
826ff2eModify p-adic Gauss sum to provide a factored option
50cd8bfFixed typo in doctest
eeecd24Merge branch 't/23780/modify_p_adic_gauss_sum_to_allow_working_directly_in_q_p' into t/23671/23671
db3363bEliminate use of ramified extension

comment:55 Changed 22 months ago by kedlaya

Fernando Rodriguez Villegas points out that Magma has built in a "wrong" (i.e., not consistent with standard references in the literature) sign convention, with the effect that the parameter values t and 1/t should be swapped. It's probably too late for Magma to change this, but it's not too late for Sage...

comment:56 Changed 22 months ago by git

  • Commit changed from db3363b4a150d7345e6ac1a8b5b72a426ccbf297 to cc19212f92fb3e1b005618e11016bf5683785a15

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

8271a6bfirst sketch of implementation of hypergeometric motives
a302f4dtrac 23671 adding tests
83aecd9Modify p-adic Gauss sum to provide a factored option
700a0b7Fixed typo in doctest
d57a3ebEliminate use of ramified extension
10377d0Interchange t for 1/t
1b0c787Merge branch 'u/kedlaya/23671' of git://trac.sagemath.org/sage into t/23671/23671
cc19212General cleanup

comment:57 Changed 22 months ago by git

  • Commit changed from cc19212f92fb3e1b005618e11016bf5683785a15 to 9f5dd9dfa9ba042532b65125cd56c520530ef25e

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

9f5dd9dAdd zigzag function, change class name to HypergeometricData

comment:58 Changed 22 months ago by kedlaya

I changed the class name from HypergeometricMotive to HypergeometricData to match Magma. This is also conceptually better: this class represents a collection of combinatorial data corresponding to not a single motive, but a one-parameter family of motives indexed by t.

comment:59 Changed 22 months ago by chapoton

Just to say that I agree with the name change. We could also have another class for objects where t has been chosen.

And I also agree with the switch t <-> 1/t.

Thanks for working on all that !

comment:60 Changed 22 months ago by git

  • Commit changed from 9f5dd9dfa9ba042532b65125cd56c520530ef25e to 60c038b4ae556a0bd9b426f29dd1156f0d8b3507

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

60c038bImplement formula for sign of the functional equation

comment:61 Changed 22 months ago by kedlaya

I implemented the explicit formula for the sign of the functional equation which I learned from Mark Watkins; this means that for degree d, no more than d//2 traces are ever needed. (In particular, for d odd this always saves one trace over the old method.)

In order to confirm that I did this correctly, I put in some more doctests. In particular, the doctests for euler_factor cover all three cases of the functional equation (odd weight; even weight and odd degree; even weight and even degree).

I agree that later, we may want to add a class HypergeometricMotive which includes the choice of t, and which would have the L-series as an attribute. Magma has a class for L-series themselves, which has attributes like checking the functional equation and computing special values. However, that is an issue for a later ticket (as is adding the Euler factors for tame and wild primes).

This is actually looking pretty good now, to the extent that I'm planning to do a demonstration of this in a talk I'm giving at ICTP (Trieste) tomorrow (which was my motivation for making all these changes at this particular moment). Is it time for a review?

comment:62 Changed 22 months ago by kedlaya

To answer my own question: no, this is not yet ready for review because I can still produce cases where it fails to agree with Magma (with some help from David Roberts). I'm guessing this has to do with the handling of the values 0 and 1 in the parameter lists; let me see if I can track it down.

comment:63 Changed 22 months ago by git

  • Commit changed from 60c038b4ae556a0bd9b426f29dd1156f0d8b3507 to e18e633f49506a264e427025775423891d85fcbd

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

e18e633Correct answers when 0 is in alpha

comment:64 Changed 22 months ago by git

  • Commit changed from e18e633f49506a264e427025775423891d85fcbd to baf8ae905722d35a513a971e757b28c75e2b26da

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

baf8ae9Modify one doctest to distinguish t from 1/t

comment:65 Changed 22 months ago by kedlaya

  • Cc jvoight added

The formulas transcribed by Watkins apparently presume that 0 does not occur in alpha. Since alpha and beta must be disjoint, this case is most easily handled as Magma does it, by swapping alpha with beta and t with 1/t.

This resolves the issue I raised at comment 62, so now I am back to wondering whether this is ready for review.

comment:66 Changed 22 months ago by kedlaya

  • Cc tornaria added

comment:67 Changed 21 months ago by tornaria

There are some errors in the documentation that have already been fixed in #23780.

Can you rebase this on top of it?

comment:68 Changed 21 months ago by git

  • Commit changed from baf8ae905722d35a513a971e757b28c75e2b26da to 65db2d1e386c734afe6e7a9a580b7854e260d4fe

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

6cc03a1Correct docstring formatting, handling of r=0 case
ef1626fMerge branch 'develop' into t/23780/modify_p_adic_gauss_sum_to_allow_working_directly_in_q_p
45da482trac 23780 some doc formatting details
65db2d1Merge branch 't/23780/modify_p_adic_gauss_sum_to_allow_working_directly_in_q_p' into t/23671/23671

comment:69 Changed 21 months ago by chapoton

  • Branch changed from u/kedlaya/23671 to u/chapoton/23671
  • Commit changed from 65db2d1e386c734afe6e7a9a580b7854e260d4fe to 9dde54a6b42f859c35cbd9945c1da426cef5146a
  • Dependencies #23585, #23780 deleted

I have made a cosmetic cleanup of the code, to conform to the pep8 standard.

Maybe we whould still add more documentation everywhere ?


New commits:

9dde54atrac 23671 pep8 cleanup

comment:70 Changed 21 months ago by git

  • Commit changed from 9dde54a6b42f859c35cbd9945c1da426cef5146a to 28e95f4586177ff08cc06e58dd69c4d5ce17c4df

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

3e247faMerge branch 'u/chapoton/23671' in 8.1.b5
28e95f4trac 23671 clean-up of references

comment:71 Changed 21 months ago by git

  • Commit changed from 28e95f4586177ff08cc06e58dd69c4d5ce17c4df to df64dbef1f40c631e6b7a75a481708c0e14d0589

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

df64dbetrac 23671 more care for references

comment:72 Changed 21 months ago by kedlaya

  • Branch changed from u/chapoton/23671 to u/kedlaya/23671

comment:73 Changed 21 months ago by git

  • Commit changed from df64dbef1f40c631e6b7a75a481708c0e14d0589 to cc53131b6ff960faee21aafdfe35175facf2ce10

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

4179f4bMore minor edits
cc53131Typo fix

comment:74 Changed 21 months ago by chapoton

  • Branch changed from u/kedlaya/23671 to u/chapoton/23671
  • Commit changed from cc53131b6ff960faee21aafdfe35175facf2ce10 to 3cdb07c4ab1b9d5274d68d224209ce0a180a1b64

I fixed the wrong syntax in the new raise statement (it was not py3 compatible).


New commits:

3cdb07ctrac 23671 fix wrong syntax for raise

comment:75 Changed 21 months ago by kedlaya

Are we ready for review at this point?

comment:76 Changed 21 months ago by chapoton

I have some small concerns :

1) we should move the references to the master-reference-file,

2) I would prefer to have more explanations in the doc everywhere.

But I think it can be set to "needs_review" once 1) is done.

I am going to do 1) right now.

comment:77 Changed 21 months ago by git

  • Commit changed from 3cdb07c4ab1b9d5274d68d224209ce0a180a1b64 to 1d1039f8b082cb4e36d7ef0e34f6860a5385b9eb

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

cd7c290Merge branch 'u/chapoton/23671' in 8.1.b6
1d1039ftrac 23671 moving refs to the huge list of refs

comment:78 Changed 21 months ago by kedlaya

  • Keywords sd91 added

comment:79 Changed 21 months ago by kedlaya

  • Milestone changed from sage-8.1 to sage-8.2
  • Status changed from needs_info to needs_review

While more documentation would be nice, there is overall a big lack of documentation in this whole area of mathematics; anyway I would propose to get some reviewer feedback on that front.

I don't see any causal doctest failures in the patchbot logs.

comment:80 Changed 21 months ago by git

  • Commit changed from 1d1039f8b082cb4e36d7ef0e34f6860a5385b9eb to 12aa8f264e4b454e991c75d48373a3d6f2f8b0cd

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

12aa8f2trac 23671 better ref to slides

comment:81 Changed 21 months ago by roed

  • Branch changed from u/chapoton/23671 to u/roed/23671

comment:82 Changed 21 months ago by roed

  • Commit changed from 12aa8f264e4b454e991c75d48373a3d6f2f8b0cd to de3f9cbac730e7031a4cc53aa291415660a2c841
  • Reviewers set to David Roe

Positive review if you're happy with my reviewer patch.


New commits:

de3f9cbReviewer changes to 23671

comment:83 Changed 21 months ago by chapoton

It seems that you introduced a TAB somewhere.

Otherwise, I agree with the changes.

comment:84 Changed 21 months ago by chapoton

  • Branch changed from u/roed/23671 to u/chapoton/23671
  • Commit changed from de3f9cbac730e7031a4cc53aa291415660a2c841 to 6bbe53ffd07df48f4a51f6e08c62ff5cfcb7c25e

I removed the tab


New commits:

6bbe53ftrac 23671 no more TAB

comment:85 Changed 21 months ago by roed

  • Status changed from needs_review to positive_review

Oops. I guess my emacs doesn't kill tabs in .rst files.

comment:86 Changed 21 months ago by roed

  • Branch changed from u/chapoton/23671 to u/roed/23671

comment:87 Changed 21 months ago by kedlaya

  • Commit changed from 6bbe53ffd07df48f4a51f6e08c62ff5cfcb7c25e to 0c54a659e37c82c1b98d9299d3dc04dc86aedec0

roed just added a @cached_method decorator to padic_H_value, H_value, and euler_factor.

I just went over roed's changes (most of which he made while sitting next to me) and they all look fine to me.


New commits:

0c54a65Add cached_method to hypergeometric motives

comment:88 Changed 21 months ago by chapoton

You re-introduced the TAB, please take care of it yourself

comment:89 Changed 21 months ago by chapoton

  • Status changed from positive_review to needs_work

comment:90 Changed 21 months ago by chapoton

  • Branch changed from u/roed/23671 to u/chapoton/23671
  • Commit changed from 0c54a659e37c82c1b98d9299d3dc04dc86aedec0 to 6bbe53ffd07df48f4a51f6e08c62ff5cfcb7c25e
  • Status changed from needs_work to positive_review

ok.. I have removed the TAB again..


New commits:

6bbe53ftrac 23671 no more TAB

comment:91 Changed 21 months ago by git

  • Commit changed from 6bbe53ffd07df48f4a51f6e08c62ff5cfcb7c25e to 778b876a2895a80c1622c11cb28441211b907ed8
  • Status changed from positive_review to needs_review

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. This was a forced push. New commits:

0c54a65Add cached_method to hypergeometric motives
778b876removing TAB again

comment:92 Changed 21 months ago by chapoton

  • Status changed from needs_review to positive_review

comment:93 Changed 21 months ago by vbraun

  • Status changed from positive_review to needs_work

Merge conflict...

comment:94 Changed 21 months ago by git

  • Commit changed from 778b876a2895a80c1622c11cb28441211b907ed8 to fd375d34156ef99ead97062523cd6db965d25f88

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

fd375d3Merge branch 'u/chapoton/23671' in 8.1.b7

comment:95 Changed 21 months ago by chapoton

  • Status changed from needs_work to positive_review

trivial rebase done (damn main reference file, I have not ordered that)

comment:96 Changed 21 months ago by vbraun

  • Status changed from positive_review to needs_work

PDF docs don't build

comment:97 Changed 21 months ago by git

  • Commit changed from fd375d34156ef99ead97062523cd6db965d25f88 to d8c961bfe6d67c265a5c615557ad7744cdf42e20

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

d8c961btrac 23671 care for pdf doc

comment:98 Changed 21 months ago by roed

Is there more to be done on the pdf docs, or should this be set back to positive review?

comment:99 Changed 21 months ago by chapoton

Well, somebody needs to check that it works, and I have not got time yet to do that.

comment:100 Changed 21 months ago by chapoton

  • Status changed from needs_work to positive_review

pdf ok

comment:101 Changed 20 months ago by chapoton

  • Milestone changed from sage-8.2 to sage-8.1

comment:102 Changed 20 months ago by vbraun

  • Branch changed from u/chapoton/23671 to d8c961bfe6d67c265a5c615557ad7744cdf42e20
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.