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: Fixing incorrect limit evaluations caused due to bug in rewriting #19297

Merged
merged 2 commits into from May 12, 2020

Conversation

sachin-4099
Copy link
Contributor

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

Fixes: #18378
Fixes: #18482
Closes #18947

Brief description of what is fixed or changed

Incorrect limit evaluation takes place because of bug in rewriting.

The function xreplace() in the rewrite() function of gruntz.py is not substituting properly.
As a result, it has been replaced by the subs() function which resolves the issue.

Other Comments

Regression Tests have been added.

Release Notes

  • series
    • Replaces xreplace() with subs() in rewrite() function of gruntz.py resolving incorrect limit evaluations

@sympy-bot
Copy link

sympy-bot commented May 12, 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
    • Replaces xreplace() with subs() in rewrite() function of gruntz.py resolving incorrect limit evaluations (#19297 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: #18378
Fixes: #18482 
Closes #18947

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

**Incorrect limit evaluation** takes place because of **bug in rewriting.**

The function `xreplace()` in the `rewrite()` function of `gruntz.py` is not substituting properly.
As a result, it has been replaced by the `subs()` function which resolves the issue.

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


#### Release Notes


<!-- BEGIN RELEASE NOTES -->
* series
  * Replaces `xreplace()` with `subs()` in `rewrite() function of gruntz.py` resolving incorrect limit evaluations 
<!-- END RELEASE NOTES -->

Update

The release notes on the wiki have been updated.

@codecov
Copy link

codecov bot commented May 12, 2020

Codecov Report

Merging #19297 into master will decrease coverage by 0.010%.
The diff coverage is 100.000%.

@@              Coverage Diff              @@
##            master    #19297       +/-   ##
=============================================
- Coverage   75.595%   75.585%   -0.011%     
=============================================
  Files          651       651               
  Lines       169545    169545               
  Branches     40016     40016               
=============================================
- Hits        128168    128151       -17     
- Misses       35762     35780       +18     
+ Partials      5615      5614        -1     

@@ -652,10 +652,18 @@ def test_issue_18306():
assert limit(sin(sqrt(x))/sqrt(sin(x)), x, 0, '+') == 1


def test_issue_18378():
assert limit(log(exp(3*x) + x)/log(exp(x) + x**100), x, oo).doit() == 3
Copy link
Member

@jksuom jksuom May 12, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why doit()?

Copy link
Contributor Author

@sachin-4099 sachin-4099 May 12, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah actually not needed. Will change it asap. Anything else?

@jksuom
Copy link
Member

jksuom commented May 12, 2020

Looks good. xreplace does not find all that should be replaced.

@jksuom jksuom merged commit fc2b1a5 into sympy:master May 12, 2020
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants