Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug in integrating x*cos(x^3) #12947

Closed
kcrisman opened this issue May 14, 2012 · 33 comments
Closed

Bug in integrating x*cos(x^3) #12947

kcrisman opened this issue May 14, 2012 · 33 comments

Comments

@kcrisman
Copy link
Member

From this sage-support thread:

sage: numerical_integral(x*cos(x^3),0,.5)
(0.1247560409610376, 1.3850702913602309e-15)
sage: integrate(x*cos(x^3),(x,0,1/2)).n()
-0.0677842754623305

Given

sage: integrate(x*cos(x^3),x)
1/12*((-I*sqrt(3) - 1)*gamma(2/3, -I*x^3) + (I*sqrt(3) - 1)*gamma(2/3, I*x^3))*(x^3)^(1/3)/x
sage: integrate(x*cos(x^3),(x,0,1/2))
-1/3*gamma(2/3) + 1/6*gamma(2/3, -1/8*I) + 1/6*gamma(2/3, 1/8*I)

this is probably something where Maxima is returning too many imaginary things and something isn't cancelling right either there or in Sage proper? We've had trouble with either side of this with incomplete gamma functions before.

CC: @orlitzky

Component: calculus

Author: Peter Bruin

Branch/Commit: 72706eb

Reviewer: Ralf Stephan

Issue created by migration from https://trac.sagemath.org/ticket/12947

@kcrisman
Copy link
Member Author

comment:1
sage: assume(u>0)
sage: integrate(x*cos(x^3),x,0,u)
-1/3*gamma(2/3) + 1/6*gamma(2/3, -I*u^3) + 1/6*gamma(2/3, I*u^3)

To compare the "right" answer, do

sage: f(u) = integrate(x*cos(x^3),x,0,u)
sage: plot(f,(u,0,2*pi))
sage: plot(lambda u: numerical_integral(x*cos(x^3),0,u)[0],(u,0,2*pi))

Note how they don't really look quite similar, not really a branch cut thing? Also, in Maxima

(%i2) display2d:false;
(%o2) false
(%i3) f:integrate(x*cos(x^3),x);
(%o3) (gamma_incomplete(2/3,%i*x^3)+gamma_incomplete(2/3,-%i*x^3))/6
(%i5) diff(f,x);
(%o5) (-3*%i*x*%e^(%i*x^3)/(-1)^(1/6)-3*%i*x*%e^-(%i*x^3)/(-1)^(1/6))/6

@kcrisman
Copy link
Member Author

comment:2

Robert Dodier of Maxima suggests the following later in the thread.


(%i5) display2d:false;

(%o5) false
(%i6) integrate(x*cos(x^3),x);

(%o6) (gamma_incomplete(2/3,%i*x^3)+gamma_incomplete(2/3,-%i*x^3))/6
(%i7) domain:complex;

(%o7) complex
(%i8) integrate(x*cos(x^3),x);

(%o8) ((sqrt(3)*%i-1)*gamma_incomplete(2/3,%i*x^3)
       +(-sqrt(3)*%i-1)*gamma_incomplete(2/3,-%i*x^3))
       *(x^3)^(1/3)
       /(12*x)

But this doesn't resolve the original issue.


(%i1) display2d:false;

(%o1) false
(%i2) integrate(x*cos(x^3),x,0,1/2);

(%o2) gamma_incomplete(2/3,%i/8)/6+gamma_incomplete(2/3,-%i/8)/6-gamma(2/3)/3
(%i3) domain:complex;

(%o3) complex
(%i4) integrate(x*cos(x^3),x,0,1/2);

(%o4) gamma_incomplete(2/3,%i/8)/6+gamma_incomplete(2/3,-%i/8)/6-gamma(2/3)/3

@kcrisman
Copy link
Member Author

comment:3

Robert points out that in 5.27.0 this is at least a little better, though we still need to change domain.


Maxima 5.27.0 http://maxima.sourceforge.net
using Lisp SBCL 1.0.24
Distributed under the GNU Public License. See the file COPYING.
Dedicated to the memory of William Schelter.
The function bug_report() provides bug reporting information.
(%i1) display2d:false;

(%o1) false
(%i2) domain:complex;

(%o2) complex
(%i3) integrate (x*cos(x^3), x, 0, 1/2); 

(%o3) %i*gamma_incomplete(2/3,%i/8)/(4*sqrt(3))
 -gamma_incomplete(2/3,%i/8)/12-%i*gamma_incomplete(2/3,-%i/8)/(4*sqrt(3))
 -gamma_incomplete(2/3,-%i/8)/12+gamma(2/3)/6
(%i4) expand(float(%));

