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

Series/limit fails unless expression is simplified first. #9173

Open
mforbes opened this issue Mar 20, 2015 · 12 comments · Fixed by #20754 · May be fixed by #26544
Open

Series/limit fails unless expression is simplified first. #9173

mforbes opened this issue Mar 20, 2015 · 12 comments · Fixed by #20754 · May be fixed by #26544
Labels
Easy to Fix This is a good issue for new contributors. Feel free to work on this if no one else has already. series

Comments

@mforbes
Copy link

mforbes commented Mar 20, 2015

Consider the following:

>>> var('p_0 p_1 p_2 p_3 b_0 b_1 b_2');
>>> Q = (p_0 + (p_1 + (p_2 + p_3/y)/y)/y)/(1 + ((p_3/(b_0*y) + (b_0*p_2 - b_1*p_3)/b_0**2)/y + (b_0**2*p_1 - b_0*b_1*p_2 - p_3*(b_0*b_2 - b_1**2))/b_0**3)/y)
>>> Q.series(y, n=3)
y**2*(-b_0*p_2**2/p_3**2 + 2*b_1*p_2/p_3 + b_2 - b_1**2/b_0) + b_1*y + b_0 + O(y**3)
>>> Q.simplify().series(y, n=3)
b_2*y**2 + b_1*y + b_0 + O(y**3)

The y**2 coefficient should be b_2, but is computed to be something else unless the expression is simplified first. The issues seems to be with limit, and so might be related to #9075:

>>> Q.diff(y, y).limit(y, 0).simplify()
-72*b_0*p_2**2/p_3**2 + 144*b_1*p_2/p_3 + 2*b_2 - 72*b_1**2/b_0
>>> expr.diff(y, y).simplify().limit(y, 0)
2*b_2
@mforbes mforbes changed the title Series fails unless expression is simplified first. Series/limit fails unless expression is simplified first. Mar 20, 2015
@mforbes
Copy link
Author

mforbes commented Nov 20, 2015

Any update on why this might be happening? It still seems to be an issue.

@jksuom
Copy link
Member

jksuom commented Nov 20, 2015

It seems that there is a bug in the computation of the series expansion of 1/f when the order of f is negative (and that of 1/f is positive).

>>> f = 1/x**2 - 1/x
>>> (1/f).series(x, n=6)
x**2 + x**3 + x**4 + O(x**6)
>>> (1/f).series(x, n=7)
x**2 + x**3 + x**4 + x**5 + O(x**7)
>>> (1/f).series(x, n=8)
x**2 + x**3 + x**4 + x**5 + O(x**8)

It is caused by an incorrect loop count in Pow._eval_nseries (on this line).

