Opened 4 years ago
Closed 3 years ago
#26098 closed enhancement (fixed)
Implement Lfunctions using the PARI library
Reported by:  Frédéric Chapoton  Owned by:  

Priority:  major  Milestone:  sage8.9 
Component:  number theory  Keywords:  dokchitser, lseries, zeta 
Cc:  John Cremona, David LowryDuda  Merged in:  
Authors:  Frédéric Chapoton  Reviewers:  Travis Scrimshaw 
Report Upstream:  N/A  Work issues:  
Branch:  091f71b (Commits, GitHub, GitLab)  Commit:  091f71b6100ec912d2e28c451ff87231abeee1e9 
Dependencies:  Stopgaps: 
Description (last modified by )
To replace Dokchitser's scripts.
some information here:
Change History (104)
comment:1 Changed 4 years ago by
Branch:  → public/26098 

Keywords:  dokchitser added 
comment:2 Changed 4 years ago by
Commit:  → f53b6432cedcdf76e3adfe2f30c5fdf679d95e46 

comment:3 Changed 4 years ago by
Description:  modified (diff) 

comment:4 Changed 4 years ago by
Commit:  f53b6432cedcdf76e3adfe2f30c5fdf679d95e46 → e985afcebe6d3747d9a7f7c8e9705ca7edc55e2e 

Branch pushed to git repo; I updated commit sha1. New commits:
e985afc  adding the Lambda function

comment:5 Changed 4 years ago by
Good idea! It's something that I always wanted to do but never found the time...
Some immediate comments:
 Regarding
""" Dokchitser's Lfunctions Calculator from Pari AUTHORS:  Tim Dokchitser (2002): original PARI code and algorithm (and the documentation below is based on Dokchitser's docs).  William Stein (20060308): Sage interface """
To what extent is this related to code from Dokchitser or Stein? If the answer is "not at all", then the author block, the copyright block, the name of the module and the name of the class Dokchitser_Pari
should be fixed.
 Assigning
self.pari = Pari()
looks strange as there is really one PARI running. Better be honest about that and use a single globalpari
variable (you can import it fromsage.libs.pari
).
self.pari('default(realprecision, %s)')
is a bad idea for multiple reasons. The proper way to deal with precisions in library mode is either to set the precision argument for every function call or to use inexact inputs.
 Avoid using global PARI variables like in the
pari_precode
exampletau(n)=...
. Remember that there is only one global PARI instance, so global variables are global state! If you want functions, why not use actual functions (using the>
syntax)?
comment:6 Changed 4 years ago by
Commit:  e985afcebe6d3747d9a7f7c8e9705ca7edc55e2e → 4f159376a18570e8af12fab21bd88de0fee07130 

Branch pushed to git repo; I updated commit sha1. New commits:
4f15937  work on Lfunction direct interface to Pari lfun

comment:7 Changed 4 years ago by
Commit:  4f159376a18570e8af12fab21bd88de0fee07130 → c48fb4ba457b1c750f496374b4a154d59052c8b0 

Branch pushed to git repo; I updated commit sha1. New commits:
c48fb4b  trac 26098 plug into elliptic curves

comment:8 Changed 4 years ago by
Commit:  c48fb4ba457b1c750f496374b4a154d59052c8b0 → a364f56563638ce2fbcd617da8218b9e97099964 

Branch pushed to git repo; I updated commit sha1. New commits:
a364f56  details on 26098

comment:9 followup: 10 Changed 4 years ago by
Some more comments:
 You can replace
pari.FUNCTION(pari(FOO))
bypari.FUNCTION(FOO)
.
 It is still not clear to me why you keeping using "dokchitser" (I may be missing something, but then it should be clarified)
 Using the
.sage()
method is often not the best way to convert a PARI object to Sage because you loose information about the parent. For example:sage: x = RR.pi() sage: parix = pari(x) sage: parent(parix.sage()) Real Field with 64 bits of precision
It would be better to replace parix.sage()
by RR(parix)
in this case.
comment:10 followup: 14 Changed 4 years ago by
Replying to jdemeyer:
Some more comments:
 You can replace
pari.FUNCTION(pari(FOO))
bypari.FUNCTION(FOO)
.
will do
 It is still not clear to me why you keeping using "dokchitser" (I may be missing something, but then it should be clarified)