(%o4) .1247560409610377

@kcrisman
Copy link
Member Author

comment:4

But since we already use domain:complex in the initialization code for Maxima for integrals and such, presumably an upgrade would fix this?

@kcrisman
Copy link
Member Author

Upstream: Fixed upstream, in a later stable release.

@orlitzky
Copy link
Contributor

comment:5

See my last comment on #11238 for why we can't have nice things.

That's the only real issue with maxima-5.27. Some messages are getting mangled e.g. when maxima asks us for more information, but they aren't substantial regressions.

@jdemeyer jdemeyer modified the milestones: sage-5.11, sage-5.12 Aug 13, 2013
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.1, sage-6.2 Jan 30, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.2, sage-6.3 May 6, 2014
@pjbruin
Copy link
Contributor

pjbruin commented May 30, 2014

comment:9

This seems to have been fixed some time ago; it works correctly in both Sage 5.13 and 6.2.

sage: integrate(x*cos(x^3),(x,0,1/2)).n()
0.124756040961038

@pjbruin
Copy link
Contributor

pjbruin commented May 30, 2014

Changed upstream from Fixed upstream, in a later stable release. to none

@pjbruin
Copy link
Contributor

pjbruin commented May 30, 2014

Work Issues: add doctest

@pjbruin
Copy link
Contributor

pjbruin commented May 30, 2014

Changed work issues from add doctest to none

@pjbruin
Copy link
Contributor

pjbruin commented May 30, 2014

Branch: u/pbruin/12947-maxima_integral

@pjbruin
Copy link
Contributor

pjbruin commented May 30, 2014

Author: Peter Bruin

@pjbruin
Copy link
Contributor

pjbruin commented May 30, 2014

Commit: 35f6081

@rwst
Copy link

rwst commented May 31, 2014

comment:11

Funny I get:

File "src/sage/interfaces/maxima_lib.py", line 736, in sage.interfaces.maxima_lib.MaximaLib.sr_integral
Failed example:
    integrate(x*cos(x^3),(x,0,1/2)).n()
Expected:
    0.124756040961038
Got:
    0.124756040961038 + 2.77555756156289e-17*I

Please set positive when you think this is resolved.

@rwst
Copy link

rwst commented May 31, 2014

Reviewer: Ralf Stephan

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 31, 2014

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

3d9a07cTrac 12947: add tolerance to doctest

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 31, 2014

Changed commit from 35f6081 to 3d9a07c

@pjbruin
Copy link
Contributor

pjbruin commented May 31, 2014

comment:13

On the system I tested this on (GNU/Linux x86_64) this gives the expected answer, both with and without the Maxima upgrade of #13973. I just checked on ARM; the first time I got the same answer as you, but then I merged this branch (did not even rebuild) and only got the expected answer from then on. I added # abs tol 1e-16; can you check if this solves the problem for you?

@kcrisman
Copy link
Member Author

comment:14

I would argue that the complex bit means that we are not quite ready to merge this if it happens on some platforms. The question is whether this is a Maxima bug or a Sage/Pynac simplification bug.

@rwst
Copy link

rwst commented May 31, 2014

comment:15

Would you equally mind if there is a real digit wrong in the 17-th place? Regardless, it's fine on AMD now.

@rwst
Copy link

rwst commented May 31, 2014

comment:16

There are 60(!) lines in the sage code with # abs tol and nearly all have lower tolerance (in the sense of 'larger error')

@pjbruin
Copy link
Contributor

pjbruin commented May 31, 2014

comment:17

The complex bit apparently comes from applying Pynac's evalf() to the following expression:

sage: e = integrate(x*cos(x^3),(x,0,1/2)); e
1/12*I*sqrt(3)*gamma(2/3, 1/8*I) - 1/12*I*sqrt(3)*gamma(2/3, -1/8*I) - 1/12*gamma(2/3, 1/8*I) - 1/12*gamma(2/3, -1/8*I) + 1/6*gamma(2/3)
sage: e.n(53)
0.124756040961038 + 2.77555756156289e-17*I
sage: e._convert({'parent': CC})  # called by .n()
0.124756040961038 + 2.77555756156289e-17*I

(This is on ARM; the imaginary part does not appear on x86_64.)

There are known precision problems in the incomplete Gamma function, so this may be related to #7099, although we only want the default 53 bits of precision here.

@pjbruin
Copy link
Contributor

pjbruin commented May 31, 2014

comment:18

The presence of the imaginary part does not depend on #7099. At least on ARM it is essentially random (maybe it depends on FPU rounding flags or something similar):