skirpichev added a commit to diofant/diofant that referenced this issue Feb 20, 2016
skirpichev added a commit to diofant/diofant that referenced this issue Feb 21, 2016
skirpichev added a commit to skirpichev/diofant that referenced this issue Nov 2, 2016
    close sympy/sympy#3112 (MrvAsympt was added in diofant#6)
    close sympy/sympy#9173 (test was added in 5a510ac)
    close sympy/sympy#9808 (fixed in 09e539b)
    close sympy/sympy#9341 (fixed in af98a00)
    close sympy/sympy#9908 (fixed in cc3fa8d)
    close sympy/sympy#6171 (test added in d278031)
    close sympy/sympy#9276 (diagnose_imports.py removed in ab8c535)
    close sympy/sympy#10201 (fixed in 0d0fc5f)
    close sympy/sympy#9057 (test was added in 8290a0c)
    close sympy/sympy#11159 (test was added in ffb76cb)
    close sympy/sympy#2839 (new AST transformers are used, see diofant#278 and diofant#167)
    close sympy/sympy#11081 (see ed01e16 and bb92329)
    close sympy/sympy#10974 (see 73fc425)
    close sympy/sympy#10806 (test in 539929a)
    close sympy/sympy#10801 (test in 2fe3da5)
    close sympy/sympy#9549 (test in 88bdefa)
    close sympy/sympy#4231 (test was added in fb411d5)
    close sympy/sympy#8634 (see 2fcbb58)
    close sympy/sympy#8481 (see 1ef20d3)
    close sympy/sympy#9956 (fixed in a34735f)
    close sympy/sympy#9747 (see e117c60)
    close sympy/sympy#7853 (see 3e4fbed)
    close sympy/sympy#9634 (see 2be03f5)
    close sympy/sympy#8500 (fixed in diofant#104 and finally in diofant#316)
    close sympy/sympy#9192 (see 9bf622f)
    close sympy/sympy#7130 (see e068fa3)
    close sympy/sympy#8514 (see b2d543b)
    close sympy/sympy#9334 (see 90de625)
    close sympy/sympy#8229 (see 9755b89)
    close sympy/sympy#8061 (see 7054f06)
    close sympy/sympy#7872 (tested in diofant#6)
    close sympy/sympy#3496 (tested in test_log_symbolic)
    close sympy/sympy#2929 (see da7db7a)
    close sympy/sympy#8203 (oo is not a real, see diofant#36)
    close sympy/sympy#7649 (0 is imaginary since diofant#8)
    close sympy/sympy#7256 (fixed in c0a4549)
    close sympy/sympy#6783 (see cb28d63)
    close sympy/sympy#5662 (is_integer issue fixed in 6bfa9f8, there is no is_bounded anymore)
    close sympy/sympy#5295 (fixed with diofant#354)
    close sympy/sympy#4856 (we now have flake/pep tests)
    close sympy/sympy#4555 (flake8 enabled after diofant#214)
    close sympy/sympy#5773 (cmp_to_key removed after diofant#164 and c9acbf0)
    close sympy/sympy#5484 (see above)

    Added regression tests:
    from https://groups.google.com/forum/#!topic/sympy/LkTMQKC_BOw
    fixes sympy/sympy#8825 (probably via diofant#209)
    fixes sympy/sympy#8635
    fixes sympy/sympy#8157
    fixes sympy/sympy#7872
    fixes sympy/sympy#7599
    fixes sympy/sympy#6179
    fixes sympy/sympy#5415
    fixes sympy/sympy#2865
    fixes sympy/sympy#5907
    fixes sympy/sympy#11722

    Closes diofant#347
skirpichev added a commit to skirpichev/diofant that referenced this issue Nov 2, 2016
    close sympy/sympy#3112 (MrvAsympt was added in diofant#6)
    close sympy/sympy#9173 (test was added in 5a510ac)
    close sympy/sympy#9808 (fixed in 09e539b)
    close sympy/sympy#9341 (fixed in af98a00)
    close sympy/sympy#9908 (fixed in cc3fa8d)
    close sympy/sympy#6171 (test added in d278031)
    close sympy/sympy#9276 (diagnose_imports.py removed in ab8c535)
    close sympy/sympy#10201 (fixed in 0d0fc5f)
    close sympy/sympy#9057 (test was added in 8290a0c)
    close sympy/sympy#11159 (test was added in ffb76cb)
    close sympy/sympy#2839 (new AST transformers are used, see diofant#278 and diofant#167)
    close sympy/sympy#11081 (see ed01e16 and bb92329)
    close sympy/sympy#10974 (see 73fc425)
    close sympy/sympy#10806 (test in 539929a)
    close sympy/sympy#10801 (test in 2fe3da5)
    close sympy/sympy#9549 (test in 88bdefa)
    close sympy/sympy#4231 (test was added in fb411d5)
    close sympy/sympy#8634 (see 2fcbb58)
    close sympy/sympy#8481 (see 1ef20d3)
    close sympy/sympy#9956 (fixed in a34735f)
    close sympy/sympy#9747 (see e117c60)
    close sympy/sympy#7853 (see 3e4fbed)
    close sympy/sympy#9634 (see 2be03f5)
    close sympy/sympy#8500 (fixed in diofant#104 and finally in diofant#316)
    close sympy/sympy#9192 (see 9bf622f)
    close sympy/sympy#7130 (see e068fa3)
    close sympy/sympy#8514 (see b2d543b)
    close sympy/sympy#9334 (see 90de625)
    close sympy/sympy#8229 (see 9755b89)
    close sympy/sympy#8061 (see 7054f06)
    close sympy/sympy#7872 (tested in diofant#6)
    close sympy/sympy#3496 (tested in test_log_symbolic)
    close sympy/sympy#2929 (see da7db7a)
    close sympy/sympy#8203 (oo is not a real, see diofant#36)
    close sympy/sympy#7649 (0 is imaginary since diofant#8)
    close sympy/sympy#7256 (fixed in c0a4549)
    close sympy/sympy#6783 (see cb28d63)
    close sympy/sympy#5662 (is_integer issue fixed in 6bfa9f8, there is no is_bounded anymore)
    close sympy/sympy#5295 (fixed with diofant#354)
    close sympy/sympy#4856 (we now have flake/pep tests)
    close sympy/sympy#4555 (flake8 enabled after diofant#214)
    close sympy/sympy#5773 (cmp_to_key removed after diofant#164 and c9acbf0)
    close sympy/sympy#5484 (see above)

    Added regression tests:
    from https://groups.google.com/forum/#!topic/sympy/LkTMQKC_BOw
    fixes sympy/sympy#8825 (probably via diofant#209)
    fixes sympy/sympy#8635
    fixes sympy/sympy#8157
    fixes sympy/sympy#7872
    fixes sympy/sympy#7599
    fixes sympy/sympy#6179
    fixes sympy/sympy#5415
    fixes sympy/sympy#2865
    fixes sympy/sympy#5907
    fixes sympy/sympy#11722

    Closes diofant#347
