Opened 11 years ago
Closed 11 years ago
#7748 closed defect (fixed)
Make incomplete gamma function symbolic
Reported by: | kcrisman | Owned by: | zimmerma |
---|---|---|---|
Priority: | minor | Milestone: | sage-4.4 |
Component: | symbolics | Keywords: | |
Cc: | mvngu | Merged in: | sage-4.4.alpha0 |
Authors: | Flavia Stan, Burcin Erocal | Reviewers: | Paul Zimmermann |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
We don't appear to have this, though perhaps it is in Pynac/Ginac? already. We also need to translate Maxima's gamma_incomplete to our gamma_inc or incomplete_gamma as part of this ticket.
Attachments (9)
Change History (28)
comment:1 follow-up: ↓ 2 Changed 11 years ago by
comment:2 in reply to: ↑ 1 Changed 11 years ago by
Replying to burcin:
After a quick look at the GiNaC sources, I don't think there is an implementation of the incomplete gamma function in there.
Okay.
Numerical evaluation of this can be done with the
gammainc()
function of mpmath.
But will it give us symbolic evaluation ala gamma_inc(1,1)=1/e? That would be best.
The conversion should just work, if the conversions dictionary argument of
BuiltinFunction.__init__
containsmaxima='gamma_incomplete'
.
Yes, of course, but we still have to do it :)
comment:3 Changed 11 years ago by
Numerical evaluation of this can be done with the gammainc() function of mpmath.
Doesn't PARI also have an incomplete gamma implementation (it's used by RR and CC, I think)?
comment:4 Changed 11 years ago by
- Status changed from new to needs_review
The 2 patches should make the incomplete gamma function symbolic. The exponential integral Ei was used in its symbolic evaluation, therefore it was also made symbolic.
comment:5 follow-ups: ↓ 6 ↓ 7 Changed 11 years ago by
I tried the exponential integral patch and it works ok. However I don't understand the changes in random_tests.py. Also the following could be evaluated:
sage: diff(Ei(x),x) D[0](Ei)(x) sage: integrate(Ei(x),x) integrate(Ei(x), x)
Should this be in a separate ticket?
comment:6 in reply to: ↑ 5 Changed 11 years ago by
Replying to zimmerma:
I tried the exponential integral patch and it works ok. However I don't understand the changes in random_tests.py.
This makes a 'random' symbolic expression, and when we add new symbolic functions (and sometimes when we change them) this doctest needs to be changed, not because anything bad happened.
comment:7 in reply to: ↑ 5 Changed 11 years ago by
- Status changed from needs_review to needs_work
Replying to zimmerma:
I tried the exponential integral patch and it works ok. However I don't understand the changes in random_tests.py. Also the following could be evaluated:
sage: diff(Ei(x),x) D[0](Ei)(x) sage: integrate(Ei(x),x) integrate(Ei(x), x)
Should this be in a separate ticket?
Thank you for the review. I'll include the derivative and submit again. Integration looks more complicated.
Greetings,
Flavia
comment:8 follow-up: ↓ 9 Changed 11 years ago by
- Owner changed from burcin to zimmerma
I tried sage -t * with both patches, and I get one additional failure with respect to those I get due to #7773:
tarte% sage -t "rings/complex_number.pyx" sage -t "rings/complex_number.pyx" ********************************************************************** File "/usr/local/sage-4.3-core2/devel/sage-main/sage/rings/complex_number.pyx", line 1611: sage: gamma_inc(2, 5) Expected: 0.0404276819945128 Got: gamma(2, 5)
Consider also the following:
sage: gamma_inc(2, 5.) 0.0404276819945128 sage: gamma(2, 5) 1
comment:9 in reply to: ↑ 8 Changed 11 years ago by
Replying to zimmerma:
Consider also the following:
sage: gamma_inc(2, 5.) 0.0404276819945128 sage: gamma(2, 5) 1
We should write a python function wrapping gamma_inc
and gamma
, similar to the provided for psi()
in #6961.
The fact that gamma()
accepts two arguments at the moment is a bug that I introduced in #7490. Looking at the code, it also doesn't handle the prec
parameter correctly. I'll upload a patch with a wrapper gamma()
function and a fix for the prec
issue.
Replying to fstan:
Integration looks more complicated.
Integration cannot be handled with a custom method in symbolic functions. The patches attached to #6465 should make this easier.
comment:10 Changed 11 years ago by
Flavia, please can you fix the rings/complex_number.pyx issue?
comment:11 Changed 11 years ago by
I've uploaded new patches which should fix the docs and the derivative.
Greetings,
Flavia
comment:12 Changed 11 years ago by
- Status changed from needs_work to needs_review
attachment:trac_7748-gamma_wrapper.2.patch adds a new wrapper function to call gamma or incomplete gamma depending on the number of arguments. It also corrects the way prec
parameter to gamma
was handled.
Flavia's patches needed to be rebased to 4.3.3.alpha1 since the changes to random_tests.py
didn't apply anymore.
The patches to be applied are (in this order):
- attachment:trac_7748-exp_integral_ver2.4.3.3.alpha1.patch
- attachment:trac_7748-incomplete_gamma_ver2.4.3.3.alpha1.patch
- attachment:trac_7748-gamma_wrapper.2.patch
I'm changing this to needs review. :)
comment:13 Changed 11 years ago by
- Status changed from needs_review to needs_work
- Work issues set to new test failure
a partial review against 4.3.3:
sage -t "devel/sage-main/sage/schemes/elliptic_curves/ell_rational_field.py" ... sage: E.Lambda(1.4+0.5*I, 50) File "/usr/local/sage-4.3.3/sage/local/lib/python/site-packages/sage/schemes/elliptic_curves/ell_rational_field.py", line 3088, in Lambda Gamma_inc = transcendental.gamma_inc AttributeError: 'module' object has no attribute 'gamma_inc'
This test was ok with vanilla 4.3.3.
comment:14 Changed 11 years ago by
apart from the above failure, all tests pass with sage 4.3.3 (apart from the Fedora 12 Segfaults).
comment:15 Changed 11 years ago by
- Reviewers set to Paul Zimmermann
- Status changed from needs_work to needs_review
I added a fix for the doctest failure in comment:13 to my patch, attachment:trac_7748-gamma_wrapper.3.patch.
The patches to be applied are (in this order):
- attachment:trac_7748-exp_integral_ver2.4.3.3.alpha1.patch
- attachment:trac_7748-incomplete_gamma_ver2.4.3.3.alpha1.patch
- attachment:trac_7748-gamma_wrapper.3.patch
I am OK with Flavia's changes. If someone can review my patch this can still get in 4.3.4. :)
comment:16 Changed 11 years ago by
- Status changed from needs_review to positive_review
Everything works now. Great work!
Paul
comment:17 follow-up: ↓ 18 Changed 11 years ago by
- Cc mvngu added; burcin removed
- Work issues new test failure deleted
Hi Minh,
What is holding up this ticket? Note that it has a positive review and the patches to be applied are listed in comment:15.
Cheers,
Burcin
comment:18 in reply to: ↑ 17 Changed 11 years ago by
Replying to burcin:
What is holding up this ticket?
I can't merge this ticket. We are now in feature freeze.
comment:19 Changed 11 years ago by
- Merged in set to sage-4.4.alpha0
- Resolution set to fixed
- Status changed from positive_review to closed
Merged in 4.4.alpha0:
- trac_7748-exp_integral_ver2.4.3.3.alpha1.patch
- trac_7748-incomplete_gamma_ver2.4.3.3.alpha1.patch
- trac_7748-gamma_wrapper.3.patch
After a quick look at the GiNaC sources, I don't think there is an implementation of the incomplete gamma function in there.
Numerical evaluation of this can be done with the
gammainc()
function of mpmath.The conversion should just work, if the conversions dictionary argument of
BuiltinFunction.__init__
containsmaxima='gamma_incomplete'
.