sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.41k stars 475 forks source link

implement symbolic lower incomplete gamma function #16697

Closed rwst closed 8 years ago

rwst commented 10 years ago

This is actually a defect because we leave a result from Maxima undefined:

sage: hypergeometric([1],[b],x).simplify_hypergeometric()
(b - 1)*x^(-b + 1)*e^x*gamma_greek(b - 1, x)
sage: gamma_greek
---------------------------------------------------------------------------
NameError                                 Traceback (most recent call last)
<ipython-input-18-6e90901bc5cb> in <module>()
----> 1 gamma_greek

NameError: name 'gamma_greek' is not defined

https://en.wikipedia.org/wiki/Incomplete_gamma_function#Lower_incomplete_Gamma_function

Mathematica seems to have Gamma[a,z] for upper and Gamma[a,0,z] for lower; Maple seems to have upper Gamma. gamma_inc (the upper one in Sage) gets immediately converted to gamma(a,x). The symbolic functions gamma_inc==incomplete_gamma are converted and never returned to the user as expression:

sage: gamma_inc(x,x,hold=True)
gamma(x, x)
sage: incomplete_gamma(x,x,hold=True)
gamma(x, x)
sage: assume(x>0)
sage: integral(t^(s-1)*e^(-t),t,0,x)
-gamma(s, x) + gamma(s)

This ticket should deprecate "incomplete_gamma" and add the symbolic function gamma_inc_lower, leaving open the question of the global alias for and the displayed name of Function_gamma_inc.

