Opened 8 years ago

Closed 6 years ago

# implement symbolic lower incomplete gamma function

Reported by: Owned by: Ralf Stephan major sage-7.2 symbolics gamma, incomplete, special, functions Karl-Dieter Crisman, Nils Bruin Ralf Stephan Buck Evan, Paul Masson N/A 157b268 157b268d4ac45de049089e633809f89d2c3ab84e #20185

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
```

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

### comment:1 Changed 8 years ago by Ralf Stephan

Cc: Karl-Dieter Crisman Nils Bruin added

### comment:2 Changed 8 years ago by Nils Bruin

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.
Last edited 8 years ago by Nils Bruin (previous) (diff)

### comment:3 Changed 8 years ago by Ralf Stephan

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

### comment:4 Changed 8 years ago by Ralf Stephan

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.

see comment

### comment:5 Changed 8 years ago by Ralf Stephan

Branch: → u/rws/implement_symbolic_lower_incomplete_gamma_function

### comment:6 Changed 8 years ago by git

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

 ​93c2f94 `16697: deprecate "incomplete_gamma"`

### comment:7 Changed 8 years ago by Ralf Stephan

Description: modified (diff)

### comment:8 Changed 8 years ago by git

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

 ​5c1dd67 `16697: implement gamma_inc_lower`

### comment:9 Changed 8 years ago by Ralf Stephan

Status: new → needs_review

### comment:10 Changed 8 years ago by git

Commit: 5c1dd67bfa51d7c871b055c3272a94af9fdb68f2 → d8d7ee29d7d0f8cf9f884c4403fe5c36cdf3336d

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

 ​d8d7ee2 `16697: use mpmath as default evalf algorithm`

### comment:11 follow-up:  12 Changed 8 years ago by Nils Bruin

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

### comment:12 in reply to:  11 ; follow-up:  13 Changed 8 years ago by Ralf Stephan

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?

### comment:13 in reply to:  12 Changed 8 years ago by Nils Bruin

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'
```

### comment:14 follow-up:  15 Changed 8 years ago by Ralf Stephan

That gives:

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

### comment:15 in reply to:  14 Changed 8 years ago by Nils Bruin

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

### comment:16 Changed 8 years ago by Nils Bruin

See #16785 for a more complete fix.

### comment:17 Changed 8 years ago by git

Commit: d8d7ee29d7d0f8cf9f884c4403fe5c36cdf3336d → a73c5e374b1ee694ba20af22654767cdf441c126

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

 ​862798b `Merge branch 'develop' into t/16697/implement_symbolic_lower_incomplete_gamma_function` ​705c34e `16697: fix a related bug` ​a73c5e3 `16697: fix previous patch and a doctest`

### comment:18 Changed 8 years ago by Ralf Stephan

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.

### comment:19 Changed 8 years ago by git

Commit: a73c5e374b1ee694ba20af22654767cdf441c126 → 1fc19e73cb83d39a17d2055ec9b661f1fb401dfd

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

 ​7558a7c `16697: revert fix in favor of merge of 16785` ​9c0f66d `trac #16785: improve differential operator translation to maxima` ​1fc19e7 `Merge branch 't/16785/ticket/16785' into t/16697/implement_symbolic_lower_incomplete_gamma_function`

### comment:20 Changed 8 years ago by Ralf Stephan

Merged in: → #16785

### comment:21 Changed 8 years ago by For batch modifications

Milestone: sage-6.3 → sage-6.4

### comment:22 Changed 8 years ago by Karl-Dieter Crisman

Merged in: #16785

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!

### comment:23 Changed 8 years ago by Ralf Stephan

So, is there anything missing?

### comment:24 Changed 8 years ago by Ralf Stephan

Authors: → Ralf Stephan

### comment:25 Changed 8 years ago by git

Commit: 1fc19e73cb83d39a17d2055ec9b661f1fb401dfd → 9033be83a52eaabe0c0f896193923a040837e0cf

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

 ​9033be8 `Merge branch 'develop' into t/16697/implement_symbolic_lower_incomplete_gamma_function`

