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:

Status badges

Description (last modified by Peter Bruin)

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)

15402_exponential_integral.patch (5.5 KB) - added by Jeroen Demeyer 9 years ago.
pari-2.5.5.p1.diff (14.4 KB) - added by Jeroen Demeyer 9 years ago.
15402_reviewer.patch (656 bytes) - added by Peter Bruin 9 years ago.
omit page number (editions of Cohen's book differ)

Download all attachments as: .zip

Change History (16)

comment:1 Changed 9 years ago by Jeroen Demeyer

Description: modified (diff)

comment:2 Changed 9 years ago by Jeroen Demeyer

Patch seems to work fine, modulo adding doctests.

comment:3 Changed 9 years ago by Jeroen Demeyer

Description: modified (diff)

comment:4 Changed 9 years ago by Jeroen Demeyer

Authors: Jeroen Demeyer
Status: newneeds_review

comment:5 Changed 9 years ago by Jeroen Demeyer

Description: modified (diff)

comment:6 Changed 9 years ago by Jeroen Demeyer

Status: needs_reviewneeds_work
Work issues: 32-bit SAGE_CHECK

Fails to install with SAGE_CHECK=yes on 32-bit systems.

comment:7 Changed 9 years ago by Peter Bruin

Just to check if I understand correctly, is the situation as follows?

  1. PARI's eint1() had potentially large errors both for a single argument and for the vector version;
  2. the single argument version is fixed upstream;
  3. for the vector version y = eint1(x, n) with n > 15, the entries y[i] with i >= 15 can still have large errors;
  4. 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 in reply to:  7 ; Changed 9 years ago by Jeroen Demeyer

Replying to pbruin:

Just to check if I understand correctly, is the situation as follows?

  1. PARI's eint1() had potentially large errors both for a single argument and for the vector version;
  2. the single argument version is fixed upstream;
  3. for the vector version y = eint1(x, n) with n > 15, the entries y[i] with i >= 15 can still have large errors;
  4. 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.

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 Jeroen Demeyer

Changed 9 years ago by Jeroen Demeyer

Attachment: pari-2.5.5.p1.diff added

comment:9 in reply to:  8 Changed 9 years ago by Peter Bruin

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 Jeroen Demeyer

Status: needs_workneeds_review
Work issues: 32-bit SAGE_CHECK

Tested on 32-bit and 64-bit.

Changed 9 years ago by Peter Bruin

Attachment: 15402_reviewer.patch added

omit page number (editions of Cohen's book differ)

comment:11 Changed 9 years ago by Peter Bruin

Cc: Peter Bruin removed
Description: modified (diff)
Reviewers: Peter Bruin
Status: needs_reviewpositive_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 in reply to:  11 Changed 9 years ago by Jeroen Demeyer

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 Jeroen Demeyer

Merged in: sage-5.13.beta4
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.