Opened 9 years ago

Closed 9 years ago

#12047 closed defect (fixed)

numerical_integral(f, a, a) should always be zero

Reported by: jdemeyer Owned by: burcin
Priority: major Milestone: sage-4.8
Component: calculus Keywords:
Cc: karsten.naert@…, zimmerma, mjo Merged in: sage-4.8.alpha3
Authors: Jeroen Demeyer Reviewers: Michael Orlitzky
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by jdemeyer)

Currently, in sage-4.7.2:

sage: integral_numerical(log(x), 0, 0)
(nan, nan)

Mathematically, the integral should certainly be zero: there is a primitive function which is continuous and defined at 0. Symbolically, we can compute the integral correctly:

sage: integral(log(x), (x,0,0))
0

So I would like to add a special-case check for integral_numerical(): if the interval of integration is a point, then always return 0.

I realize that this means that also the integral of 1/x from 0 to 0 would be 0, even though 1/x has no continuous primitive at 0. But according to the Lebesgue theory of integration, I think this is not even a problem.

Also: remove various unused things from the file sage/gsl/integration.pyx and clean up the documentation.

Attachments (2)

numerical_integration_tests.py (1.8 KB) - added by mjo 9 years ago.
A one-off script I used to throw random functions at the numerical integration
12047.patch (15.8 KB) - added by jdemeyer 9 years ago.

Download all attachments as: .zip

Change History (17)

comment:1 Changed 9 years ago by jdemeyer

  • Description modified (diff)

comment:2 Changed 9 years ago by jdemeyer

  • Description modified (diff)

comment:3 Changed 9 years ago by jdemeyer

  • Authors set to Jeroen Demeyer
  • Status changed from new to needs_review

comment:4 follow-up: Changed 9 years ago by kcrisman

Was that Fourier stuff ever going to be used? Or is it elsewhere? I didn't really look at it.

Considering that

sage: integral(1/x,x,0,0)
0

already, I guess we should do this. Are there any symbolic integrals on a point that Maxima doesn't give 0 for?

I'll review this more formally - especially the doc stuff, which seems good at a quick glance - if I get a chance, otherwise someone else feel free to do so!

comment:5 in reply to: ↑ 4 Changed 9 years ago by jdemeyer

Replying to kcrisman:

Was that Fourier stuff ever going to be used? Or is it elsewhere?

Probably "no" to both questions. That commented-out code has been there since 2007, the day that file was created. It does not look very developed, so I think it's okay to remove it.

comment:6 Changed 9 years ago by jdemeyer

  • Cc karsten.naert@… added

comment:7 Changed 9 years ago by zimmerma

  • Cc zimmerma added

Changed 9 years ago by mjo

A one-off script I used to throw random functions at the numerical integration

comment:8 follow-up: Changed 9 years ago by mjo

  • Cc mjo added
  • Status changed from needs_review to needs_work

You should add the ticket number to the new doctests, and maybe add yourself as author/copyright. Other than that, it looks good.

comment:9 in reply to: ↑ 8 Changed 9 years ago by jdemeyer

Replying to mjo:

You should add the ticket number to the new doctests, and maybe add yourself as author/copyright.

Done that.

Changed 9 years ago by jdemeyer

comment:10 Changed 9 years ago by jdemeyer

  • Status changed from needs_work to needs_review

comment:11 follow-up: Changed 9 years ago by mjo

  • Status changed from needs_review to positive_review

Positive review from me, then, although this is my first review so you may want a second opinion.

comment:12 Changed 9 years ago by kcrisman

  • Reviewers set to Michael Orlitzky

comment:13 in reply to: ↑ 11 Changed 9 years ago by jdemeyer

Replying to mjo:

Positive review from me, then, although this is my first review so you may want a second opinion.

First time for everything, right? I guess this patch is pretty safe, it makes only a small functional change to Sage. If all doctests pass and the documentation builds and looks okay (which is the case), then this patch should be merged.

Thanks for the review!

comment:14 Changed 9 years ago by kcrisman

@mjo:

Given the author and his current role, I'm sure this isn't an issue, but one thing to make sure to do with things that change the reference manual is to do

sage -docbuild reference html

or the like, just to check that links look right and there isn't a misplaced colon or something.

Looking forward to seeing many more reviews - and other contributions? - from you! Thanks for helping.

comment:15 Changed 9 years ago by jdemeyer

  • Merged in set to sage-4.8.alpha3
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.