### comment:26 Changed 8 years ago by git

Commit: 9033be83a52eaabe0c0f896193923a040837e0cf → 05195e75302bb598d61d724d8635b08cc1712846

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

 ​05195e7 `Merge branch 'develop' into t/16697/implement_symbolic_lower_incomplete_gamma_function`

### comment:27 Changed 8 years ago by Jeroen Demeyer

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

### comment:28 Changed 8 years ago by Karl-Dieter Crisman

```
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

```

### comment:29 Changed 8 years ago by Ralf Stephan

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

### comment:30 follow-up:  69 Changed 8 years ago by Ralf Stephan

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

### comment:31 Changed 8 years ago by git

Commit: 05195e75302bb598d61d724d8635b08cc1712846 → 6f59f3a6133b9654f349a1228a6bc9fe9c90c944

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

 ​6d10729 `Simplify numerical evaluation of BuiltinFunctions` ​b6e1ed4 `Merge remote-tracking branches 'origin/u/jdemeyer/ticket/17131' and 'origin/u/jdemeyer/ticket/17133' into ticket/17130` ​382f97a `Merge branch 'u/jdemeyer/ticket/17130' of trac.sagemath.org:sage into 6.5beta1` ​7265989 `17130: reviewer's patch: fix typo` ​c47dbd4 `Fix Trac #17328 again in a better way` ​a486db2 `Call the factorial() method of an Integer` ​9d3cbbd `Fix numerical noise` ​abab222 `Fix more numerical noise` ​a383a5b `Merge branch 'u/jdemeyer/ticket/17130' of trac.sagemath.org:sage into t/16697/implement_symbolic_lower_incomplete_gamma_function` ​6f59f3a `16697: adaptations due to trac 17130`

### comment:32 Changed 8 years ago by Ralf Stephan

Dependencies: → #17130

### comment:33 Changed 8 years ago by Frédéric Chapoton

Branch: u/rws/implement_symbolic_lower_incomplete_gamma_function → public/ticket/16697 6f59f3a6133b9654f349a1228a6bc9fe9c90c944 → 202b202c0fd19a4848997e7c18895a216d9ad57b

rebased on latest 6.5.beta3

also added one missing trac role

New commits:

 ​202b202 `Merge branch 'u/rws/implement_symbolic_lower_incomplete_gamma_function' of trac.sagemath.org:sage into 6.5.b3`

### comment:34 Changed 8 years ago by git

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

 ​f8fcbc2 `trac #16697 one more trac role missing was added`

### comment:35 follow-up:  36 Changed 8 years ago by Karl-Dieter Crisman

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?

### comment:36 in reply to:  35 ; follow-up:  37 Changed 8 years ago by Ralf Stephan

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?

### comment:37 in reply to:  36 ; follow-up:  38 Changed 8 years ago by Karl-Dieter Crisman

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

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.

### comment:38 in reply to:  37 Changed 8 years ago by Ralf Stephan

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

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)

### comment:39 Changed 8 years ago by Karl-Dieter Crisman

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.

### comment:40 Changed 8 years ago by Karl-Dieter Crisman

Note sympy apparently implements this as `lowergamma` (?).

### comment:41 Changed 8 years ago by Ralf Stephan

Milestone: sage-6.4 → sage-6.6 needs_review → needs_work

Thanks, I was completely unaware. I think I will use their `eval`. The `diff` cannot be used in completeness because we don't know about the Meijer G function (EDIT: #17970), but I can steal half of it.

Last edited 8 years ago by Ralf Stephan (previous) (diff)

### comment:42 Changed 8 years ago by git

Commit: f8fcbc25f545e412f6c1cf18d58e7f50f4e44cb3 → 5f4a7fbd8598ef3fb22d8cb61d199dfffea23a8c

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

 ​9cc532c `Merge branch 'develop' into t/16697/public/ticket/16697` ​5f4a7fb `16697: eval using sympy; diff; doctests`

### comment:43 Changed 8 years ago by Ralf Stephan

