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

Limit Bug #20365

Closed
iammosespaulr opened this issue Oct 31, 2020 · 17 comments · Fixed by #21589
Closed

Limit Bug #20365

iammosespaulr opened this issue Oct 31, 2020 · 17 comments · Fixed by #21589
Labels

Comments

@iammosespaulr
Copy link
Contributor

Limit gives the wrong answer.

Screen Shot 2020-11-01 at 12 42 44 AM

>>> Limit(((x + 1)**(1/x) - E)/x, x, 0).doit()
0

This is the expr if anyone wants to dig deeper
image

skirpichev added a commit to skirpichev/diofant that referenced this issue Nov 1, 2020
@jksuom jksuom added the limits label Nov 1, 2020
@ABKor752
Copy link

ABKor752 commented Nov 3, 2020

I'm interested in working on this but I'm new. If I say I'd like to work on this, is it a binding contract?

@oscarbenjamin
Copy link
Contributor

No it's not a binding contract. You can try to work on it and if it turns out to be more difficult than you hoped then you are not committed in any way.

@dakshisdaksh
Copy link

I'll work on this issue!

@gopalamn
Copy link
Contributor

gopalamn commented Nov 7, 2020

Also looking into this. Let's see if we can bounce ideas @dakshisdaksh!

@ABKor752
Copy link

ABKor752 commented Nov 9, 2020

I'll try working on this too!

@sayandip18
Copy link
Contributor

sayandip18 commented Nov 25, 2020

Just to add, Sympy 1.6.1 gives -E/2 as the answer.

@sayandip18
Copy link
Contributor

This seems to be the result of a bug in gruntz.py and not in the doit() method

@sidhu1012
Copy link
Contributor

On current master it gives 0

@sayandip18
Copy link
Contributor

I tried to figure out where the bug was introduced, but I'm afraid I haven't progressed much. I ran SYMPY_DEBUG=True ./bin/isympy and these are the results I got.

On the current master:

limitinf(x*((1 + 1/x)**x - E), x) = 0
+-mrv_leadterm(_p*((1 + 1/_p)**_p - E), _p) = (0, 0)
| +-mrv(_p*((1 + 1/_p)**_p - E), _p) = ({_p: _Dummy_28}, {}, 0)
| | +-mrv((1 + 1/_p)**_p - E, _p) = ({}, {}, 0)
| | | +-mrv((1 + 1/_p)**_p, _p) = ({}, {}, E)
| | |   +-limitinf(1 + 1/_p, _p) = 1
| | |   | +-mrv_leadterm(1 + 1/_p, _p) = (1, 0)
| | |   | | +-mrv(1 + 1/_p, _p) = ({_p: _Dummy_24}, {}, 1 + 1/_Dummy_24)
| | |   | | | +-mrv(1/_p, _p) = ({_p: _Dummy_24}, {}, 1/_Dummy_24)
| | |   | | |   +-mrv(_p, _p) = ({_p: _Dummy_24}, {}, _Dummy_24)
| | |   | | +-rewrite(1 + 1/_Dummy_24, {exp(_p): _Dummy_24}, {}, _p, _w) = (_w + 1, -_p)
| | |   | | | +-sign(_p, _p) = 1
| | |   | | | +-limitinf(1, _p) = 1
| | |   | | +-calculate_series(_w + 1, _w) = 1
| | |   | +-sign(0, _p) = 0
| | |   | +-limitinf(1, _p, False) = 1
| | |   +-mrv(E, _p) = ({}, {}, E)
| | +-mrv(_p, _p) = ({_p: _Dummy_28}, {}, _Dummy_28)
| +-rewrite(0, {exp(_p): _Dummy_28}, {}, _p, _w) = (0, -_p)
| | +-sign(_p, _p) = 1
| | +-limitinf(1, _p) = 1
| +-calculate_series(0, _w) = 0
+-sign(0, _p) = 0
+-limitinf(0, _p, False) = 0

On version 1.6.1:

