Opened 9 years ago
Closed 9 years ago
#15402 closed enhancement (fixed)
PARI: add patch for exponential_integral_1() precision
Reported by: | Jeroen Demeyer | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-5.13 |
Component: | packages: standard | Keywords: | |
Cc: | Merged in: | sage-5.13.beta4 | |
Authors: | Jeroen Demeyer | Reviewers: | Peter Bruin |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
There is a bug in PARI where the precision of the eint1()
function (exponential integral) is quite bad. This is fixed upstream and we should take the patch into the PARI spkg. See also #15299.
spkg http://boxen.math.washington.edu/home/jdemeyer/spkg/pari-2.5.5.p1.spkg (spkg diff)
apply 15402_exponential_integral.patch, 15402_reviewer.patch
Attachments (3)
Change History (16)
comment:1 Changed 9 years ago by
Description: | modified (diff) |
---|
comment:2 Changed 9 years ago by
comment:3 Changed 9 years ago by
Description: | modified (diff) |
---|
comment:4 Changed 9 years ago by
Authors: | → Jeroen Demeyer |
---|---|
Status: | new → needs_review |
comment:5 Changed 9 years ago by
Description: | modified (diff) |
---|
comment:6 Changed 9 years ago by
Status: | needs_review → needs_work |
---|---|
Work issues: | → 32-bit SAGE_CHECK |
Fails to install with SAGE_CHECK=yes
on 32-bit systems.
comment:7 follow-up: 8 Changed 9 years ago by
Just to check if I understand correctly, is the situation as follows?
- PARI's
eint1()
had potentially large errors both for a single argument and for the vector version; - the single argument version is fixed upstream;
- for the vector version
y = eint1(x, n)
withn > 15
, the entriesy[i]
withi >= 15
can still have large errors; - the patch fixes (1) by backporting the upstream fix (2), and fixes (3) by increasing the precision depending on n.
A small observation about where to look for the cause of (3): it could be related to the fact that mpveceint1()
(in basemath/trans3.c
) apparently computes the y[i]
with i >= 15
via a recursive formula going from i = n
downwards. I guess any detailed discussion of this should continue on the pari-dev
mailing list.
comment:8 follow-up: 9 Changed 9 years ago by
Replying to pbruin:
Just to check if I understand correctly, is the situation as follows?
- PARI's
eint1()
had potentially large errors both for a single argument and for the vector version;- the single argument version is fixed upstream;
- for the vector version
y = eint1(x, n)
withn > 15
, the entriesy[i]
withi >= 15
can still have large errors;- the patch fixes (1) by backporting the upstream fix (2), and fixes (3) by increasing the precision depending on n.
A small observation about where to look for the cause of (3): it could be related to the fact that
mpveceint1()
(inbasemath/trans3.c
) apparently computes they[i]
withi >= 15
via a recursive formula going fromi = n
downwards. I guess any detailed discussion of this should continue on thepari-dev
mailing list.
You're mostly right, except that the vector version is also much better in the PARI git version. That algorithm was completely rewritten and I also backported that.
I found the extra number of bits needed experimentally.
Changed 9 years ago by
Attachment: | 15402_exponential_integral.patch added |
---|
Changed 9 years ago by
Attachment: | pari-2.5.5.p1.diff added |
---|
comment:9 Changed 9 years ago by
Replying to jdemeyer:
You're mostly right, except that the vector version is also much better in the PARI git version. That algorithm was completely rewritten and I also backported that.
I see, this new implementation was not in Karim Belabas's recent patch, but was written by Henri Cohen and was already in PARI 2.6.1, before #15299 existed.
comment:10 Changed 9 years ago by
Status: | needs_work → needs_review |
---|---|
Work issues: | 32-bit SAGE_CHECK |
Tested on 32-bit and 64-bit.
Changed 9 years ago by
Attachment: | 15402_reviewer.patch added |
---|
omit page number (editions of Cohen's book differ)
comment:11 follow-up: 12 Changed 9 years ago by
Cc: | Peter Bruin removed |
---|---|
Description: | modified (diff) |
Reviewers: | → Peter Bruin |
Status: | needs_review → positive_review |
Looks good, works as documented, and the new doctests appear to be very thorough and as good as you can possibly get, apart from proving the error bound from the PARI source code.
Just a trivial reviewer patch to fix the reference to Cohen (even though you didn't touch this): the page number of Prop. 5.6.12 in my edition is different, so I just removed the page number.
comment:12 Changed 9 years ago by
Replying to pbruin:
Looks good, works as documented, and the new doctests appear to be very thorough and as good as you can possibly get, apart from proving the error bound from the PARI source code.
Exactly. The errors bounds that I get are a combination of reading the PARI source code and experiments, they are not proven.
comment:13 Changed 9 years ago by
Merged in: | → sage-5.13.beta4 |
---|---|
Resolution: | → fixed |
Status: | positive_review → closed |
Patch seems to work fine, modulo adding doctests.