Opened 3 years ago
Closed 2 years ago
#20260 closed enhancement (fixed)
padic polylogarithms, after Besserde Jeu
Reported by:  jen  Owned by:  

Priority:  major  Milestone:  sage7.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 )
Add code to compute padic polylogarithms, in particular, Li_n, based on the work of Besser and de Jeu ("Li^{(p)}service? An algorithm for computing padic polylogarithms").
Change History (31)
comment:1 Changed 2 years ago by
 Cc caruso added
comment:2 Changed 2 years ago by
 Keywords sd87 added
comment:3 Changed 2 years ago by
 Branch set to u/alexjbest/padicpolylog
comment:4 Changed 2 years ago by
 Commit set to b591293b9e48e70de27cb5ce4a6bf915ae83977a
 Description modified (diff)
 Status changed from new to needs_review
comment:5 Changed 2 years ago by
This has some trailing whitespace that should be removed.
comment:6 Changed 2 years ago by
 Commit changed from b591293b9e48e70de27cb5ce4a6bf915ae83977a to e5f1701274052616477fe61b6658dc829e7c838e
Branch pushed to git repo; I updated commit sha1. New commits:
e5f1701  remove trailing whitespace

comment:7 Changed 2 years ago by
 Status changed from needs_review to needs_work
comment:8 Changed 2 years ago by
 You must not recreate the parent (by
K = Qp(p,prec)
) but instead writeK = 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 methodpolylog
 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
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
 Commit changed from e5f1701274052616477fe61b6658dc829e7c838e to d22d7a25db9f892050de5e3484df5633386eaeb6
Branch pushed to git repo; I updated commit sha1. New commits:
d22d7a2  Improved polylog implementation

comment:11 Changed 2 years ago by
 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 Besserde 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
 Commit changed from d22d7a25db9f892050de5e3484df5633386eaeb6 to 36555913e09a3319384d8487528a5656d19debeb
Branch pushed to git repo; I updated commit sha1. New commits:
3655591  Fix docstring

comment:13 Changed 2 years ago by
 Commit changed from 36555913e09a3319384d8487528a5656d19debeb to 1adce545f98d83e726e9faa00c4d10de49acf7e8
Branch pushed to git repo; I updated commit sha1. New commits:
1adce54  made a lot of improvements suggested by roed and caruso

comment:14 Changed 2 years ago by
 Commit changed from 1adce545f98d83e726e9faa00c4d10de49acf7e8 to 33b4e87ebb9901b0cc8b4c695df5e0f3edf2790a
Branch pushed to git repo; I updated commit sha1. New commits:
33b4e87  fix power series initialisation when tsl not an integer

comment:15 Changed 2 years ago by
 Commit changed from 33b4e87ebb9901b0cc8b4c695df5e0f3edf2790a to afc8e211df6f1ca33bd285c25d8aff2d6f5c6712
comment:16 Changed 2 years ago by
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
 Commit changed from afc8e211df6f1ca33bd285c25d8aff2d6f5c6712 to f2e28ffd2c5cdb7436709ab7c0a19264694212bd
Branch pushed to git repo; I updated commit sha1. New commits:
f2e28ff  Merge branch 'develop' into padicpolylog

comment:18 Changed 2 years ago by
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
 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
n
th  prop 6.1 should be Prop 6.1
comment:20 Changed 2 years ago by
 Keywords days88 added
comment:21 Changed 2 years ago by
 Commit changed from f2e28ffd2c5cdb7436709ab7c0a19264694212bd to 5194c5a3c11f8ecb5cdf415a5d9e1d6ce26073b8
Branch pushed to git repo; I updated commit sha1. New commits:
5194c5a  Fixes to doc

comment:22 Changed 2 years ago by
 Cc kedlaya added
comment:23 Changed 2 years ago by
 Commit changed from 5194c5a3c11f8ecb5cdf415a5d9e1d6ce26073b8 to 1b581a20a9e36d57e2c5eeaa8e63ead9c53d1a8b
Branch pushed to git repo; I updated commit sha1. New commits:
1b581a2  More fixes to docs

comment:24 Changed 2 years ago by
 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): 52915354.
should be
2016.17 (2016): 52915354.
This occurs in two places.
comment:25 Changed 2 years ago by
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/articleabstract/2016/17/5291/2451667/MixedTateMotivesandtheUnitEquation?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
Hmm. I was going off MathSciNet?:
DanCohen, Ishai(DDUES2M); Wewers, Stefan(DULMIPM) 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
 Commit changed from 1b581a20a9e36d57e2c5eeaa8e63ead9c53d1a8b to 48be838a73c10e1558a489514260aeadc89625af
Branch pushed to git repo; I updated commit sha1. New commits:
48be838  Change dates for references

comment:28 Changed 2 years ago by
 Status changed from needs_work to needs_review
MathSciNet? is good enough for me, thanks!
comment:29 Changed 2 years ago by
 Branch changed from u/alexjbest/padicpolylog to u/aly.deines/padicpolylog
comment:30 Changed 2 years ago by
 Commit changed from 48be838a73c10e1558a489514260aeadc89625af to 9a9b388da4af27e3276e23b806b7f92f2f0b8244
 Status changed from needs_review to positive_review
comment:31 Changed 2 years ago by
 Branch changed from u/aly.deines/padicpolylog to 9a9b388da4af27e3276e23b806b7f92f2f0b8244
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
initial implementation of padic polylogarithms
finish polylog implementation