There are Dokchitser's algorithms (described in a famous article in Experimental Math) and Dokchitser's implementation of them (that we use so far). Pari is using (maybe an enhanced version of) the algorithms of Dokchitser or at least similar basic ideas.
comment:11 Changed 4 years ago by
Commit:  a364f56563638ce2fbcd617da8218b9e97099964 → 68b4d8f04a20c91104454be1fa3e952b6e302578 

Branch pushed to git repo; I updated commit sha1. New commits:
68b4d8f  trying to add Dirichlet Lfunctions

comment:12 Changed 4 years ago by
Commit:  68b4d8f04a20c91104454be1fa3e952b6e302578 → 574c147ca7a45854b3ae26f3fc4a8cb334744a3b 

Branch pushed to git repo; I updated commit sha1. New commits:
574c147  trac 26098 adding the derivative

comment:13 Changed 4 years ago by
Description:  modified (diff) 

New commits:
574c147  trac 26098 adding the derivative

comment:14 Changed 4 years ago by
Replying to chapoton:
Pari is using (maybe an enhanced version of) the algorithms of Dokchitser or at least similar basic ideas.
I don't think that is sufficient reason to use the name "Dokchitser". Personally, I think it is especially confusing because we also have Dokchitser's GP scripts (where he is the actual author). I would rather name the module lfunctions_pari.py
or something along those lines.
comment:15 followup: 20 Changed 4 years ago by
Branch:  public/26098 → u/chapoton/26098 

Cc:  John Cremona added 
Commit:  574c147ca7a45854b3ae26f3fc4a8cb334744a3b → 0fc42d265d744fc0cc8ea4d5c7dbe9e5a9e0d67f 
here is a new branch, with squashed commits
What is still lacking is a correct handling of precision
To people of lmfdb, is this something of interest for you ?
New commits:
0fc42d2  trac 26098 interface for the Lfunctions setting of Pari

comment:16 Changed 4 years ago by
Commit:  0fc42d265d744fc0cc8ea4d5c7dbe9e5a9e0d67f → ff9bef13851a57f2c2be5a29b2b40f4dec79ee24 

Branch pushed to git repo; I updated commit sha1. New commits:
ff9bef1  trac 26098 fixing some doctests

comment:17 Changed 4 years ago by
Commit:  ff9bef13851a57f2c2be5a29b2b40f4dec79ee24 → 96aa04d08c21c75e390808c9e9d6c47f8eb64ad7 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
96aa04d  trac 26098 interface for the Lfunctions setting of Pari

comment:18 Changed 4 years ago by
Commit:  96aa04d08c21c75e390808c9e9d6c47f8eb64ad7 → 8efdaf7702c14ed1761ceb42f11c6700f35aba86 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
8efdaf7  trac 26098 interface for the Lfunctions setting of Pari

comment:19 Changed 4 years ago by
Description:  modified (diff) 

Summary:  using Dokchitser setting direct from pari → Implement Lfunctions using the PARI library 
comment:20 Changed 4 years ago by
Replying to chapoton:
What is still lacking is a correct handling of precision
Every function where precision matters takes a precision
argument. For example
sage: pari(1).sin(precision=64).sage() 0.841470984807896507 sage: pari(1).sin(precision=256).sage() 0.8414709848078965066525023216302989996225630607983710656727517099919104043912
comment:21 Changed 4 years ago by
It would be nice to support calling lfuncreate
in LFunction.__init__
. Something like
if isinstance(lfun, lfun_generic): # preparation using Dokchitser data self._L = lfun.lfun() elif isinstance(lfun, Gen): # already some Pari lfun self._L = lfun else: # create a PARI lfunction from input data self._L = pari.lfuncreate(lfun)
This would remove the need for lfun_elliptic_curve()
and lfun_number_field()
.
comment:22 Changed 4 years ago by
Note that PARI functions also act as methods. So you could replace
pari.lfun(self._L, s)
by the simpler
self._L.lfun(s)
comment:23 followups: 24 26 Changed 4 years ago by
Concerning precision, this is not so easy..
sage: from sage.lfunctions.lfunctions_pari import * sage: lf = lfun_number_field(QQ) sage: pari.lfun(lf, 2) 1.64493406684823 sage: pari.lfun(lf, 2, precision=200) 1.64493406684823 sage: pari.lfun(lf, ComplexField(200)(2)) 1.64493406684823
I also have a difficulty with passing a function as you suggested.
comment:24 Changed 4 years ago by
Replying to chapoton:
Concerning precision, this is not so easy..
Don't confuse the printed output by PARI with the actual output. Try again adding .sage()
to those examples.
comment:25 Changed 4 years ago by
Commit:  8efdaf7702c14ed1761ceb42f11c6700f35aba86 → ccc3d7f706935f5a02719a686c484a8f517ca3d6 