+-mrv_leadterm(_p*((1 + 1/_p)**_p - E), _p) = (-E/2, 0)
| +-mrv(_p*((1 + 1/_p)**_p - E), _p) = ({_p: _Dummy_188}, {}, _Dummy_188*(exp(_Dummy_188*log(1 + 1/_Dummy_188)) - E))
| | +-mrv((1 + 1/_p)**_p - E, _p) = ({_p: _Dummy_188}, {}, exp(_Dummy_188*log(1 + 1/_Dummy_188)) - E)
| | | +-mrv((1 + 1/_p)**_p, _p) = ({_p: _Dummy_188}, {}, exp(_Dummy_188*log(1 + 1/_Dummy_188)))
| | |   +-mrv(exp(_p*log(1 + 1/_p)), _p) = ({_p: _Dummy_188}, {}, exp(_Dummy_188*log(1 + 1/_Dummy_188)))
| | |     +-limitinf(_p*log(1 + 1/_p), _p) = 1
| | |     | +-mrv_leadterm(_p*log(1 + 1/_p), _p) = (1, 0)
| | |     | | +-mrv(_p*log(1 + 1/_p), _p) = ({_p: _Dummy_170}, {}, _Dummy_170*log(1 + 1/_Dummy_170))
| | |     | | | +-mrv(_p, _p) = ({_p: _Dummy_170}, {}, _Dummy_170)
| | |     | | | +-mrv(log(1 + 1/_p), _p) = ({_p: _Dummy_171}, {}, log(1 + 1/_Dummy_171))
| | |     | | |   +-mrv(1 + 1/_p, _p) = ({_p: _Dummy_171}, {}, 1 + 1/_Dummy_171)
| | |     | | |     +-mrv(1/_p, _p) = ({_p: _Dummy_171}, {}, 1/_Dummy_171)
| | |     | | |       +-mrv(_p, _p) = ({_p: _Dummy_171}, {}, _Dummy_171)
| | |     | | +-rewrite(_Dummy_170*log(1 + 1/_Dummy_170), {exp(_p): _Dummy_170}, {}, _p, _w) = (log(_w + 1)/_w, -_p)
| | |     | | | +-sign(_p, _p) = 1
| | |     | | | +-limitinf(1, _p) = 1
| | |     | | +-calculate_series(log(_w + 1)/_w, _w) = 1
| | |     | |   +-limitinf(_w, _w) = oo
| | |     | |   | +-mrv_leadterm(_w, _w) = (1, -1)
| | |     | |   | | +-mrv(_w, _w) = ({_w: _Dummy_175}, {}, _Dummy_175)
| | |     | |   | | +-rewrite(_Dummy_175, {exp(_w): _Dummy_175}, {}, _w, _w) = (1/_w, -_w)
| | |     | |   | | | +-sign(_w, _w) = 1
| | |     | |   | | | +-limitinf(1, _w) = 1
| | |     | |   | | +-calculate_series(1/_w, _w) = 1/_w
| | |     | |   | +-sign(-1, _w) = -1
| | |     | |   | +-sign(1, _w) = 1
| | |     | |   +-limitinf(_w**2, _w) = oo
| | |     | |   | +-mrv_leadterm(_w**2, _w) = (1, -2)
| | |     | |   | | +-mrv(_w**2, _w) = ({_w: _Dummy_180}, {}, _Dummy_180**2)
| | |     | |   | | | +-mrv(_w, _w) = ({_w: _Dummy_180}, {}, _Dummy_180)
| | |     | |   | | +-rewrite(_Dummy_180**2, {exp(_w): _Dummy_180}, {}, _w, _w) = (_w**(-2), -_w)
| | |     | |   | | | +-sign(_w, _w) = 1
| | |     | |   | | | +-limitinf(1, _w) = 1
| | |     | |   | | +-calculate_series(_w**(-2), _w) = _w**(-2)
| | |     | |   | +-sign(-2, _w) = -1
| | |     | |   | +-sign(1, _w) = 1
| | |     | |   +-limitinf(_w, _w) = oo
| | |     | |   +-limitinf(_w, _w) = oo
| | |     | |   +-limitinf(_w**2, _w) = oo
| | |     | +-sign(0, _p) = 0
| | |     | +-limitinf(1, _p, False) = 1
| | |     +-mrv(_p*log(1 + 1/_p), _p) = ({_p: _Dummy_188}, {}, _Dummy_188*log(1 + 1/_Dummy_188))
| | |       +-mrv(_p, _p) = ({_p: _Dummy_188}, {}, _Dummy_188)
| | |       +-mrv(log(1 + 1/_p), _p) = ({_p: _Dummy_189}, {}, log(1 + 1/_Dummy_189))
| | |         +-mrv(1 + 1/_p, _p) = ({_p: _Dummy_189}, {}, 1 + 1/_Dummy_189)
| | |           +-mrv(1/_p, _p) = ({_p: _Dummy_189}, {}, 1/_Dummy_189)
| | |             +-mrv(_p, _p) = ({_p: _Dummy_189}, {}, _Dummy_189)
| | +-mrv(_p, _p) = ({_p: _Dummy_190}, {}, _Dummy_190)
| +-rewrite(_Dummy_188*(exp(_Dummy_188*log(1 + 1/_Dummy_188)) - E), {exp(_p): _Dummy_188}, {}, _p, _w) = ((exp(log(_w + 1)/_w) - E)/_w, -_p)
| | +-sign(_p, _p) = 1
| | +-limitinf(1, _p) = 1
| +-calculate_series((exp(log(_w + 1)/_w) - E)/_w, _w) = -E/2
|   +-limitinf(_w, _w) = oo
|   | +-mrv_leadterm(_w, _w) = (1, -1)
|   | | +-mrv(_w, _w) = ({_w: _Dummy_194}, {}, _Dummy_194)
|   | | +-rewrite(_Dummy_194, {exp(_w): _Dummy_194}, {}, _w, _w) = (1/_w, -_w)
|   | | | +-sign(_w, _w) = 1
|   | | | +-limitinf(1, _w) = 1
|   | | +-calculate_series(1/_w, _w) = 1/_w
|   | +-sign(-1, _w) = -1
|   | +-sign(1, _w) = 1
|   +-limitinf(_w**2, _w) = oo
|   | +-mrv_leadterm(_w**2, _w) = (1, -2)
|   | | +-mrv(_w**2, _w) = ({_w: _Dummy_199}, {}, _Dummy_199**2)
|   | | | +-mrv(_w, _w) = ({_w: _Dummy_199}, {}, _Dummy_199)
|   | | +-rewrite(_Dummy_199**2, {exp(_w): _Dummy_199}, {}, _w, _w) = (_w**(-2), -_w)
|   | | | +-sign(_w, _w) = 1
|   | | | +-limitinf(1, _w) = 1
|   | | +-calculate_series(_w**(-2), _w) = _w**(-2)
|   | +-sign(-2, _w) = -1
|   | +-sign(1, _w) = 1
|   +-limitinf(_w, _w) = oo
|   +-limitinf(_w**2, _w) = oo
|   +-limitinf(_w, _w) = oo
|   +-limitinf(_w**2, _w) = oo
|   +-limitinf(_w**2, _w) = oo
|   +-limitinf(_w**(-2), _w) = 0
|   | +-mrv_leadterm(_w**(-2), _w) = (1, 2)
|   | | +-mrv(_w**(-2), _w) = ({_w: _Dummy_209}, {}, _Dummy_209**(-2))
|   | | | +-mrv(_w, _w) = ({_w: _Dummy_209}, {}, _Dummy_209)
|   | | +-rewrite(_Dummy_209**(-2), {exp(_w): _Dummy_209}, {}, _w, _w) = (_w**2, -_w)
|   | | | +-sign(_w, _w) = 1
|   | | | +-limitinf(1, _w) = 1
|   | | +-calculate_series(_w**2, _w) = _w**2
|   | +-sign(2, _w) = 1
|   +-limitinf(_w**(-2), _w) = 0
|   +-limitinf(1 - 1/(2*_w), _w) = 1
|   | +-mrv_leadterm(1 - 1/(2*_w), _w) = (1, 0)
|   | | +-mrv(1 - 1/(2*_w), _w) = ({_w: _Dummy_218}, {}, 1 - 1/(2*_Dummy_218))
|   | | | +-mrv(-1/(2*_w), _w) = ({_w: _Dummy_218}, {}, -1/(2*_Dummy_218))
|   | | |   +-mrv(1/_w, _w) = ({_w: _Dummy_218}, {}, 1/_Dummy_218)
|   | | |     +-mrv(_w, _w) = ({_w: _Dummy_218}, {}, _Dummy_218)
|   | | +-rewrite(1 - 1/(2*_Dummy_218), {exp(_w): _Dummy_218}, {}, _w, _w) = (1 - _w/2, -_w)
|   | | | +-sign(_w, _w) = 1
|   | | | +-limitinf(1, _w) = 1
|   | | +-calculate_series(1 - _w/2, _w) = 1
|   | |   +-limitinf(_w, _w) = oo
|   | |     +-mrv_leadterm(_w, _w) = (1, -1)
|   | |     | +-mrv(_w, _w) = ({_w: _Dummy_220}, {}, _Dummy_220)
|   | |     | +-rewrite(_Dummy_220, {exp(_w): _Dummy_220}, {}, _w, _w) = (1/_w, -_w)
|   | |     | | +-sign(_w, _w) = 1
|   | |     | | +-limitinf(1, _w) = 1
|   | |     | +-calculate_series(1/_w, _w) = 1/_w
|   | |     +-sign(-1, _w) = -1
|   | |     +-sign(1, _w) = 1
|   | +-sign(0, _w) = 0
|   | +-limitinf(1, _w, False) = 1
|   +-limitinf(_w**(-3), _w) = 0
|     +-mrv_leadterm(_w**(-3), _w) = (1, 3)
|     | +-mrv(_w**(-3), _w) = ({_w: _Dummy_226}, {}, _Dummy_226**(-3))
|     | | +-mrv(_w, _w) = ({_w: _Dummy_226}, {}, _Dummy_226)
|     | +-rewrite(_Dummy_226**(-3), {exp(_w): _Dummy_226}, {}, _w, _w) = (_w**3, -_w)
|     | | +-sign(_w, _w) = 1
|     | | +-limitinf(1, _w) = 1
|     | +-calculate_series(_w**3, _w) = _w**3
|     +-sign(3, _w) = 1
+-sign(0, _p) = 0
+-limitinf(-E/2, _p, False) = -E/2

