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

testing limit of n*tan(pi/n) results in incorrect answer in 1.7rc1+ #23319

Open
chrisjohgorman opened this issue Apr 5, 2022 · 6 comments
Open
Labels

Comments

@chrisjohgorman
Copy link

from future import division
from sympy import *
x, y, z, t = symbols('x y z t')
k, m, n = symbols('k m n', integer=True)
f, g, h = symbols('f g h', cls=Function)
init_printing()
A_n = n*tan(pi/n)
limit(A_n, n, oo)

The result should be pi, but sympy returns 0 in versions later than 1.6.2. I used maxima and live.sympy.org to confirm the correct answer.

A git bisect between 1.6.2 and 1.7rc1 returns '[267a4fd] Using is_meromorphic() for limit evaluations' as the commit which causes the break.

Let me know if you need more information on this.

skirpichev added a commit to skirpichev/diofant that referenced this issue Apr 5, 2022
@anutosh491
Copy link
Member

Is it happening that way , I get the correct result through current master

>>> limit(x*tan(pi/x) , x, oo)
pi

@dhananjaypai08
Copy link

When integer set to true it does give us 0 else it doesn't

@anutosh491
Copy link
Member

anutosh491 commented Apr 5, 2022

When integer set to true it does give us 0 else it doesn't

Okay I'll look into the error here , thanks for mentioning this !
EDIT1: What goes wrong here is some intermediate evaluations are taking place . Consider this

>>> n = Symbol('n', integer=True)
>>> n*tan(pi/n)
n*tan(pi/n)
>>> _.subs(n, 1/n)
0

The error happening here is that tan(pi*n) evaluates to 0 when n is integer , hence what is being computed is n*0 , giving us back 0.
This intermediate evaluation on line 277 of limits.py should be taken care of !!

@jksuom
Copy link
Member

jksuom commented Apr 6, 2022

It seems that discrete (integer) variables like n should be replaced by continuous (preferably real) variables when computing limits. Maybe something like this:

--- a/sympy/series/limits.py
+++ b/sympy/series/limits.py
@@ -216,6 +216,12 @@ def doit(self, **hints):
             raise NotImplementedError("Limits at complex "
                                     "infinity are not implemented")
 
+        if z.is_integer:
+            from sympy.core import Dummy
+            x = Dummy('x', real=True)
+            e = e.subs(z, x)
+            z = x
+
         if hints.get('deep', True):
             e = e.doit(**hints)
             z = z.doit(**hints)

@anutosh491
Copy link
Member

anutosh491 commented Apr 6, 2022

It seems that discrete (integer) variables like n should be replaced by continuous (preferably real) variables when computing limits. Maybe something like this:

True , I agree with this logic, someone should run the series tests to confirm this . Maybe we could find some failing tests specially in test_limitseq.py as that mainly deals with limits where we use integer variables or something like n = Symbol('n', integer=True)

@chrisjohgorman
Copy link
Author

chrisjohgorman commented Apr 7, 2022 via email

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

No branches or pull requests

4 participants