Dependencies: #17130 → #17130, sympy-0.7.7 sage-6.6 → sage-pending needs_work → needs_review

Three doctests fail now because we depend on `sympy.erf._sage_()` which will be in Sympy-0.7.7. So this is pending.

### comment:44 follow-up:  45 Changed 8 years ago by Buck Evan

Is there a ticket for sympy-0.7.7?

### comment:45 in reply to:  44 Changed 8 years ago by Ralf Stephan

Is there a ticket for sympy-0.7.7?

There will be as soon it is released!

### comment:46 Changed 8 years ago by Ralf Stephan

The diff link of this ticket shows not the correct diff but if you `git trac checkout 16697` you get the right branch.

### comment:47 follow-up:  48 Changed 8 years ago by Buck Evan

I tried to review this (per ticket:18210#comment:18) but quickly got confused. (This would be a lot easier on github.)

1. Could one of you point me to the doc pertaining to `_sage_`? I did look, but the special sage functions page doesn't mention it.
2. There are some doctests regarding precision that have been obliterated. Was this intentional? Why?
3. It seems like this new function (gamma_inc_lower) would be deprecated as soon as #17722 is implemented. Don't we want to take a minute to figure out a design that works for both issues?

I think if this patch implemented lower-gamma as a special case of a single general gamma function we would end up with less if statements / edge cases / opportunities for bugs.

Edit: ... end up with *less* bugs

Last edited 8 years ago by Buck Evan (previous) (diff)

### comment:48 in reply to:  47 Changed 8 years ago by Ralf Stephan

1. Could one of you point me to the doc pertaining to `_sage_`?

https://github.com/sagemath/sage/blob/master/src/sage/structure/sage_object.pyx#L557 This is overridden where necessary so please see the resp. class. Probably you want to know about interfaces: https://github.com/sagemath/sage/blob/master/src/sage/interfaces/interface.py#L803 http://sagemath.org/doc/reference/interfaces/index.html

1. There are some doctests regarding precision that have been obliterated. Was this intentional? Why?

Oh that's true, I don't know why that got ditched.

1. It seems like this new function (gamma_inc_lower) would be deprecated as soon as #17722 is implemented. Don't we want to take a minute to figure out a design that works for both issues?

There should be no problem to support both interfaces, and without code duplication. We already have

```sage: incomplete_gamma(1,2)
e^(-2)
sage: gamma(1,2)
e^(-2)
sage: gamma(2)
1
```

I think if this patch implemented lower-gamma as a special case of a single general gamma function we would end up with less if statements / edge cases / opportunities for bugs.

Well that's the task for #17722, right. To have many doctests will help a good deal.

Note that even if you review this ticket (please add your real name to the ticket head in case) we would need to wait for Sympy-0.7.7 to be released. But a review of the code and the math would leave only a `make ptestlong` to finish it all.

EDIT: typos

Last edited 8 years ago by Ralf Stephan (previous) (diff)

### comment:49 Changed 8 years ago by Buck Evan

Reviewers: → Buck Evan needs_review → needs_work

### comment:50 Changed 7 years ago by Buck Evan

I left this as "needs work" because of your "Oh that's true, I don't know why that got ditched."

Could you merge this up with master and I'll attempt to review again?

Also, please show me how to run all the relevant tests, please?

I'm surprised sympy hasn't cut a release yet, still. If I'm counting correctly, it's been eight months since their last release.

### comment:51 Changed 7 years ago by Buck Evan

Merged and attempted to run the doctests, and got this result:

```./sage -t src/sage/functions/other.py                                                                    ⬆ ✱ ◼
too few successful tests, not using stored timings
Running doctests with ID 2015-07-26-17-39-51-10ac5315.
Git branch: master
Using --optional=mpir,python2,sage,scons
Doctesting 1 file.
sage -t src/sage/functions/other.py
**********************************************************************
File "src/sage/functions/other.py", line 1076, in sage.functions.other.Function_gamma_inc_lower._eval_
Failed example:
gamma_inc_lower(1/2,2)
Exception raised:
Traceback (most recent call last):
File "/home/buck/trees/mine/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 496, in _run
self.compile_and_execute(example, compiler, test.globs)
File "/home/buck/trees/mine/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 858, in compile_and_execute
exec(compiled, globs)
File "<doctest sage.functions.other.Function_gamma_inc_lower._eval_[2]>", line 1, in <module>
gamma_inc_lower(Integer(1)/Integer(2),Integer(2))
File "sage/symbolic/function.pyx", line 994, in sage.symbolic.function.BuiltinFunction.__call__ (/home/buck/trees/mine/sage/src/build/cythonized/sage/symbolic/function.cpp:10875)
res = super(BuiltinFunction, self).__call__(
File "sage/symbolic/function.pyx", line 507, in sage.symbolic.function.Function.__call__ (/home/buck/trees/mine/sage/src/build/cythonized/sage/symbolic/function.cpp:6887)
res = g_function_eval2(self._serial, (<Expression>args[0])._gobj,
File "sage/symbolic/pynac.pyx", line 185, in sage.symbolic.pynac.pyExpression_to_ex (/home/buck/trees/mine/sage/src/build/cythonized/sage/symbolic/pynac.cpp:3934)
t = ring.SR.coerce(res)
File "sage/structure/parent.pyx", line 1310, in sage.structure.parent.Parent.coerce (/home/buck/trees/mine/sage/src/build/cythonized/sage/structure/parent.c:10682)
cpdef coerce(self, x):
File "sage/structure/parent.pyx", line 1339, in sage.structure.parent.Parent.coerce (/home/buck/trees/mine/sage/src/build/cythonized/sage/structure/parent.c:10628)
return (<map.Map>mor)._call_(x)
File "sage/symbolic/ring.pyx", line 927, in sage.symbolic.ring.UnderscoreSageMorphism._call_ (/home/buck/trees/mine/sage/src/build/cythonized/sage/symbolic/ring.cpp:10428)
return self.codomain()(a._sage_())
File "/home/buck/trees/mine/sage/local/lib/python2.7/site-packages/sympy/core/mul.py", line 1467, in _sage_
s *= x._sage_()
AttributeError: 'erf' object has no attribute '_sage_'
**********************************************************************
File "src/sage/functions/other.py", line 1078, in sage.functions.other.Function_gamma_inc_lower._eval_
Failed example:
gamma_inc_lower(1/2,1)
Exception raised:
Traceback (most recent call last):
File "/home/buck/trees/mine/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 496, in _run
self.compile_and_execute(example, compiler, test.globs)
File "/home/buck/trees/mine/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 858, in compile_and_execute
exec(compiled, globs)
File "<doctest sage.functions.other.Function_gamma_inc_lower._eval_[3]>", line 1, in <module>
gamma_inc_lower(Integer(1)/Integer(2),Integer(1))
File "sage/symbolic/function.pyx", line 994, in sage.symbolic.function.BuiltinFunction.__call__ (/home/buck/trees/mine/sage/src/build/cythonized/sage/symbolic/function.cpp:10875)
res = super(BuiltinFunction, self).__call__(
File "sage/symbolic/function.pyx", line 507, in sage.symbolic.function.Function.__call__ (/home/buck/trees/mine/sage/src/build/cythonized/sage/symbolic/function.cpp:6887)
res = g_function_eval2(self._serial, (<Expression>args[0])._gobj,
File "sage/symbolic/pynac.pyx", line 185, in sage.symbolic.pynac.pyExpression_to_ex (/home/buck/trees/mine/sage/src/build/cythonized/sage/symbolic/pynac.cpp:3934)
t = ring.SR.coerce(res)
File "sage/structure/parent.pyx", line 1310, in sage.structure.parent.Parent.coerce (/home/buck/trees/mine/sage/src/build/cythonized/sage/structure/parent.c:10682)
cpdef coerce(self, x):
File "sage/structure/parent.pyx", line 1339, in sage.structure.parent.Parent.coerce (/home/buck/trees/mine/sage/src/build/cythonized/sage/structure/parent.c:10628)
return (<map.Map>mor)._call_(x)
File "sage/symbolic/ring.pyx", line 927, in sage.symbolic.ring.UnderscoreSageMorphism._call_ (/home/buck/trees/mine/sage/src/build/cythonized/sage/symbolic/ring.cpp:10428)
return self.codomain()(a._sage_())
File "/home/buck/trees/mine/sage/local/lib/python2.7/site-packages/sympy/core/mul.py", line 1467, in _sage_
s *= x._sage_()
AttributeError: 'erf' object has no attribute '_sage_'
**********************************************************************
File "src/sage/functions/other.py", line 1092, in sage.functions.other.Function_gamma_inc_lower._eval_
Failed example:
gamma_inc_lower(9/2,37/7)
Exception raised:
Traceback (most recent call last):
File "/home/buck/trees/mine/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 496, in _run
self.compile_and_execute(example, compiler, test.globs)
File "/home/buck/trees/mine/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 858, in compile_and_execute
exec(compiled, globs)
File "<doctest sage.functions.other.Function_gamma_inc_lower._eval_[10]>", line 1, in <module>
gamma_inc_lower(Integer(9)/Integer(2),Integer(37)/Integer(7))
File "sage/symbolic/function.pyx", line 994, in sage.symbolic.function.BuiltinFunction.__call__ (/home/buck/trees/mine/sage/src/build/cythonized/sage/symbolic/function.cpp:10875)
res = super(BuiltinFunction, self).__call__(
File "sage/symbolic/function.pyx", line 507, in sage.symbolic.function.Function.__call__ (/home/buck/trees/mine/sage/src/build/cythonized/sage/symbolic/function.cpp:6887)
res = g_function_eval2(self._serial, (<Expression>args[0])._gobj,
File "sage/symbolic/pynac.pyx", line 185, in sage.symbolic.pynac.pyExpression_to_ex (/home/buck/trees/mine/sage/src/build/cythonized/sage/symbolic/pynac.cpp:3934)
t = ring.SR.coerce(res)
File "sage/structure/parent.pyx", line 1310, in sage.structure.parent.Parent.coerce (/home/buck/trees/mine/sage/src/build/cythonized/sage/structure/parent.c:10682)
cpdef coerce(self, x):
File "sage/structure/parent.pyx", line 1339, in sage.structure.parent.Parent.coerce (/home/buck/trees/mine/sage/src/build/cythonized/sage/structure/parent.c:10628)
return (<map.Map>mor)._call_(x)
File "sage/symbolic/ring.pyx", line 927, in sage.symbolic.ring.UnderscoreSageMorphism._call_ (/home/buck/trees/mine/sage/src/build/cythonized/sage/symbolic/ring.cpp:10428)
return self.codomain()(a._sage_())
File "/home/buck/trees/mine/sage/local/lib/python2.7/site-packages/sympy/core/add.py", line 732, in _sage_
s += x._sage_()
File "/home/buck/trees/mine/sage/local/lib/python2.7/site-packages/sympy/core/mul.py", line 1467, in _sage_
s *= x._sage_()
AttributeError: 'erf' object has no attribute '_sage_'
**********************************************************************
3 of  12 in sage.functions.other.Function_gamma_inc_lower._eval_
[509 tests, 3 failures, 3.90 s]
----------------------------------------------------------------------
sage -t src/sage/functions/other.py  # 3 doctests failed
----------------------------------------------------------------------
Total time for all tests: 4.1 seconds
cpu time: 2.5 seconds
cumulative wall time: 3.9 seconds
```

### comment:52 Changed 7 years ago by Buck Evan

I realized I needed to upgrade sympy for this branch to work. After that, the functions.other doctests pass, but a full ptest shows some failures that seem unrelated, but it's hard for me to be sure. I've attached the full result.

ptest results

### comment:53 Changed 7 years ago by Buck Evan

There's something not quite right with this branch. I get `gamma_greek` if I use combine().

```sage: e
gamma(x, y) + gamma_inc_lower(x, y)
sage: e.combine()
gamma(x, y) + gamma_greek(x, y)
sage: e(x=1, y=2)
1
sage: e.combine()(x=1, y=2)
e^(-2) + gamma_greek(1, 2)
```

### comment:54 Changed 7 years ago by Ralf Stephan

Branch: public/ticket/16697 → u/rws/16697-1

### comment:55 Changed 7 years ago by Ralf Stephan

Commit: 5f4a7fbd8598ef3fb22d8cb61d199dfffea23a8c → f3be6f16908734f6c6e752578653a3ca09189011 #17130, sympy-0.7.7 → sympy-0.7.7 needs_work → needs_review

Cannot confirm your doctest failures (except those where sympy-0.7.7 is needed), nor the combine problem. It looks like you tested with an older Sage the doctests of a newer Sage.

I have squashed all commits into one, and uploaded a new branch based on Sage-6.8. In this branch the three missing doctests were added as well.

Really, a full test should only be done with sympy-0.7.7 but if you think the rest is fine(?) then the missing long test eventually is done quickly by the patchbot.

New commits:

 ​f3be6f1 `16697: implement symbolic lower incomplete gamma function`

### comment:56 follow-up:  57 Changed 7 years ago by Buck Evan

I did the full test with sympy-0.7.7, only after I got your new doctests in functions.other passing.

I reproduced the combine-gamma-greek problem with sage -br, which I would think should avoid any version-mismatch issues no? (Apparently the maxima writer forgot that uppercase gamma is also greek?)

That said, I can't reproduce it anymore after merging your branch and running another sage -br.

Should this not come out as gamma_inc_lower?

```./sage -br
sage: maxima('gamma_greek(x, y)')
gamma_greek(x,y)
```

I do see that `_sage_` of the same expression yields gamma_inc_lower. I suppose I don't know enough about the coersion going on here. I did try looking in the docs but didn't find anything that helped me understand this particular aspect.

```sage: maxima('gamma_greek(x, y)')
gamma_greek(x,y)

sage: maxima('gamma_greek(x, y)')._sage_()
gamma_inc_lower(x, y)

sage: maxima('gamma_greek(x, y)').simplify()
simplify(gamma_greek(x,y))

sage: maxima('gamma_greek(x, y)')._sage_().simplify()
gamma_inc_lower(x, y)
```

### comment:57 in reply to:  56 Changed 7 years ago by Ralf Stephan

I did the full test with sympy-0.7.7, only after I got your new doctests in functions.other passing.

How did you do that. Are there release tarballs of sympy-0.7.7? Note that having it on your machine does not mean Sage will use it. The version used is that in the upstream directory of Sage.

### comment:58 follow-up:  59 Changed 7 years ago by Buck Evan

I installed their master branch to the local/ tree. Putting local/bin on the \$PATH makes it close enough to a virtualenv that pip will Just Work.

I needed this to get the doctests working as well, so I presumed it was also your procedure. If not, how were you getting your tests to pass?

### comment:59 in reply to:  58 Changed 7 years ago by Ralf Stephan

I needed this to get the doctests working as well, so I presumed it was also your procedure. If not, how were you getting your tests to pass?

Never said I did. The ticket has pending and dependency set as well. But thanks anyway 8) So, except from a final test, would you say your review is positive?

