Opened 3 years ago

Closed 2 years ago

#20260 closed enhancement (fixed)

p-adic polylogarithms, after Besser-de Jeu

Reported by: jen Owned by:
Priority: major Milestone: sage-7.2
Component: padics Keywords: days71, sd87, days88
Cc: frvillegas, caruso, kedlaya Merged in:
Authors: Alex J. Best Reviewers: Aly Deines
Report Upstream: N/A Work issues:
Branch: 9a9b388 (Commits) Commit: 9a9b388da4af27e3276e23b806b7f92f2f0b8244
Dependencies: Stopgaps:

Description (last modified by alexjbest)

Add code to compute p-adic polylogarithms, in particular, Li_n, based on the work of Besser and de Jeu ("Li(p)-service? An algorithm for computing p-adic polylogarithms").

Change History (31)

comment:1 Changed 2 years ago by caruso

  • Cc caruso added

comment:2 Changed 2 years ago by roed

  • Keywords sd87 added

comment:3 Changed 2 years ago by alexjbest

  • Branch set to u/alexjbest/padicpolylog

comment:4 Changed 2 years ago by alexjbest

  • Authors set to alexjbest
  • Commit set to b591293b9e48e70de27cb5ce4a6bf915ae83977a
  • Description modified (diff)
  • Status changed from new to needs_review

New commits:

41c9c1ainitial implementation of padic polylogarithms
b591293finish polylog implementation

comment:5 Changed 2 years ago by aly.deines

This has some trailing whitespace that should be removed.

comment:6 Changed 2 years ago by git

  • Commit changed from b591293b9e48e70de27cb5ce4a6bf915ae83977a to e5f1701274052616477fe61b6658dc829e7c838e

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

e5f1701remove trailing whitespace

comment:7 Changed 2 years ago by aly.deines

  • Status changed from needs_review to needs_work

comment:8 Changed 2 years ago by caruso

  • You must not recreate the parent (by K = Qp(p,prec)) but instead write K = self.parent().fraction_field() (in order to keep the same modes)
  • You should compute once for all K(self) and give a name to it
  • Why are you using the name _polylog_c whereas your function is not written in C
  • Is that really useful to have three external functions (namely _polylog_c, _findprec and _compute_g); cannot we instead include them directly in the method polylog
  • Instead of
            for x in K.teichmuller_system():
                if (self - x).valuation() > 0:
                    zeta = x
    

you probably should write zeta = K.teichmuller(x).

comment:9 Changed 2 years ago by saraedum

  • Authors alexjbest deleted

You should put a real name into the Authors field (so it shows up like that in the change notes.)

comment:10 Changed 2 years ago by git

  • Commit changed from e5f1701274052616477fe61b6658dc829e7c838e to d22d7a25db9f892050de5e3484df5633386eaeb6

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

d22d7a2Improved polylog implementation

comment:11 Changed 2 years ago by alexjbest

  • Authors set to Alex J. Best
  • Status changed from needs_work to needs_review

Thanks for all the comments caruso, I have made many of the changes you suggested, we discussed this offline but for completeness of the ticket:

Regarding the module functions, they are used in two of the public methods, so they cannot really be moved/removed, and the function _c is so named to be consistent with the paper of Besser-de Jeu I've tried to make that clearer in the docstring. The way precision works has been made much better, the user no longer specifies a precision instead the best possible is chosen and it is up to the user to change their element to lower precision first if they want quicker evaluation.

comment:12 Changed 2 years ago by git

  • Commit changed from d22d7a25db9f892050de5e3484df5633386eaeb6 to 36555913e09a3319384d8487528a5656d19debeb

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

3655591Fix docstring

comment:13 Changed 2 years ago by git

  • Commit changed from 36555913e09a3319384d8487528a5656d19debeb to 1adce545f98d83e726e9faa00c4d10de49acf7e8

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

1adce54made a lot of improvements suggested by roed and caruso

comment:14 Changed 2 years ago by git

  • Commit changed from 1adce545f98d83e726e9faa00c4d10de49acf7e8 to 33b4e87ebb9901b0cc8b4c695df5e0f3edf2790a

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

33b4e87fix power series initialisation when tsl not an integer

comment:15 Changed 2 years ago by git

  • Commit changed from 33b4e87ebb9901b0cc8b4c695df5e0f3edf2790a to afc8e211df6f1ca33bd285c25d8aff2d6f5c6712

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