@sayandip18
Copy link
Contributor

@oscarbenjamin What do you think?

@0sidharth
Copy link
Member

The following diff computes the correct limit and passes all tests with negligible increase in time (about 1 second more to run test_limits.py completely), but this is not the best workaround, as this doesn't precisely capture only the specific case of this limit, i.e. other limits like limit(1 + (1 + 1/x)**x, x, oo) also have mrv0 as E, but are currently computing correctly.

This change is not desirable (I believe) in those cases as this increases the number of recursive calls and hence the time taken to calculate the limit. However, I am not able to come up with a better method. Is this diff fine or will the change in recursive calls in a few specific cases cause some serious time issues and a better method should be thought of? Any help would be appreciated.

diff --git a/sympy/series/gruntz.py b/sympy/series/gruntz.py
index a3e27db2b5..5c2cc63d00 100644
--- a/sympy/series/gruntz.py
+++ b/sympy/series/gruntz.py
@@ -272,8 +272,9 @@ def mrv(e, x):
             return SubsSet(), b1
         if e1.has(x):
             base_lim = limitinf(b1, x)
-            if base_lim is S.One:
-                return mrv(exp(e1 * (b1 - 1)), x)
+            mrv0 = exp(e1 * (b1 - 1))
+            if base_lim is S.One and mrv0 is not exp(1):
+                return mrv(mrv0, x)
             return mrv(exp(e1 * log(b1)), x)
         else:
             s, expr = mrv(b1, x)

@0sidharth
Copy link
Member

@jksuom any comments on the above-mentioned method?

@jksuom
Copy link
Member

jksuom commented Jan 16, 2021

I think that gruntz is not needed for this limit as there is a power series expansion.

@sidhu1012
Copy link
Contributor

sidhu1012 commented Jan 16, 2021

I was thinking can't we use differentiation method for limits of form 0/0 and oo/oo ?

@jksuom
Copy link
Member

jksuom commented Jan 16, 2021

I'd prefer series expansions.

@0sidharth
Copy link
Member

@jksuom To me it seems to be more of a corner-case not being handled in the optimization introduced after 1.6.1 (specifically in 14a7d00 which caused the limit being output to change from -E/2 to 0), so it looks like a change in gruntz might be needed. Additionally, in the current implementation, _eval_nseries of Pow is not called when trying to find this limit. If we change according to the diff I mentioned, then the series expansion is called. In this case, won't the change be required?

@jksuom
Copy link
Member

jksuom commented Jan 18, 2021

I think that this should be handled by Limit.doit before calling gruntz.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants