Skip to content

[Issue1000] Changing definition of limits #219

Closed
wants to merge 11 commits into from

4 participants

@hector1618

http://code.google.com/p/sympy/issues/detail?id=1000

In this pull request, I made elementary changes where it only checks whether the right limit and left limit is same or not. This changes the definition of limit in SymPy ( limit exists iff limit_right == limit_left ) and hence for lots of expressions where limit used to exists before, doesn't exist now.

Although I have changed test in test_limits, because of previous definition of limits, I need to modify following test files also -

  • sympy/functions/elementary/tests/test_interface.py[3]

    test_function_series1 E

  • sympy/integrals/tests/test_integrals.py[56]

    test_transform E

  • sympy/series/tests/test_demidovich.py[17]

    test_Limits_simple_3b F

  • sympy/series/tests/test_nseries.py[64]

    test_exp_1 E

  • sympy/solvers/tests/test_solvers.py[18]

    test_tsolve_1 F

  • sympy/test_external/test_autowrap.py[10]

    test_wrap_twice_c_cython E

If people agree on changing this definition of limit, I can proceed further for other changes.

hector1618 added some commits Apr 12, 2011
@hector1618 hector1618 Limit from both directions (Issue1000)
Changed default direction in limit from '+' to 'r'.
'r' stands for real and value of limit exists only when value of right side limit equals left side limit.
b4b845b
@hector1618 hector1618 Changed test files in series/tests/test_limits.py
Since the defination of limits has changed, we need to change some of the tests.
test from other test-files needs to be changed.
71cb79a
@hector1618 hector1618 Stripped white space 6c4e47a
@hector1618

Among the above errors, last two were present in 'master' itself.

@rlamy
SymPy member

limit should never return a string. You need to raise some exception here.

That make more sense.
Can you suggest me which type of error should be raised here?

@hector1618 hector1618 Included an error msg where limit is not defined.
In limit.py, replaced return 'limit does not exist' by an error msg.
9f2222f
@hector1618

@rlamy : I used PoleError. Hope it is fine.

hector1618 added some commits Apr 13, 2011
@hector1618 hector1618 Stripped whitespace 9efa624
@hector1618 hector1618 Changed limit to limit_eval and introduced limit.
Changed default function limit to limit_eval so that new defination of limit can be used with function limit.
2278b38
@hector1618 hector1618 Tests in sympy/series/tests/tests_limits.py
Defined new tests and modified existing tests accourding to modified defination of limits.
i.e. limit exists if and only if limit_right == limit_left.
3ac2fa8
@hector1618 hector1618 Modified test in test_interface.py
Modfied test in sympy/functions/elementary/tests/test_interface.py
    assert limit(my_function(x)/x, x, 0) == 1
is replaced by
    assert limit(my_function(x)/x, x, 0,dir="+") == 1.
efb51f4
@hector1618 hector1618 Modified function in integrals.py
Modified the function calc_limit(a,b) in last line.

    limit(sign(b)*inverse_mapping, x, a)

is replaced by

    limit(sign(b)*inverse_mapping, x, a, dir ="+")
c610376
@hector1618 hector1618 Modified test file in test_demidovich.py
Modified test - test_Limits_simple_3b() in sympy/series/tests/test_demidovich.py.
With modifited definition of limit, following limit does not exist.
    limit((sqrt3(x**2)-2*sqrt3(x)+1)/(x-1)**2,x,1)
affecf3
@hector1618 hector1618 Modified functions in exponential.py
Modifed the function _eval_nseries(self, x, n). In this function,
    arg0 = limit(arg_series.removeO(), x, 0)
is replaced by
    arg0 = limit(arg_series.removeO(), x, 0,dir = "+")
a84f33e
@goodok goodok commented on the diff Apr 13, 2011
sympy/series/limits.py
msg = "Don't know how to calculate the limit(%s, %s, %s, dir=%s), sorry."
raise PoleError(msg % (e, z, z0, dir))
+def limit(e, z, z0, dir = 'r'):
+ """
+ Compute the limit of e(z) at the point z0.
+
+ z0 can be any expression, including oo and -oo.
+
+ For dir="+" (default) it calculates the limit from the right
@goodok
goodok added a note Apr 13, 2011

My preliminary remarks:
To be not confusing, since you have changed a function's definition, then docstring must be changed respective.
And I don't understand "r" name-token, (it mixed with right).

Why don't use more closer to definition [1] thelimit_onesided instead of limit_eval? Or may be in future, you are going to extend this function to operate with filters [2] ?

[1] http://en.wikipedia.org/wiki/One-sided_limit

[2] https://github.com/sympy/sympy/wiki/Infinities-and-Singularities

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

Should I replace "r" by "real"? Than in future, for complex variables I need to specify default argument as "complex".
Is it fine with these tokens?

@goodok
goodok commented Apr 13, 2011

I can't say exactly, I prefer to accent that (for complex too) this default value implicitly means "from every sides" or "for every filter" (in other words "without filter"), or means "standard definition", "common definition".

@goodok
goodok commented Apr 13, 2011

Might be "common". Or better use even None (assuming that in general dir is only one part of description limit procedure, and in common case it is undefined)

@goodok
goodok commented Apr 14, 2011

I used PoleError. Hope it is fine.

PoleError is not fine :)
Relying upon the Pole definition, in case of abs(x)/x it is not Pole.

You can define and then use LimitError. But I should also consider a "nan" variant.
In mail-list I just right now answer to you and Tom (in one letter). And there is this question among others.

@ness01 ness01 commented on the diff Apr 14, 2011
sympy/series/order.py
@@ -232,7 +233,7 @@ class Order(Expr):
return None
r = None
for s in common_symbols:
- l = limit(powsimp(self.expr/expr.expr, deep=True,\
+ l = limit_eval(powsimp(self.expr/expr.expr, deep=True,\
@ness01
ness01 added a note Apr 14, 2011

Why is this necessary?

@hector1618
hector1618 added a note Apr 14, 2011

Please have a look at limit.py in function limit ( line 184). The structure of code is from line 213 to line 223.
The main idea was to call limit function only ones and than calling limit_eval function twice, one for each direction. Hence in above code, if I had kept it limit instead of limit_eval it calls limit more than ones which increases iteration and in some cases wrong result. I remember the test failed at following result -

In [4] : limit(floor(sin(x)), x, 0,dir = '-')
Out[4]:  0

with limit in order.py instead of limit_eval.

@ness01
ness01 added a note Apr 14, 2011

I see. This fails because limits, series expansions, and order are by default one-sided. Since gruntz is decidedly one-sided, one-sided series expansions and orders have to be possible (and as far as I can tell two-sided orders are not implemented, yet). So I think your change is fine here.

I would suggest you make it clear that we need a one-sided limit. So in fact as suggested by others there should be no limit_eval, just limit which can be given a direction, and here it should be limit(.., x, dir="+"), with a comment saying that order are always from the right (for now).

[Note that I am not a 'core developer' so you may wish to wait for objections before plunging ahead and doing what I say.]

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

It seems that introducing limit_eval is very inefficient way of changing the definition of limit.
I am closing this pull request. The modified pull request is #230.

@hector1618 hector1618 closed this Apr 15, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.