aafb6f0Merge branch 'develop' into padicpolylog
afc8e21Correct known precision in residue zero case

comment:16 Changed 2 years ago by roed

Hi Alex, This doesn't currently merge.

I won't be able to look at this over the weekend, but I'll be at Sage Days 88 next week and should have a chance then.

comment:17 Changed 2 years ago by git

  • Commit changed from afc8e211df6f1ca33bd285c25d8aff2d6f5c6712 to f2e28ffd2c5cdb7436709ab7c0a19264694212bd

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

f2e28ffMerge branch 'develop' into padicpolylog

comment:18 Changed 2 years ago by alexjbest

Hi David, oops, hopefully it's fixed now. I'm pretty happy with the code right now, I've thrown a lot of tests at it and it looks to be working fine, so when you do get a chance more comments would be great!

comment:19 Changed 2 years ago by aly.deines

  • Status changed from needs_review to needs_work

Hi Alex, I found a couple typos in your documentation.

  • Rob De Jeu should be Rob de Jeu
  • Computation should be capitalized in Mathematics of Computation
  • Tate should be capitalized.
  • The year in the citation [DCW2015] should be 2016
  • In the polylog function, 'n'th the should be the nth
  • prop 6.1 should be Prop 6.1

comment:20 Changed 2 years ago by jen

  • Keywords days88 added

comment:21 Changed 2 years ago by git

  • Commit changed from f2e28ffd2c5cdb7436709ab7c0a19264694212bd to 5194c5a3c11f8ecb5cdf415a5d9e1d6ce26073b8

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

5194c5aFixes to doc

comment:22 Changed 2 years ago by kedlaya

  • Cc kedlaya added

comment:23 Changed 2 years ago by git

  • Commit changed from 5194c5a3c11f8ecb5cdf415a5d9e1d6ce26073b8 to 1b581a20a9e36d57e2c5eeaa8e63ead9c53d1a8b

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

1b581a2More fixes to docs

comment:24 Changed 2 years ago by aly.deines

  • Reviewers set to Aly Deines

Sorry, I have another change. Everywhere the citation [DCW2016] is referenced, the 2015 needs to be changed to 2016. In particular:

2016.17 (2015): 5291-5354.

should be

2016.17 (2016): 5291-5354.

This occurs in two places.

comment:25 Changed 2 years ago by alexjbest

No problem, I like things to be as good/correct as possible! In this instance I thought about it but was unsure what to do, if we look at https://academic.oup.com/imrn/article-abstract/2016/17/5291/2451667/Mixed-Tate-Motives-and-the-Unit-Equation?redirectedFrom=fulltext the journal issue is 2016 but date of publication listed as 2015, I don't know much about correct citation styles though. If you still think it should be 2016 I will make it so!

comment:26 Changed 2 years ago by aly.deines

Hmm. I was going off MathSciNet?:

Dan-Cohen, Ishai(D-DUES2M); Wewers, Stefan(D-ULM-IPM)
Mixed Tate motives and the unit equation. (English summary) 
Int. Math. Res. Not. IMRN 2016, no. 17, 5291–5354. 

comment:27 Changed 2 years ago by git

  • Commit changed from 1b581a20a9e36d57e2c5eeaa8e63ead9c53d1a8b to 48be838a73c10e1558a489514260aeadc89625af

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

48be838Change dates for references

comment:28 Changed 2 years ago by alexjbest

  • Status changed from needs_work to needs_review

MathSciNet? is good enough for me, thanks!

comment:29 Changed 2 years ago by aly.deines

  • Branch changed from u/alexjbest/padicpolylog to u/aly.deines/padicpolylog

comment:30 Changed 2 years ago by aly.deines

  • Commit changed from 48be838a73c10e1558a489514260aeadc89625af to 9a9b388da4af27e3276e23b806b7f92f2f0b8244
  • Status changed from needs_review to positive_review

I removed some trailing whitespace in the references.

It looks good to me.


New commits:

4b11445Merge branch 'u/alexjbest/padicpolylog' of git://trac.sagemath.org/sage into t/20260/padicpolylog
9a9b388Removed trailing whitespace.

comment:31 Changed 2 years ago by vbraun

  • Branch changed from u/aly.deines/padicpolylog to 9a9b388da4af27e3276e23b806b7f92f2f0b8244
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.