$ ./sage -c 'print integrate(x*cos(x^3),(x,0,1/2)).n()'
0.124756040961038
$ ./sage -c 'print integrate(x*cos(x^3),(x,0,1/2)).n()'
0.124756040961038 - 2.77555756156289e-17*I
$ ./sage -c 'print integrate(x*cos(x^3),(x,0,1/2)).n()'
0.124756040961038 - 1.38777878078145e-17*I
$ ./sage -c 'print integrate(x*cos(x^3),(x,0,1/2)).n()'
0.124756040961038 + 1.38777878078145e-17*I

@kcrisman
Copy link
Member Author

kcrisman commented Jun 1, 2014

comment:19

Would you equally mind if there is a real digit wrong in the 17-th place? Regardless, it's fine on AMD now.

Haha! No, but as I said sometimes the complex bit indicates a different kind of problem, or has in the past. Given Peter's experiment in comment:18, though, I guess we can make that a different ticket.

@pjbruin
Copy link
Contributor

pjbruin commented Jun 2, 2014

comment:20

The imaginary part turns out to be "simply" the fact that floating-point addition is not associative, and for some reason the terms in the expression e from comment:17 are added in random order by .n().

sage: e = integrate(x*cos(x^3),(x,0,1/2)); e
1/12*I*sqrt(3)*gamma(2/3, 1/8*I) - 1/12*I*sqrt(3)*gamma(2/3, -1/8*I) - 1/12*gamma(2/3, 1/8*I) - 1/12*gamma(2/3, -1/8*I) + 1/6*gamma(2/3)
sage: v = [term.n() for term in e.operands()]; v
[0.0454319516149362 + 0.166098636946833*I,
 0.0454319516149362 - 0.166098636946833*I,
 -0.0958970927532841 + 0.0262301494946934*I,
 -0.0958970927532841 - 0.0262301494946934*I,
 0.225686323237733]
sage: uniq([sum(p.action(v)) for p in permutations(5)])
[0.124756040961038 - 2.77555756156289e-17*I,
 0.124756040961038,
 0.124756040961038 + 2.77555756156289e-17*I,
 0.124756040961038 - 1.38777878078145e-17*I,
 0.124756040961038,
 0.124756040961038 + 1.38777878078145e-17*I,
 0.124756040961038 - 2.77555756156289e-17*I,
 0.124756040961038,
 0.124756040961038 + 2.77555756156289e-17*I,
 0.124756040961038 - 1.38777878078145e-17*I,
 0.124756040961038 + 1.38777878078145e-17*I,
 0.124756040961038 - 2.77555756156289e-17*I,
 0.124756040961038 + 2.77555756156289e-17*I,
 0.124756040961038]

This gives 14 different outcomes depending on the summation order (some of the differences are hidden due to the number of digits printed). I don't think this can be solved easily, so I propose we stick to the # abs tol fix.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 11, 2014

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

8274e47Merge branch 'develop' into ticket/12947-numerical_integral

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 11, 2014

Changed commit from 3d9a07c to 8274e47

@rwst
Copy link

rwst commented Jun 29, 2014

comment:22

As already said ...

@vbraun
Copy link
Member

vbraun commented Jun 30, 2014

comment:23

The # abs tol just changes how the floating point numbers are compared, it doesn't know about complex numbers.

sage -t --long src/sage/interfaces/maxima_lib.py
**********************************************************************
File "src/sage/interfaces/maxima_lib.py", line 763, in sage.interfaces.maxima_lib.MaximaLib.sr_integral
Failed example:
    integrate(x*cos(x^3),(x,0,1/2)).n()  # abs tol 1e-16
Expected:
    0.124756040961038
Got:
    0.124756040961038 - 1.38777878078145e-17*I
**********************************************************************

and on a different machine

sage -t --long src/sage/interfaces/maxima_lib.py
**********************************************************************
File "src/sage/interfaces/maxima_lib.py", line 763, in sage.interfaces.maxima_lib.MaximaLib.sr_integral
Failed example:
    integrate(x*cos(x^3),(x,0,1/2)).n()  # abs tol 1e-16
Expected:
    0.124756040961038
Got:
    0.124756040961038 - 2.77555756156289e-17*I
**********************************************************************

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 18, 2014

Changed commit from 8274e47 to 72706eb

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 18, 2014

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

72706ebTrac 12947: handle numerical noise correctly

@pjbruin
Copy link
Contributor

pjbruin commented Jul 18, 2014

comment:26

This should solve it; probably no reason to ask for another review.

@vbraun
Copy link
Member

vbraun commented Jul 19, 2014

Changed branch from u/pbruin/12947-maxima_integral to 72706eb

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants