Opened 10 years ago
Closed 10 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: |
Description (last modified by )
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)
Change History (17)
comment:1 Changed 10 years ago by
- Description modified (diff)
comment:2 Changed 10 years ago by
- Description modified (diff)
comment:3 Changed 10 years ago by
- Status changed from new to needs_review
comment:4 follow-up: ↓ 5 Changed 10 years ago by
comment:5 in reply to: ↑ 4 Changed 10 years ago by
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 10 years ago by
- Cc karsten.naert@… added
comment:7 Changed 10 years ago by
- Cc zimmerma added
Changed 10 years ago by
A one-off script I used to throw random functions at the numerical integration
comment:8 follow-up: ↓ 9 Changed 10 years ago by
- 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 10 years ago by
Replying to mjo:
You should add the ticket number to the new doctests, and maybe add yourself as author/copyright.
Done that.
Changed 10 years ago by
comment:10 Changed 10 years ago by
- Status changed from needs_work to needs_review
comment:11 follow-up: ↓ 13 Changed 10 years ago by
- 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 10 years ago by
- Reviewers set to Michael Orlitzky
comment:13 in reply to: ↑ 11 Changed 10 years ago by
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 10 years ago by
@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 10 years ago by
- Merged in set to sage-4.8.alpha3
- Resolution set to fixed
- Status changed from positive_review to closed
Was that Fourier stuff ever going to be used? Or is it elsewhere? I didn't really look at it.
Considering that
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!