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

[GSoC] Series: Correcting incorrect limit evaluations based on different assumptions of the limit variable #19292

Merged
merged 4 commits into from May 12, 2020

Conversation

sachin-4099
Copy link
Contributor

@sachin-4099 sachin-4099 commented May 11, 2020

Fixes: #8481
Fixes: #9041
Fixes: #14556
Fixes: #16722
Fixes: #17792
Fixes: #18992

Brief description of what is fixed or changed

Incorrect limit evaluation takes place based on different assumptions of the variable in the limit expression.

The assumption integer = True is causing these issues.

As we know, Gruntz algorithm changes a variable to a dummy with positive=True if the variable does not possess that property. We can make it define a dummy, if the limit variable has integer=True property.
So, Gruntz algorithm would not need to work with variables having integer=True property and thus this issue won’t occur.
The rewrite line can come later, after the dummy variable has been substituted.

Now the limit evaluations take place correctly:

In [6]: b = Symbol('b', positive=True, integer=True)
In [7]: limit(factorial(b+1)**(1/(b+1)) - factorial(b)**(1/b), b, oo)
Out[7]:  
 -1
e

In [8]: n = Symbol('n',positive=True,integer=True)

In [9]: limit(factorial(n)/sqrt(n)*(E/n)**n,n,oo)
Out[9]:
  ___   ____
\/ 2 *\/ pi


In [10]: z = symbols('z', positive=True)

In [11]: limit(binomial(n + z, n)*n**-z, n, oo)
Out[11]: 1/gamma(z + 1)

In [12]: z = symbols('z', positive=True, integer=True)

In [13]: limit(binomial(n + z, n)*n**-z, n, oo)
Out[13]: 1/gamma(z + 1)

Other Comments

Regression Tests have been added.

Release Notes

  • series
    • Adds a condition to limitinf() function of gruntz.py resolving incorrect limit evaluations

@sympy-bot
Copy link

sympy-bot commented May 11, 2020

Hi, I am the SymPy bot (v158). I'm here to help you write a release notes entry. Please read the guide on how to write release notes.

Your release notes are in good order.

Here is what the release notes will look like:

  • series
    • Adds a condition to limitinf() function of gruntz.py resolving incorrect limit evaluations (#19292 by @sachin-4099)

This will be added to https://github.com/sympy/sympy/wiki/Release-Notes-for-1.7.

Note: This comment will be updated with the latest check if you edit the pull request. You need to reload the page to see it.

Click here to see the pull request description that was parsed.

Fixes: #14556
Fixes: #16722 
Fixes: #17792

#### Brief description of what is fixed or changed

**Incorrect limit evaluation** takes place based on **different assumptions** of the variable in the limit expression.

The assumption `integer = True` is causing these issues.

As we know, Gruntz algorithm changes a variable to a dummy with `positive=True` if the variable does not possess that property. We can make it define a dummy, if the limit variable has `integer=True` property. 
So, Gruntz algorithm would not need to work with variables having `integer=True` property and thus this issue won’t occur. 
The rewrite line can come later, after the dummy variable has been substituted.

Now the limit evaluations take place correctly:

```
In [6]: b = Symbol('b', positive=True, integer=True)
In [7]: limit(factorial(b+1)**(1/(b+1)) - factorial(b)**(1/b), b, oo)
Out[7]:  
 -1
e

In [8]: n = Symbol('n',positive=True,integer=True)

In [9]: limit(factorial(n)/sqrt(n)*(E/n)**n,n,oo)
Out[9]:
  ___   ____
\/ 2 *\/ pi


In [10]: z = symbols('z', positive=True)

In [11]: limit(binomial(n + z, n)*n**-z, n, oo)
Out[11]: 1/gamma(z + 1)

In [12]: z = symbols('z', positive=True, integer=True)

In [13]: limit(binomial(n + z, n)*n**-z, n, oo)
Out[13]: 1/gamma(z + 1)
```

#### Other Comments
Regression Tests have been added.


#### Release Notes


<!-- BEGIN RELEASE NOTES -->
* series
  * Adds a condition to `limitinf() function of gruntz.py` resolving incorrect limit evaluations 
<!-- END RELEASE NOTES -->

Update

The release notes on the wiki have been updated.

@sachin-4099 sachin-4099 changed the title Series: Correcting incorrect limit evaluations based on different assumptions of the limit variable [GSoC] Series: Correcting incorrect limit evaluations based on different assumptions of the limit variable May 11, 2020
@sachin-4099 sachin-4099 requested a review from jksuom May 11, 2020
@sachin-4099 sachin-4099 requested a review from leosartaj May 11, 2020
@sachin-4099
Copy link
Contributor Author

sachin-4099 commented May 11, 2020

Please let me know if this is a similar issue: #9252

@codecov
Copy link

codecov bot commented May 11, 2020

Codecov Report

Merging #19292 into master will decrease coverage by 0.020%.
The diff coverage is 100.000%.

@@              Coverage Diff              @@
##            master    #19292       +/-   ##
=============================================
- Coverage   75.593%   75.573%   -0.021%     
=============================================
  Files          651       651               
  Lines       169545    169545               
  Branches     40016     40016               
=============================================
- Hits        128165    128131       -34     
- Misses       35762     35795       +33     
- Partials      5618      5619        +1     

sympy/series/gruntz.py Outdated Show resolved Hide resolved
@jksuom
Copy link
Member

jksuom commented May 11, 2020

This looks good. Gruntz's algorithm deals with functions of a continuous variable.

sympy/series/gruntz.py Outdated Show resolved Hide resolved
@sachin-4099
Copy link
Contributor Author

sachin-4099 commented May 11, 2020

Please let me know if this is a similar issue: #9252

What about this? Ping @jksuom

@jksuom
Copy link
Member

jksuom commented May 11, 2020

That looks like a bug. The result should depend on c.

@sachin-4099
Copy link
Contributor Author

sachin-4099 commented May 11, 2020

That looks like a bug. The result should depend on c.

Alright.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment