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

implement symbolic lower incomplete gamma function #16697

Closed
rwst opened this issue Jul 22, 2014 · 98 comments
Closed

implement symbolic lower incomplete gamma function #16697

rwst opened this issue Jul 22, 2014 · 98 comments

Comments

@rwst
Copy link
Contributor

rwst commented Jul 22, 2014

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

@rwst rwst added this to the sage-6.3 milestone Jul 22, 2014
@nbruin
Copy link
Contributor

nbruin commented Jul 22, 2014

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
Copy link
Contributor Author

rwst commented Jul 22, 2014

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
Copy link
Contributor Author

rwst commented Jul 23, 2014

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
Copy link
Contributor Author

rwst commented Jul 23, 2014

Attachment: 16697-gammainc-alt-patch.gz

see comment

@rwst
Copy link
Contributor Author

rwst commented Jul 23, 2014

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 23, 2014

Commit: 93c2f94

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 23, 2014

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

93c2f9416697: deprecate "incomplete_gamma"

@rwst

This comment has been minimized.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 23, 2014

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

5c1dd6716697: implement gamma_inc_lower

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 23, 2014

Changed commit from 93c2f94 to 5c1dd67

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 26, 2014

Changed commit from 5c1dd67 to d8d7ee2

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 26, 2014

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

d8d7ee216697: use mpmath as default evalf algorithm

@nbruin
Copy link
Contributor

nbruin commented Aug 8, 2014

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
Copy link
Contributor Author

rwst commented Aug 8, 2014

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
Copy link
Contributor

nbruin commented Aug 8, 2014

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
Copy link
Contributor Author

rwst commented Aug 8, 2014

comment:14

That gives:

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

@nbruin
Copy link
Contributor

nbruin commented Aug 8, 2014

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
Copy link
Contributor

nbruin commented Aug 8, 2014

comment:16

See #16785 for a more complete fix.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 9, 2014

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 9, 2014

Changed commit from d8d7ee2 to a73c5e3

@rwst
Copy link
Contributor Author

rwst commented Aug 9, 2014

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.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 10, 2014

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 10, 2014

Changed commit from a73c5e3 to 1fc19e7

@rwst
Copy link
Contributor Author

rwst commented Aug 10, 2014

Merged: #16785

@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin removed this from the sage-6.3 milestone Aug 10, 2014
@fredrik-johansson
Copy link
Contributor

comment:70

You could also compute the incomplete gamma function with Arb.

@rwst
Copy link
Contributor Author

rwst commented Mar 14, 2016

Changed branch from u/rws/16697-1 to u/rws/16697-2

@rwst
Copy link
Contributor Author

rwst commented Mar 14, 2016

Changed commit from 8661837 to c6be42f

@rwst
Copy link
Contributor Author

rwst commented Mar 14, 2016

Changed dependencies from sympy-0.7.7 to #20185

@rwst
Copy link
Contributor Author

rwst commented Mar 14, 2016

New commits:

c6be42fMerge branch 'u/rws/16697-1' of git://trac.sagemath.org/sage into i16697

@rwst rwst added this to the sage-7.2 milestone Mar 14, 2016
@rwst rwst removed the pending label Mar 14, 2016
@fchapoton
Copy link
Contributor

comment:73

does not apply

@rwst
Copy link
Contributor Author

rwst commented May 14, 2016

comment:74

Wanna review?

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 20, 2016

Changed commit from c6be42f to 157b268

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 20, 2016

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

1f1c367Merge branch 'develop' into t/16697/16697-2
157b268remove some proposed but obsolete doctests

@paulmasson
Copy link
Mannequin

paulmasson mannequin commented Jun 26, 2016

comment:77

Current 7.3.beta5 Sage builds fine with these modifications. Doctests all pass. Documentation builds.

Random numeric tests accurate. Function plots. One curious difference from gamma_inc:

sage: gamma_inc_lower(a,x).diff(x)
x^(a - 1)*e^(-x)
sage: gamma_inc(a,x).diff(x)
D[1](gamma)(a, x)

Looks good to me. Sympy already updated: any other reason not to merge?

@paulmasson
Copy link
Mannequin

paulmasson mannequin commented Jun 26, 2016

Changed reviewer from Buck Evan to Buck Evan, Paul Masson

@rwst
Copy link
Contributor Author

rwst commented Jun 27, 2016

comment:79

Thanks.

@vbraun
Copy link
Member

vbraun commented Jun 27, 2016

Changed branch from u/rws/16697-2 to 157b268

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

7 participants