### comment:60 Changed 7 years ago by Buck Evan

There's doctest coverage missing (for the deprecated function), and this patch doesn't fix the numerical instability in incomplete gamma as promised elsewhere.

```sage: gamma_inc_lower(25, 14.5).n()
-6.63538851954338e22
sage: gamma_inc_lower(25, 14.5).n(algorithm='mpmath')
-6.63538851954338e22
sage: gamma_inc_lower(25, 14.5).n(algorithm='pari')
-6.63538851954338e22
```

### comment:61 Changed 7 years ago by Buck Evan

Of note, gamma (even incomplete) is strictly positive in the reals; it's an integral of a strictly positive function.

### comment:62 Changed 7 years ago by Buck Evan

Wolfram gives the value of the same as 4.7e21, which seems correct.

### comment:63 follow-up:  66 Changed 7 years ago by Buck Evan

Actually, if I ask pari directly, it gets the correct value, so maybe the algorithm argument is broken?

```sage: pari('gamma(25) - incgam(25, 14.5)')
4.71197173824555 E21
sage: pari('incgamc(25, 14.5)')
4.71197173824555 E21
```

### comment:64 follow-up:  65 Changed 7 years ago by Buck Evan

Same story if I ask mpmath directly. I can't imagine what's wrong.

```sage: call(mpmath.gammainc, 25, 0, 14.5)
4.71197173824555e21
```