Branch pushed to git repo; I updated commit sha1. New commits:
ccc3d7f  some details in #26098

comment:26 Changed 4 years ago by
Replying to chapoton:
I also have a difficulty with passing a function as you suggested.
Are you getting the error incorrect type in vecan_closure [wrong arity] (t_CLOSURE)
? That could be considered a cypari2
bug.
comment:27 Changed 4 years ago by
Support for functions in cypari2
is still incomplete, but this hack works for me:
sage: def zeta_coeffs(n): ....: return [1 for i in range(n)] sage: zeta = pari.lfuncreate([pari("f > (n > f(n))")(zeta_coeffs), 1, [0], 1, 1, 1, 1])
It's quite important to keep a reference to the function zeta_coeffs
, otherwise this will crash.
comment:28 Changed 4 years ago by
Commit:  ccc3d7f706935f5a02719a686c484a8f517ca3d6 → d153705675da7568191586aec32cab96f4a97758 

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

comment:29 Changed 4 years ago by
Commit:  d153705675da7568191586aec32cab96f4a97758 → 641b84e57dcfa5b422ffaf3884efb49d4c870729 

Branch pushed to git repo; I updated commit sha1. New commits:
641b84e  trac 26098 adding the Hardy function

comment:30 Changed 4 years ago by
some things that remain to be done:
 be able to have L function for Dirichlet characters
 fix the doctest with Ramanujan tau, how to handle that ?
 make sure that poles and residues are handled correctly
comment:31 Changed 4 years ago by
Description:  modified (diff) 

comment:32 Changed 4 years ago by
Commit:  641b84e57dcfa5b422ffaf3884efb49d4c870729 → 33170b3929a218de25d705976c8c204fced2a860 

Branch pushed to git repo; I updated commit sha1. New commits:
33170b3  toward dirichlet chars

comment:33 Changed 4 years ago by
Description:  modified (diff) 

comment:34 Changed 4 years ago by
Commit:  33170b3929a218de25d705976c8c204fced2a860 → 7a878dda2638c21392e7cf8b275721eaedb45c66 

Branch pushed to git repo; I updated commit sha1. New commits:
7a878dd  trac 26098 work on Dirichlet characters

comment:35 Changed 4 years ago by
Dependencies:  → #25567 

according to K. Belabas, one should rather wait for pari 2.11
comment:36 Changed 4 years ago by
Commit:  7a878dda2638c21392e7cf8b275721eaedb45c66 → cf501609842feb5235976b05930bc89e7f9702ac 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
cf50160  trac 26098 interface for the Lfunctions setting of Pari

comment:37 Changed 4 years ago by
Commit:  cf501609842feb5235976b05930bc89e7f9702ac → 98a04a175f3254cac6ce0aff2e8062a6ded9394f 

comment:38 Changed 4 years ago by
Commit:  98a04a175f3254cac6ce0aff2e8062a6ded9394f → e4053ea3b18a81bb74b3cbb424070f97fad8a007 

Branch pushed to git repo; I updated commit sha1. New commits:
e4053ea  adding lfunction method to Dirichlet characters

comment:39 Changed 4 years ago by
Commit:  e4053ea3b18a81bb74b3cbb424070f97fad8a007 → bc30647c0697e6345c22d8e5652508aa0944d031 

comment:40 Changed 4 years ago by
Dependencies:  #25567 