@mohitacecode
Copy link
Contributor

I think this issue is resolved in the current master.
IMO it can be closed.

>>> var('p_0 p_1 p_2 p_3 b_0 b_1 b_2')
(p_0, p_1, p_2, p_3, b_0, b_1, b_2)
>>> Q = (p_0 + (p_1 + (p_2 + p_3/y)/y)/y)/(1 + ((p_3/(b_0*y) + (b_0*p_2 - b_1*p_3)/b_0**2)/y + (b_0**2*p_1 - b_0*b_1*p_2 - p_3*(b_0*b_2 - b_1**2))/b_0**3)/y)
>>> Q.series(y, n=3)
b_2*y**2 + b_1*y + b_0 + O(y**3)
>>> Q.simplify().series(y, n=3)
b_2*y**2 + b_1*y + b_0 + O(y**3)
>>> Q.diff(y, y).limit(y, 0).simplify()
2*b_2
>>> 

@sachin-4099
Copy link
Contributor

I can confirm @mohitacecode 's results. This only needs a test.

@sachin-4099 sachin-4099 added the Easy to Fix This is a good issue for new contributors. Feel free to work on this if no one else has already. label Jun 1, 2020
Shardul555 added a commit to Shardul555/sympy that referenced this issue Dec 26, 2020
Initially the issue was the failure of series/limit unless the expression is simplified first, but it is resoved in current master.
So I have added test case addressing this one.
@Shardul555
Copy link

Shardul555 commented Dec 29, 2020

I run the code for this expression in the terminal having installed sympy version 1.7.1 but here the coefficients of y**2 and y are not simplified to final answer. Anyone guide me regarding this issue whether it is done with some purpose or this should be fixed?

>>> from sympy import *
>>> from sympy.abc import y
>>> var('p_0 p_1 p_2 p_3 b_0 b_1 b_2')
(p_0, p_1, p_2, p_3, b_0, b_1, b_2)
>>> Q = (p_0 + (p_1 + (p_2 + p_3/y)/y)/y)/(1 + ((p_3/(b_0*y) + (b_0*p_2 - b_1*p_3)/b_0**2)/y + (b_0**2*p_1 - b_0*b_1*p_2 - p_3*(b_0*b_2 - b_1**2))/b_0**3)/y)
>>> Q.series(y, n=3)
y*(b_0*p_2/p_3 + b_0*(-p_2/p_3 + b_1/b_0)) + y**2*(b_0*p_1/p_3 + b_0*p_2*(-p_2/p_3 + b_1/b_0)/p_3 + b_0*(-p_1/p_3 + (p_2/p_3 - b_1/b_0)**2 + b_1*p_2/(b_0*p_3) + b_2/b_0 - b_1**2/b_0**2)) + b_0 + O(y**3)
>>> simplify(y*(b_0*p_2/p_3 + b_0*(-p_2/p_3 + b_1/b_0)))
b_1*y
>>> simplify(y**2*(b_0*p_1/p_3 + b_0*p_2*(-p_2/p_3 + b_1/b_0)/p_3 + b_0*(-p_1/p_3 + (p_2/p_3 - b_1/b_0)**2 + b_1*p_2/(b_0*p_3) + b_2/b_0 - b_1**2/b_0**2)))
b_2*y**2

@anutosh491
Copy link
Member

I came across this issue while trying to address something similar and I realize this now ends up giving an even more complicated result than before and even if we simplify first .... we don't get the correct result which is a worse condition than what we had before !