### comment:65 in reply to:  64 Changed 7 years ago by Ralf Stephan

Same story if I ask mpmath directly. I can't imagine what's wrong.

'algorithm' isn't propagated to `evalf` so we always get the buggy pari values. As we would have to switch the default anyway as soon as a later pari version fixes the bug, I'll now simply disable pari. We'll have to live with `mpmath`'s 53 bits for now.

### comment:66 in reply to:  63 Changed 7 years ago by Ralf Stephan

Actually, if I ask pari directly, it gets the correct value, so maybe the algorithm argument is broken?

```sage: pari('gamma(25) - incgam(25, 14.5)')
4.71197173824555 E21
sage: pari('incgamc(25, 14.5)')
4.71197173824555 E21
```

This works because it uses the gp interface (the libpari might be broken on our side). Ah, that's two different bugs. However, even if it is in our interface to libpari and we fix this, or if we use the gp interface, there is still the unfixed pari bug at http://pari.math.u-bordeaux.fr/cgi-bin/bugreport.cgi?bug=1689PARI/GP

### comment:67 Changed 7 years ago by git

Commit: f3be6f16908734f6c6e752578653a3ca09189011 → 86618379012a463b10f0d392f57403755300236a

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

 ​8661837 `16697: change algorithm; add, fix doctests`

### comment:68 Changed 7 years ago by Ralf Stephan

Actually the 53-bit restriction applies only to larger values, so all is not lost.

### comment:69 in reply to:  30 Changed 7 years ago by Ralf Stephan

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

The mpmath issue is now fixed as well so we have both options, mpmath or Pari/GP.

### comment:70 Changed 7 years ago by Fredrik Johansson

You could also compute the incomplete gamma function with Arb.

### comment:71 Changed 7 years ago by Ralf Stephan

Branch: u/rws/16697-1 → u/rws/16697-2

### comment:72 Changed 7 years ago by Ralf Stephan

Commit: 86618379012a463b10f0d392f57403755300236a → c6be42fd29966c4b824010c998820c37169c89c0 sympy-0.7.7 → #20185 sage-pending → sage-7.2

New commits:

 ​c6be42f `Merge branch 'u/rws/16697-1' of git://trac.sagemath.org/sage into i16697`

### comment:73 Changed 7 years ago by Frédéric Chapoton

Status: needs_review → needs_work

does not apply

Wanna review?

### comment:75 Changed 7 years ago by git

Commit: c6be42fd29966c4b824010c998820c37169c89c0 → 157b268d4ac45de049089e633809f89d2c3ab84e

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

 ​1f1c367 `Merge branch 'develop' into t/16697/16697-2` ​157b268 `remove some proposed but obsolete doctests`

### comment:76 Changed 7 years ago by Ralf Stephan

Status: needs_work → needs_review

### comment:77 Changed 6 years ago by Paul Masson

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?

### comment:78 Changed 6 years ago by Paul Masson

Reviewers: Buck Evan → Buck Evan, Paul Masson needs_review → positive_review

Thanks.

### comment:80 Changed 6 years ago by Volker Braun

Branch: u/rws/16697-2 → 157b268d4ac45de049089e633809f89d2c3ab84e → fixed positive_review → closed
Note: See TracTickets for help on using tickets.