comment:41 Changed 4 years ago by
Still not able to make that work:
+ sage: from sage.lfunctions.lfunctions_pari import lfun_generic, LFunction + sage: lf = lfun_generic(conductor=1, gammaV=[0,1], weight=12, eps=1) + sage: tau = pari('n>(5*sigma(n,3)+7*sigma(n,5))*n/12  35*sum(k=1,n1,(6*k4*(nk))*sigma(k,3)*sigma(nk,5))') + sage: lf.init_coeffs(tau) + sage: L = LFunction(lf)
comment:42 Changed 4 years ago by
To make it easier for me to help, it would be good to say why it doesn't work. I don't want to recompile this branch this branch every time just to answer your comments.
comment:43 Changed 4 years ago by
Well.
sage: from sage.lfunctions.lfunctions_pari import lfun_generic, LFunction sage: lf = lfun_generic(conductor=1, gammaV=[0,1], weight=12, eps=1) sage: tau = pari('n>(5*sigma(n,3)+7*sigma(n,5))*n/12  35*sum(k=1,n1,(6*k4*(n ....: k))*sigma(k,3)*sigma(nk,5))') sage: lf.init_coeffs(tau) sage: L = LFunction(lf)
All this works, or seems to. But here is the traceback.
sage: L(5)  PariError Traceback (most recent call last) <ipythoninput6fd3c681015b7> in <module>() > 1 L(Integer(5)) /home/chapoton/sage/local/lib/python2.7/sitepackages/sage/lfunctions/lfunctions_pari.pyc in __call__(self, s) 635 pass 636 try: > 637 value = pari.lfun(self._L, s, precision=self.prec).sage() 638 except NameError: 639 raise ArithmeticError('pole here') cypari2/auto_instance.pxi in cypari2.pari_instance.Pari_auto.lfun() cypari2/handle_error.pyx in cypari2.handle_error._pari_err_handle() PariError: incorrect type in vecan_closure (t_INT)
Not clear to me where this "t_INT" could come from. And the failure appears only at a late stage..
comment:44 Changed 4 years ago by
The function is supposed to return a vector with n
components, not a single number. See the docs for lfuncreate
.
So you should conceptually replace n>f(n)
by n>vector(n, k, f(k))
.
comment:45 Changed 4 years ago by
Commit:  bc30647c0697e6345c22d8e5652508aa0944d031 → 833f3ba6e86e83fb64b39ed53864e3e2d7116321 

Branch pushed to git repo; I updated commit sha1. New commits:
833f3ba  pari Lfunctions, fix syntax for call with functions

comment:46 Changed 4 years ago by
Commit:  833f3ba6e86e83fb64b39ed53864e3e2d7116321 → b9bf637b0feb9b3cb3a2e27d9b33e58e73d5dd5c 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
b9bf637  trac 26098 interface for the Lfunctions setting of Pari

comment:47 Changed 4 years ago by
Commit:  b9bf637b0feb9b3cb3a2e27d9b33e58e73d5dd5c → 01a330dc3fdb1ae74993ee1f89f9a804156232ac 

Branch pushed to git repo; I updated commit sha1. New commits:
01a330d  adding the case of Lfunction for the Delta modular form

comment:48 Changed 4 years ago by
Commit:  01a330dc3fdb1ae74993ee1f89f9a804156232ac → d105016b51e59b44e9e265aa8372b4541e79a3b4 

Branch pushed to git repo; I updated commit sha1. New commits:
d105016  adding other types of Lfunctions

comment:49 Changed 4 years ago by
Commit:  d105016b51e59b44e9e265aa8372b4541e79a3b4 → c3c60b1af85a4574c2a162f46b0bf33625357658 

Branch pushed to git repo; I updated commit sha1. New commits:
c3c60b1  Lfunction for elliptic curves over number fields

comment:50 Changed 4 years ago by
Commit:  c3c60b1af85a4574c2a162f46b0bf33625357658 → 1e3b387b54069bbc9ac72c2b4ae1e1bd03f7ecb8 

Branch pushed to git repo; I updated commit sha1. New commits:
1e3b387  trac 26098 coverage 100%

comment:52 followup: 55 Changed 4 years ago by
Just a few quick comments to start with, as I have not yet built the code or looked in detail at the new interface:
 In the example for a Dirichlet Lfunction can you add the display of L itself to show the effect of its having been renamed?
 The docstring for delta_lseries is surely wrong: only one of the options interfaces with the (soon to be obsolete?) code of Tim Dok. And why is the default not the new pari version? Same for Dedekind Zeta.
 Should we have some examples computed with both gp and pari to show how they compare?