Previous part of description:


  1. Provide all three "user input interfaces" gamma_inc, incomplete_gamma and lower_incomplete_gamma, and convert the Maxima gamma_greek to -gamma(a, x) + gamma(a)
  2. Provide all three "user input interfaces" gamma_inc, incomplete_gamma and lower_incomplete_gamma, and convert to gamma(a,x) and gamma(a,0,x)n on output
  3. Provide all three "user input interfaces" gamma_inc, incomplete_gamma and lower_incomplete_gamma, and have incomplete_gamma and lower_incomplete_gamma as result instead of `gamma(...)'
  4. Change the user interface completely to lower_incomplete_gamma and upper_incomplete_gamma
  5. Change the user interface completely to gamma_inc_lower and gamma_inc_upper
  6. Change the user interface completely to gamma(a,x) and gamma(a,0,x)

Depends on #20185

CC: @kcrisman @nbruin

Component: symbolics

Keywords: gamma, incomplete, special, functions

Author: Ralf Stephan

Branch/Commit: 157b268

Reviewer: Buck Evan, Paul Masson

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

nbruin commented 10 years ago
comment:2

We don't need all of gamma, gamma_inc, incomplete_gamma as symbolic function if they all mean the same. Some of them are probably there for user convenience or because of legacy. I think we want only one spelling for gamma_upper, whatever it may be. The gamma(a,x) convention seems fairly prominent in computer algebra, but is a poor translation of math notation, where the difference between upper and lower is made with case (and indeed, the complete gamma function tends to be capital gamma. I don't think I've ever seen different).

Because of tab completion, you definitely want a binding available that starts with gamma. I'd be fine with gamma_lower but if you want to go with gamma_lower_incomplete, that's fine with me too. So, I'd propose

  1. Maintain status quo with respect to upper incomplete gamma, i.e., don't change sage's behaviour for gamma(a,x) and gamma_inc(a,x). Just implement a symbolic function gamma_lower (or modified spelling) to correspond to maxima's gamma_greek.
rwst commented 10 years ago
comment:3

I don't underswtand. With "leave as is" you would keep gamma(a,x) as returned expression for the upper function? Or would you make gamma_inc fully symbolic in that it gets no longer converted to gamma(a,x)?

rwst commented 10 years ago
comment:4

The reason why gamma_inc(x,y) gets output as gamma(x,y) is not conversion as I thought but simple naming super in the constructor. It would be just as easy to make that gamma_inc and change a few doctests such that then:

         sage: f = exp_integral_e(-1,x)
         sage: f.integrate(x)
-        Ei(-x) - gamma(-1, x)
+        Ei(-x) - gamma_inc(-1, x)

             sage: gamma_inc(3,2)
-            gamma(3, 2)
+            gamma_inc(3, 2)

             sage: gamma(3,2)
             gamma_inc(3, 2)

             sage: gamma(x,0)
             gamma(x)

             sage: gamma(x, 0, hold=True)
-            gamma(x, 0)
+            gamma_inc(x, 0)

I'm appending the patch that enables this here, so it is preserved.

rwst commented 10 years ago

Attachment: 16697-gammainc-alt-patch.gz

see comment

rwst commented 10 years ago

Branch: u/rws/implement_symbolic_lower_incomplete_gamma_function

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 10 years ago

Commit: 93c2f94

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 10 years ago

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

93c2f9416697: deprecate "incomplete_gamma"
rwst commented 10 years ago

Description changed:

--- 
+++ 
@@ -13,7 +13,7 @@

https://en.wikipedia.org/wiki/Incomplete_gamma_function#Lower_incomplete_Gamma_function

-Mathematica seems to have Gamma[a,z] for upper and Gamma[a,0,z] for lower; Maple seems to have upper Gamma. gamma_inc (the upper one in Sage) gets immediately converted to gamma(a,x), so Sage has already went the Mathematica path of gamma(a)=gamma(a,x)+gamma(a,0,x). The symbolic functions gamma_inc==incomplete_gamma are converted and never returned to the user as expression: +Mathematica seems to have Gamma[a,z] for upper and Gamma[a,0,z] for lower; Maple seems to have upper Gamma. gamma_inc (the upper one in Sage) gets immediately converted to gamma(a,x). The symbolic functions gamma_inc==incomplete_gamma are converted and never returned to the user as expression:

 sage: gamma_inc(x,x,hold=True)
@@ -24,7 +24,11 @@
 sage: integral(t^(s-1)*e^(-t),t,0,x)
 -gamma(s, x) + gamma(s)

-So, what's the plan? +This ticket should deprecate "incomplete_gamma" and add the symbolic function gamma_inc_lower, leaving open the question of the global alias for and the displayed name of Function_gamma_inc. + +Previous part of description: + +---

  1. Provide all three "user input interfaces" gamma_inc, incomplete_gamma and lower_incomplete_gamma, and convert the Maxima gamma_greek to -gamma(a, x) + gamma(a)
  2. Provide all three "user input interfaces" gamma_inc, incomplete_gamma and lower_incomplete_gamma, and convert to gamma(a,x) and gamma(a,0,x)n on output
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 10 years ago

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

5c1dd6716697: implement gamma_inc_lower
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 10 years ago

Changed commit from 93c2f94 to 5c1dd67

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 10 years ago

Changed commit from 5c1dd67 to d8d7ee2

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 10 years ago

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

d8d7ee216697: use mpmath as default evalf algorithm
nbruin commented 10 years ago
comment:11

On sage-support Rafael Greenblatt reports

sage: (incomplete_gamma(x,y).diff(x)).simplify()
TypeError: ECL says: Error executing code in Maxima: gamma: wrong number of arguments.

The problem probably arises because the function ends up separated from its arguments:

sage: incomplete_gamma(x,y).diff(x)
D[0](gamma)(x, y)

which might confuse the translator (the behaviour is consistent with just going by strings). The changes here might improve the situation. It's worth checking.

The following of course works because it does NOT primarily look at strings:

sage: from sage.interfaces.maxima_lib import *
sage: maxima_calculus(sr_to_max(incomplete_gamma(x,y).diff(x)))
'diff(gamma_incomplete(x,y),x,1)
rwst commented 10 years ago
comment:12

Replying to @nbruin:

On sage-support Rafael Greenblatt reports

sage: (incomplete_gamma(x,y).diff(x)).simplify()
TypeError: ECL says: Error executing code in Maxima: gamma: wrong number of arguments.

The problem probably arises because the function ends up separated from its arguments:

sage: incomplete_gamma(x,y).diff(x)
D[0](gamma)(x, y)

which might confuse the translator (the behaviour is consistent with just going by strings). The changes here might improve the situation. It's worth checking.

Does the same with the patch. Maybe include the fix here? Is it simple?

nbruin commented 10 years ago
comment:13

Replying to @rwst:

Does the same with the patch. Maybe include the fix here? Is it simple?

Might not be too bad. I think the problem is in sage.symbolic.expression_conversions.InterfaceInit.derivative (line 515 or so). You can see there that the string representation of a derivative expression gets assembled by using f.name(). Replacing that by f._maxima_init_() might already do a better job:

sage: gamma(x,y).operator().name()
'gamma'
sage: gamma(x).operator().name()
'gamma'
sage: gamma(x,y).operator()._maxima_init_()
'gamma_incomplete'
sage: gamma(x).operator()._maxima_init_()
'gamma'
rwst commented 10 years ago
comment:14

That gives:

sage: (incomplete_gamma(x,y).diff(x)).simplify()
D[0](gamma)(x, y)
nbruin commented 10 years ago
comment:15

Replying to @rwst:

That gives:

sage: (incomplete_gamma(x,y).diff(x)).simplify()
D[0](gamma)(x, y)

Cool! so it IS easy. Be sure to also test (I haven't checked the answer is correct, but at least it shouldn't raise an error):

sage: (incomplete_gamma(x,x+1).diff(x)).simplify().simplify()
-(x + 1)^(x - 1)*e^(-x - 1) + D[0](gamma)(x, x + 1)

to check that both occurrences of f.name() have been replaced. I check the rest of that file and didn't find any further suspicious uses of name so after this, our maxima interface is probably perfect :-).

nbruin commented 10 years ago
comment:16

See #16785 for a more complete fix.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 10 years ago

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

862798bMerge branch 'develop' into t/16697/implement_symbolic_lower_incomplete_gamma_function
705c34e16697: fix a related bug
a73c5e316697: fix previous patch and a doctest
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 10 years ago

Changed commit from d8d7ee2 to a73c5e3

rwst commented 10 years ago
comment:18

I also had to fix a doctest in integration where the given result didn't have enough precision because it came from maxima. Now the default is mpmath and that uncovered it.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 10 years ago

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

7558a7c16697: revert fix in favor of merge of 16785
9c0f66dtrac #16785: improve differential operator translation to maxima
1fc19e7Merge branch 't/16785/ticket/16785' into t/16697/implement_symbolic_lower_incomplete_gamma_function
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 10 years ago

Changed commit from a73c5e3 to 1fc19e7

rwst commented 10 years ago

Merged: #16785

kcrisman commented 10 years ago

Changed merged from #16785 to none

kcrisman commented 10 years ago
comment:22

Sorry, we should make this clearer - that field is one currently not used but which should be (!!!) that was for saying which version/beta of Sage a given ticket was merged in. I agree that might not be clear given the word "merge" meaning something else in git!

rwst commented 10 years ago
comment:23

So, is there anything missing?

rwst commented 10 years ago

Author: Ralf Stephan

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 10 years ago

Changed commit from 1fc19e7 to 9033be8

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 10 years ago

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

9033be8Merge branch 'develop' into t/16697/implement_symbolic_lower_incomplete_gamma_function
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 10 years ago

Changed commit from 9033be8 to 05195e7

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 10 years ago

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

05195e7Merge branch 'develop' into t/16697/implement_symbolic_lower_incomplete_gamma_function
jdemeyer commented 10 years ago
comment:27

Just to let you know: this conflicts with #17130.

kcrisman commented 9 years ago
comment:28

See also the information here, which seems to be more information than the documentation of Maxima gives.


Implementation of the Incomplete Gamma function

 New Maxima User function: gamma_incomplete(a,z)

 The following features are implemented:

 - Evaluation for real and complex numbers in double float and
   bigfloat precision
 - Special values for gamma_incomplete(a,0) and gamma_incomplete(a,inf)
 - When $gamma_expand T expand the following expressions:
   gamma_incomplete(0,z)
   gamma_incomplete(n+1/2)
   gamma_incomplete(1/2-n)
   gamma_incomplete(n,z)
   gamma_incomplete(-n,z)
   gamma_incomplete(a+n,z)
   gamma_incomplete(a-n,z)
 - Mirror symmetry
 - Derivative wrt the arguments a and z

--------------------------------------------------------------------------------
Implementation of the Generalized Incomplete Gamma function

 New Maxima User function: gamma_incomplete_generalized(a,z1,z2)

 The following features are implemented:

 - Evaluation for real and complex numbers in double float and
   bigfloat precision
 - Special values for:
   gamma_incomplete_generalized(a,z1,0)
   gamma_incomplete_generalized(a,0,z2),
   gamma_incomplete_generalized(a,z1,inf)
   gamma_incomplete_generalized(a,inf,z2)
   gamma_incomplete_generalized(a,0,inf)
   gamma_incomplete_generalized(a,x,x)
 - When $gamma_expand T and n an integer expand
   gamma_incomplete_generalized(a+n,z1,z2)
 - Implementation of Mirror symmetry
 - Derivative wrt the arguments a, z1 and z2

--------------------------------------------------------------------------------
Implementation of the Regularized Incomplete Gamma function

 New Maxima User function: gamma_incomplete_regularized(a,z)

 The following features are implemented:

 - Evaluation for real and complex numbers in double float and
   bigfloat precision
 - Special values for:
   gamma_incomplete_regularized(a,0)
   gamma_incomplete_regularized(0,z)
   gamma_incomplete_regularized(a,inf)
 - When $gamma_expand T and n a positive integer expansions for
   gamma_incomplete_regularized(n+1/2,z)
   gamma_incomplete_regularized(1/2-n,z)
   gamma_incomplete_regularized(n,z)
   gamma_incomplete_regularized(a+n,z)
   gamma_incomplete_regularized(a-n,z)
 - Derivative wrt the arguments a and z
 - Implementation of Mirror symmetry
rwst commented 9 years ago
comment:29

We don't have gamma_incomplete_regularized either. This would be a different ticket.

rwst commented 9 years ago
comment:30

I'm going back to Pari as default since mpmath has similar problems as earlier Pari, see #17328 and https://github.com/fredrik-johansson/mpmath/issues/301

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 9 years ago

Changed commit from 05195e7 to 6f59f3a

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 9 years ago

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

6d10729Simplify numerical evaluation of BuiltinFunctions
b6e1ed4Merge remote-tracking branches 'origin/u/jdemeyer/ticket/17131' and 'origin/u/jdemeyer/ticket/17133' into ticket/17130
382f97aMerge branch 'u/jdemeyer/ticket/17130' of trac.sagemath.org:sage into 6.5beta1
726598917130: reviewer's patch: fix typo
c47dbd4Fix Trac #17328 again in a better way
a486db2Call the factorial() method of an Integer
9d3cbbdFix numerical noise
abab222Fix more numerical noise
a383a5bMerge branch 'u/jdemeyer/ticket/17130' of trac.sagemath.org:sage into t/16697/implement_symbolic_lower_incomplete_gamma_function
6f59f3a16697: adaptations due to trac 17130
rwst commented 9 years ago

Dependencies: #17130

fchapoton commented 9 years ago
comment:33

rebased on latest 6.5.beta3

also added one missing trac role


New commits:

202b202Merge branch 'u/rws/implement_symbolic_lower_incomplete_gamma_function' of trac.sagemath.org:sage into 6.5.b3
fchapoton commented 9 years ago

Changed commit from 6f59f3a to 202b202

fchapoton commented 9 years ago

Changed branch from u/rws/implement_symbolic_lower_incomplete_gamma_function to public/ticket/16697

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 9 years ago

Changed commit from 202b202 to f8fcbc2

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 9 years ago

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

f8fcbc2trac #16697 one more trac role missing was added
kcrisman commented 9 years ago
comment:35

Frédéric, any comments on review, or was this just a drive-by improvement?

Does the following (from the description) now come out better?

sage: hypergeometric([1],[b],x).simplify_hypergeometric()

Could we add the gamma(a,0,x) interface for the lower gamma? Should we?

rwst commented 9 years ago
comment:36

Replying to @kcrisman:

Does the following (from the description) now come out better?

sage: hypergeometric([1],[b],x).simplify_hypergeometric()

(b - 1)*x^(-b + 1)*e^x*gamma_inc_lower(b - 1, x)

Could we add the gamma(a,0,x) interface for the lower gamma? Should we?

A separate ticket, please?

kcrisman commented 9 years ago
comment:37
sage: hypergeometric([1],[b],x).simplify_hypergeometric()

(b - 1)*x^(-b + 1)*e^x*gamma_inc_lower(b - 1, x)

Nice.

Could we add the gamma(a,0,x) interface for the lower gamma? Should we?

A separate ticket, please?

Seems quite reasonable. Do you agree this would be a useful addition? I only mention it because it seems that at least a couple 'competitors' support it, so presumably it is somewhat of a standard and not a surprise if Sage implements it.

rwst commented 9 years ago
comment:38

Replying to @kcrisman:

Could we add the gamma(a,0,x) interface for the lower gamma? Should we?

A separate ticket, please?

Seems quite reasonable. Do you agree this would be a useful addition? I only mention it because it seems that at least a couple 'competitors' support it, so presumably it is somewhat of a standard and not a surprise if Sage implements it.

Any UI change that reduces surprise is fine with me. (Actually, I have a GUI development history and have read and care much about this subject)

kcrisman commented 9 years ago
comment:39

For the record, I know nearly nothing about UI development! It was only a question. #17722

I'm going to hold off on looking at this in more detail in case chapoton already did but a brief glance looks good.

kcrisman commented 9 years ago
comment:40

Note sympy apparently implements this as lowergamma (?).