>>> Q = (p_0 + (p_1 + (p_2 + p_3/y)/y)/y)/(1 + ((p_3/(b_0*y) + (b_0*p_2 - b_1*p_3)/b_0**2)/y + (b_0**2*p_1 - b_0*b_1*p_2 - p_3*(b_0*b_2 - b_1**2))/b_0**3)/y)
>>> Q.series(y, n=3)
y*(b_0*p_2/p_3 + b_0*(-p_2/p_3 + b_1/b_0)) + y**2*(b_0*p_1/p_3 + b_0*p_2*(-p_2/p_3 + b_1/b_0)/p_3 + b_0*(-p_1/p_3 + (p_2/p_3 - b_1/b_0)**2 + b_1*p_2/(b_0*p_3) + (b_0*b_2 - b_1**2)/b_0**2)) + b_0 + O(y**3)
>>>
>>> Q.simplify().series(y, n=3)
y*(b_0*p_2/p_3 + b_0*(-p_2/p_3 + b_1/b_0)) + y**2*(b_0*p_1/p_3 + b_0*p_2*(-p_2/p_3 + b_1/b_0)/p_3 + b_0*(-p_1/p_3 + (p_2/p_3 - b_1/b_0)**2 + b_1*p_2/(b_0*p_3) + (b_0*b_2 - b_1**2)/b_0**2)) + b_0 + O(y**3)
>>>
>>> # Expected - b_2*y**2 + b_1*y + b_0 + O(y**3)
>>> 
>>> # Previously gave - y**2*(-b_0*p_2**2/p_3**2 + 2*b_1*p_2/p_3 + b_2 - b_1**2/b_0) + b_1*y + b_0 + O(y**3)

I guess this was closed without adding a test hence we haven't come across this before, hence I guess we should reopen this now . Feel free to correct me if I'm missing something here !

@anutosh491 anutosh491 reopened this Jun 27, 2022
@arhaque09
Copy link

arhaque09 commented Feb 4, 2024

I would like to work on this issue! Let me know if its available so I can get assigned!

@arhaque09
Copy link

Hi! I am working on adding tests right now!

@Qaaee123
Copy link

Hello, I saw this issue and I checked some of the other linked threads. In particular, I saw that in #20724 the issue was closed because it was solved elsewhere, but when testing locally on versions 1.8+, the issue still persisted. Locally, I edited expr.py to include the same proposed change as #20724, and it seemed to solve the issue. However, it has started to fail testcases and I am unsure about how to proceed. I would greatly appreciate any guidance.

@Qaaee123
Copy link

Hello, I managed to fix this bug by incorporating ratsimp and it passes all of the test_expr test cases. However, when tested with tan(x).series(x, 2, 6, "+"), the output is incorrect. Any guidance would be greatly appreciated.

>>> tan(x).series(x, 2, 6, "+")
-2 + tan(2) + (x - 2)*tan(2)**2 + (x - 2)**2*tan(2) + (x - 2)**2*tan(2)**3 + (x - 2)**3*tan(2)**4 + 4*(x - 2)**3*tan(2)**2/3 + (x - 2)**3/3 + 2*(x - 2)**4*tan(2)/3 + 5*(x - 2)**4*tan(2)**3/3 + (x - 2)**4*tan(2)**5 + (x - 2)**5*tan(2)**6 + 2*(x - 2)**5*tan(2)**4 + 17*(x - 2)**5*tan(2)**2/15 + 2*(x - 2)**5/15 + x + O((x - 2)**6, (x, 2))
>>> # correct output
tan(2) + (1 + tan(2)**2)*(x - 2) + (x - 2)**2*(tan(2)**3 + tan(2)) + (x - 2)**3*(1/3 + 4*tan(2)**2/3 + tan(2)**4) + (x - 2)**4*(tan(2)**5 + 5*tan(2)**3/3 + 2*tan(2)/3) + (x - 2)**5*(2/15 + 17*tan(2)**2/15 + 2*tan(2)**4 + tan(2)**6) + O((x - 2)**6, (x, 2))

@oscarbenjamin
Copy link
Contributor

I'm not sure that there is an issue here. The example comes out more complicated than it needs to but it is correct. If we want to canonicalise the outputs then we should really use ringseries.

@oscarbenjamin
Copy link
Contributor

I think best thing is just to add some test cases since they were not added before.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Easy to Fix This is a good issue for new contributors. Feel free to work on this if no one else has already. series
Projects
None yet
10 participants