comment:53 Changed 4 years ago by
Commit:  1e3b387b54069bbc9ac72c2b4ae1e1bd03f7ecb8 → 3f667241a3d4641332f3a5e1ca61ce5bd6475422 

Branch pushed to git repo; I updated commit sha1. New commits:
3f66724  trac 2609_ fix doc of delta_lseries

comment:54 Changed 4 years ago by
Commit:  3f667241a3d4641332f3a5e1ca61ce5bd6475422 → c1fa53345f2889986eba0f27f3b8878aefa5cb0f 

Branch pushed to git repo; I updated commit sha1. New commits:
c1fa533  trac 26098 fix

comment:55 Changed 4 years ago by
Replying to cremona:
Just a few quick comments to start with, as I have not yet built the code or looked in detail at the new interface:
 In the example for a Dirichlet Lfunction can you add the display of L itself to show the effect of its having been renamed?
Sure, done.
 The docstring for delta_lseries is surely wrong: only one of the options interfaces with the (soon to be obsolete?) code of Tim Dok. And why is the default not the new pari version? Same for Dedekind Zeta.
Fixed the docstring of delta_lseries
Concerning the switch to "pari" by default, maybe this new interface is not yet mature enough.
 Should we have some examples computed with both gp and pari to show how they compare?
But this is already the case, I think.
comment:56 Changed 4 years ago by
Commit:  c1fa53345f2889986eba0f27f3b8878aefa5cb0f → ea66f8987db654568b9002dd33c0180d7c07278a 

Branch pushed to git repo; I updated commit sha1. New commits:
ea66f89  trac 26098 fix details

comment:57 Changed 4 years ago by
Commit:  ea66f8987db654568b9002dd33c0180d7c07278a → c1311d5f9c372bbe00eb3b36f072c228dc01f3d0 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
c1311d5  trac 26098 interface for the Lfunctions setting of Pari

comment:58 Changed 4 years ago by
Commit:  c1311d5f9c372bbe00eb3b36f072c228dc01f3d0 → 9dec5842840c084dab33da617b8fb23e0569e2b4 

Branch pushed to git repo; I updated commit sha1. New commits:
9dec584  trac 26098 work on Dirichlet characters

comment:59 Changed 4 years ago by
Commit:  9dec5842840c084dab33da617b8fb23e0569e2b4 → c1cf2678e2de2e56297ab0a281b515a61284bd23 

Branch pushed to git repo; I updated commit sha1. New commits:
c1cf267  trac 26098 cut long doctest into short lines

comment:60 Changed 4 years ago by
As I mentioned before, try to avoid using the .sage()
method for PARI objects, as it doesn't give control over the resulting parent. In general, an explicit conversion like
R(pari_object)
should be preferred over
pari_object.sage()
comment:61 Changed 4 years ago by
In other words: the .sage()
method exists mostly for convenience, not for correctness.
comment:62 Changed 4 years ago by
The changes to src/sage/schemes/elliptic_curves/ell_generic.py
look independent from the rest of this ticket, could they be split off?
comment:63 Changed 4 years ago by
Dependencies:  → #26232 

