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

Make incomplete gamma function symbolic #7748

Closed
kcrisman opened this issue Dec 22, 2009 · 34 comments
Closed

Make incomplete gamma function symbolic #7748

kcrisman opened this issue Dec 22, 2009 · 34 comments

Comments

@kcrisman
Copy link
Member

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.

CC: @sagetrac-mvngu

Component: symbolics

Author: Flavia Stan, Burcin Erocal

Reviewer: Paul Zimmermann

Merged: sage-4.4.alpha0

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

@burcin
Copy link

burcin commented Dec 22, 2009

comment:1

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__ contains maxima='gamma_incomplete'.

@kcrisman
Copy link
Member Author

comment:2

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__ contains maxima='gamma_incomplete'.

Yes, of course, but we still have to do it :)

@williamstein
Copy link
Contributor

comment:3

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)?

@sagetrac-fstan
Copy link
Mannequin

sagetrac-fstan mannequin commented Feb 7, 2010

Make Ei symbolic.

@sagetrac-fstan
Copy link
Mannequin

sagetrac-fstan mannequin commented Feb 7, 2010

Attachment: trac_7748-exp_integral.patch.gz

Attachment: trac_7748-incomplete_gamma.patch.gz

Make incomplete gamma function symbolic.

@sagetrac-fstan
Copy link
Mannequin

sagetrac-fstan mannequin commented Feb 7, 2010

comment:4

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.

@sagetrac-fstan
Copy link
Mannequin

sagetrac-fstan mannequin commented Feb 7, 2010

Author: Flavia Stan

@sagetrac-fstan sagetrac-fstan mannequin added the s: needs review label Feb 7, 2010
@zimmermann6
Copy link

comment:5

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?

@kcrisman
Copy link
Member Author

kcrisman commented Feb 8, 2010

comment:6

Replying to @zimmermann6:

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.

@sagetrac-fstan
Copy link
Mannequin

sagetrac-fstan mannequin commented Feb 8, 2010

comment:7

Replying to @zimmermann6:

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

@zimmermann6
Copy link

comment:8

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

@zimmermann6 zimmermann6 assigned zimmermann6 and unassigned burcin Feb 8, 2010
@burcin
Copy link

burcin commented Feb 8, 2010

comment:9

Replying to @zimmermann6:

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 @sagetrac-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.

@zimmermann6
Copy link

comment:10

Flavia, please can you fix the rings/complex_number.pyx issue?

@sagetrac-fstan
Copy link
Mannequin

sagetrac-fstan mannequin commented Feb 15, 2010

Attachment: trac_7748-exp_integral_ver2.patch.gz

Added derivative method.

@sagetrac-fstan
Copy link
Mannequin

sagetrac-fstan mannequin commented Feb 15, 2010

Fixed tests from complex_number.pyx

@sagetrac-fstan
Copy link
Mannequin

sagetrac-fstan mannequin commented Feb 15, 2010

comment:11

Attachment: trac_7748-incomplete_gamma_ver2.patch.gz

I've uploaded new patches which should fix the docs and the derivative.

Greetings,

Flavia

@burcin
Copy link

burcin commented Feb 20, 2010

Attachment: trac_7748-exp_integral_ver2.4.3.3.alpha1.patch.gz

rebased to 4.3.3.alpha1

@burcin
Copy link

burcin commented Feb 20, 2010

rebased to 4.3.3.alpha1

@burcin
Copy link

burcin commented Feb 20, 2010

Attachment: trac_7748-incomplete_gamma_ver2.4.3.3.alpha1.patch.gz

Attachment: trac_7748-gamma_wrapper.patch.gz

wrapper for gamma and incomplete gamma

@burcin
Copy link

burcin commented Feb 20, 2010

Attachment: trac_7748-gamma_wrapper.2.patch.gz

wrapper for gamma and incomplete gamma

@burcin
Copy link

burcin commented Feb 20, 2010

comment:12

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):

I'm changing this to needs review. :)

@zimmermann6
Copy link

comment:13

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.

@zimmermann6
Copy link

Work Issues: new test failure

@zimmermann6
Copy link

comment:14

apart from the above failure, all tests pass with sage 4.3.3 (apart from the Fedora 12 Segfaults).

@burcin
Copy link

burcin commented Mar 2, 2010

Attachment: trac_7748-gamma_wrapper.3.patch.gz

fix new doctest failure

@burcin
Copy link

burcin commented Mar 2, 2010

comment:15

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):

I am OK with Flavia's changes. If someone can review my patch this can still get in 4.3.4. :)

@burcin
Copy link

burcin commented Mar 2, 2010

Reviewer: Paul Zimmermann

@zimmermann6
Copy link

comment:16

Everything works now. Great work!

Paul

@zimmermann6
Copy link

Changed author from Flavia Stan to Flavia Stan, Burcin Erocal

@burcin
Copy link

burcin commented Mar 17, 2010

Changed work issues from new test failure to none

@burcin
Copy link

burcin commented Mar 17, 2010

comment:17

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

@sagetrac-mvngu
Copy link
Mannequin

sagetrac-mvngu mannequin commented Mar 17, 2010

comment:18

Replying to @burcin:

What is holding up this ticket?

I can't merge this ticket. We are now in feature freeze.

@jhpalmieri
Copy link
Member

comment:19

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

@jhpalmieri
Copy link
Member

Merged: sage-4.4.alpha0

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

5 participants