I have separated the change in pari conversion of elliptic curves over number fields (#26232).
comment:64 Changed 4 years ago by
Commit:  c1cf2678e2de2e56297ab0a281b515a61284bd23 → afdc0d8bb26c17f85992b1b3c1038fdee81e1f7c 

comment:66 Changed 4 years ago by
I agree with @jdemeyer that using x.sage() is dangerous. I am right now writing code which gets some quite complicated data back from gp functions and am having to write customised conversion functions.
comment:67 Changed 4 years ago by
Commit:  afdc0d8bb26c17f85992b1b3c1038fdee81e1f7c → f9211d99517193afec4632bc9b26f66ff6c1c43d 

comment:68 Changed 4 years ago by
Description:  modified (diff) 

comment:70 Changed 4 years ago by
Commit:  f9211d99517193afec4632bc9b26f66ff6c1c43d → 0649b7799691023042ba0d5017328ab60ffc276d 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
0649b77  trac 26098 interface for the Lfunctions setting of Pari

comment:71 Changed 4 years ago by
Replacing the last .sage() seems to result in 3 less digits in the results. Should I do that nevertheless ?
Failed example: L(2) Expected: 1.64493406684822644 Got: 1.64493406684823
comment:72 Changed 4 years ago by
Do you mean that it shows fewer decimals with .sage() than without? When converting to/from pari remember that Sage allows any bit precision but pari only allows multiples of 32 (or something like that). So often the underlying pari computation is more precise than necessary.
comment:73 Changed 4 years ago by
Commit:  0649b7799691023042ba0d5017328ab60ffc276d → f170198af4143561fe36d950cc3b7fdf46d8561f 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
f170198  trac 26098 interface for the Lfunctions setting of Pari

comment:74 Changed 4 years ago by
Any other comment on this proposal ? I feel that this is not quite polished, but maybe already useful. I would appreciate feedback.
comment:75 Changed 4 years ago by
Cc:  David LowryDuda added 

Milestone:  sage8.4 → sage8.7 
comment:76 Changed 4 years ago by
Dependencies:  #26232 

comment:77 Changed 4 years ago by
Authors:  → Frédéric Chapoton 

Status:  new → needs_review 
comment:78 Changed 4 years ago by
Branch:  u/chapoton/26098 → u/jdemeyer/26098 

comment:79 Changed 4 years ago by
Commit:  f170198af4143561fe36d950cc3b7fdf46d8561f → c5858fa661ca4fcb96a7f423bbe753beb9995fa4 

Rebased to 8.7.beta5 (fixed trivial merge conflict).
New commits:
c5858fa  trac 26098 interface for the Lfunctions setting of Pari

comment:80 Changed 4 years ago by
I have various comments, but it's probably easier for both of us if I just add an extra commit.
comment:82 Changed 4 years ago by
Commit:  c5858fa661ca4fcb96a7f423bbe753beb9995fa4 → b53fe6ed13a1a697cf3a7de78e8b3259dbbfe463 

comment:83 Changed 4 years ago by
This is very much work in progress. I'm currently working on cypari2 to add support for __floordiv__
(for some reason, this was missing) and closures of a given arity. That will help for this ticket.
comment:84 Changed 4 years ago by
Status:  needs_review → needs_work 

comment:85 Changed 4 years ago by
Keywords:  lseries zeta added 

comment:86 Changed 4 years ago by
@jdemeyer, is the cypari part now ok ? I have seen the floordiv was added.
comment:88 Changed 4 years ago by
Branch:  u/jdemeyer/26098 → u/chapoton/26098 

Commit:  b53fe6ed13a1a697cf3a7de78e8b3259dbbfe463 → c902517d7ca39c0c586a2466c67eb412dc0a6d19 
comment:89 Changed 4 years ago by
One thing to note is that the default precision with the PARI implementation is 53, but it was 64 with the GP scripts.
comment:90 Changed 4 years ago by
Milestone:  sage8.7 → sage8.8 

Ticket retargeted after milestone closed (if you don't believe this ticket is appropriate for the Sage 8.8 release please retarget manually)
comment:91 Changed 3 years ago by
Commit:  c902517d7ca39c0c586a2466c67eb412dc0a6d19 → e179f7cbed931932ebae23dd29468d29a3538a06 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
11b04eb  trac 26098 interface for the Lfunctions setting of Pari

c8690e8  Use 'pari' algorithm by default for Lfunctions

47bd670  Rename lfunctions_pari > pari

7215b8a  Further fixes to PARI Lfunctions

e2eeb03  trac 26098 fixing some of the doctests

e179f7c  trac #26098 fix doctests

comment:92 Changed 3 years ago by
Status:  needs_work → needs_review 

doctests fixed, back to needs review
comment:94 Changed 3 years ago by
A comment and a question:
RANK 1 ELLIPTIC CURVE:
> .. RUBRIC:: Rank 1 elliptic curve
(see, e.g., combinat/root_system/cartan_type.py
)
Should init_coeffs
be exposed to the user as a public function? If so, should it be allowed to be run multiple times?
comment:95 Changed 3 years ago by
Commit:  e179f7cbed931932ebae23dd29468d29a3538a06 → 0ecf8c1b43e9986bfa6f7ef52dba1b78d917b1ce 

comment:96 Changed 3 years ago by
Can we please try to move on here ? This seems to me to be a progress over the current situation, even if a tiny one.
I think one can keep init_coeffs
as a public method, to be used as explained:
+ Initialisation of a :pari:`lfun` from motivic data. + + This can happen either in one stage or in two stages: the coefficients + can be given using the ``init`` keyword, or entered using + the :meth:`init_coeffs`.
There remains something not very clear about the handling of poles and residues.
I would like to get more feedback from lmfdb or pari people.
comment:97 Changed 3 years ago by
Commit:  0ecf8c1b43e9986bfa6f7ef52dba1b78d917b1ce → 9d69f415b4826df03a2db35ba254f69e692d5765 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
a7f23b9  trac 26098 interface for the Lfunctions setting of Pari

f29ca95  Use 'pari' algorithm by default for Lfunctions

1e575b3  Rename lfunctions_pari > pari

4658c47  Further fixes to PARI Lfunctions

3ae97a5  trac 26098 fixing some of the doctests

03ea8bd  trac #26098 fix doctests

e1ed425  using rubric as suggested

9d69f41  32 bit fixes

comment:98 Changed 3 years ago by
Milestone:  sage8.8 → sage8.9 

Moving tickets from the Sage 8.8 milestone that have been actively worked on in the last six months to the next release milestone (optimistically).
comment:100 Changed 3 years ago by
Sorry for taking time to get back to this.
I do not quite understand the necessity of the init_coeffs
method. I feel like there would be more standard ways to refactor that out (for example, just allowing initialization as normal). Or is this meant to be something done more lazily? You could also remove the __init
attribute by just setting self._L = None
in the __init__
and checking against that.
A little bit better doc formatting IMO:
 Create a PARI `L`function (:pari:`lfun` instance) with + Create a PARI `L`function (:pari:`lfun` instance) with::  ``lfun_generic(conductor, gammaV, weight, eps, poles, residues, init)`` + lfun_generic(conductor, gammaV, weight, eps, poles, residues, init)
This seems dubious to me:
values_on_gens = (chi(x) for x in pari_gens) # now compute the input for pari (list of exponents) P = chi.parent() if is_ComplexField(P.base_ring()): zeta = P.zeta() zeta_argument = zeta.argument() v = [int(round(x.argument() / zeta_argument)) for x in values_on_gens()] # <<< This part here else: dlog = P._zeta_dlog v = [dlog[x] for x in values_on_gens]
with value_on_gens
being a generator object, I don't think it should be callable.
check_functional_equation
could use some more standard docstring formatting.
Otherwise everything else LGTM. A lot of these points I do not care about too much (since I doubt I will ever use this code because it is outside my research area), but I still feel it is worth a quick moment of thought.
comment:101 Changed 3 years ago by
Commit:  9d69f415b4826df03a2db35ba254f69e692d5765 → 091f71b6100ec912d2e28c451ff87231abeee1e9 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
b3a8793  trac 26098 interface for the Lfunctions setting of Pari

ed1b8bd  Use 'pari' algorithm by default for Lfunctions

60cbdd4  Rename lfunctions_pari > pari

f1adcb1  Further fixes to PARI Lfunctions

852104e  trac 26098 fixing some of the doctests

fa4b096  trac #26098 fix doctests

a3c1834  using rubric as suggested

5aa9723  32 bit fixes

091f71b  some reviewer's suggestions

comment:102 Changed 3 years ago by
Reviewers:  → Travis Scrimshaw 

Status:  needs_review → positive_review 
Thanks. LGTM. (Sorry, I didn't notice the update.)
comment:103 Changed 3 years ago by
Regarding the Dirichlet Lfunctions, see also #16477. Does this ticket supersede it or at least implement some of it? (Maybe it only does it for Dirichlet characters and not general arithmetic functions.)
comment:104 Changed 3 years ago by
Branch:  u/chapoton/26098 → 091f71b6100ec912d2e28c451ff87231abeee1e9 

Resolution:  → fixed 
Status:  positive_review → closed 
Branch pushed to git repo; I updated commit sha1. New commits:
tentative to use the Dokchitser setting from pari itself..