Expr: _eval_interval checks both limits(left and right) #11889

karan1276 · 2016-11-30T15:01:40Z

Expr._eval_interval now evaluates right limit and left limit both

Before
>>> integrate(log(0.5-x), (x, 0, 0.5)) −0.846573590279973+1.0iπ

After:
>>> integrate(log(0.5-x), (x, 0, 0.5)) 799, in _eval_interval raise MathError("Right limit and left limit do not match")

Expr._eval_interval only evaluates right limit and left limit, checks if they are equal, gives error if theyare not equal, else behaves normaly.

jksuom · 2016-11-30T15:46:26Z

It does not matter if left and right limits differ at an end point, since only one of them is used by _eval_interval (and therefore no error message should be returned). It will suffice to compute the limit from the side where the interval lies. So, for example, the limit at a should be from the right if a < b and from the left if a > b, and similarly with a and b interchanged. (If a == b, then 0 is returned.)

karan1276 · 2016-11-30T16:08:04Z

@jksuom makes sense ill fix that.

This reverts commit ebc2cae.

karan1276 · 2016-11-30T16:23:15Z

@jksuom also could you please hint at why the tests are failing?

jksuom · 2016-11-30T17:08:18Z

They are failing because some evaluations now incorrectly return an error. That should not happen after the lines raising an error are removed.

Expr._eval_limit now compute the limit from the side where the interval lies. This is crutial since left and right limit need not be same.

karan1276 · 2016-11-30T18:03:33Z

@jksuom i have made changes, please review them :)

jksuom · 2016-11-30T18:25:01Z

sympy/core/expr.py

@@ -781,7 +781,11 @@ def _eval_interval(self, x, a, b):
        else:
            A = self.subs(x, a)
            if A.has(S.NaN, S.Infinity, S.NegativeInfinity, S.ComplexInfinity):
-                A = limit(self, x, a)
+                if a.__lt__(b):


if a < b: should be preferred. It would work even if a.__lt__() is not defined, but b.__gt__() is defined, in which case the interpreter will automatically use b.__gt__(a).

One could also write:

dir = '+' if a < b else '-' A = limit(self, x, a, dir=dir)

You need to guard against symbolic a < b. Not sure what to do in that case. Maybe just assume a < b by default?

Maybe just assume a < b by default?

I guess that is what should be done. So maybe if (a < b) is not False ?

@asmeurer, @jksuom silly question, i am still learning. what you mean guard against symbolic a < b ? if that has to do with symbols used for a and b (pi for example) shouldn't we modify __lt__ and __gt__ to check symbols as well? How does if (a < b) is not False do that?

Thanks

Yes, the typical way is if (a < b) == True (be careful, don't use is, since it gives sympy.true, not True). The issue is that if you just use if a < b then if the inequality is symbolic (can't be evaluated, like if a and b are symbols), then it will raise a TypeError.

In fact, you can see the TypeError in the failing tests.

karan1276 · 2016-12-01T13:09:46Z

@asmeurer , @jksuom I replaced lt() with <. Regarding guard against symbolic a < b, should i just use if (a < b) is not False ?

jksuom · 2016-12-02T15:12:10Z

If you use if a < b:, and the values are not fixed, then an exception is raised:

In [2]: if 0 < x:
   ...:     pass
   ...: 
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
/home/jks/sympy/<ipython-input-2-728352b32280> in <module>()
----> 1 if 0 < x:
      2     pass

/home/jks/sympy/sympy/core/relational.pyc in __nonzero__(self)
    193 
    194     def __nonzero__(self):
--> 195         raise TypeError("cannot determine truth value of Relational")
    196 
    197     __bool__ = __nonzero__

TypeError: cannot determine truth value of Relational

because 0 < x is neither True nor False until the value of x has been decided. (In fact, it is StrictGreaterThan(Symbol('x'), Integer(0)))

To avoid the error you can write:

In [3]: if (0 < x) is not False:
   ...:     pass
   ...: 
because

In [5]: (0 < x) is not False
Out[5]: True

One can also think that there are three truth values: true, false, and undecided.
In the general situation, (a < b) is not False: will always be either True or False. It is True, if a < b is true or undecided, and False only if a < b is false, which is probably the preferred solution.
On the other hand, (a < b) == True will be False also when a < b is undecided, which we may not want.

karan1276 · 2016-12-03T15:58:26Z

@jksuom thanks for explanation, i think it's ready for merge

jksuom · 2016-12-03T16:41:10Z

I wanted to point out that (a < b) is not False is better than (a < b) == True. There are two reasons: First, we should treat the case where a < b is undecided in the same way as if it were true, since that is how an interval of type (a, b) is usually implicitly understood. And then, is (or is not) is more efficient than == because the interpreter can see that quickly without having to run a SymPy method (namely Basic.__eq__). Hence I wish you would use the form I suggested.

jksuom · 2016-12-03T17:17:39Z

There could also be a line testing the case a == b immediately after it has been seen that not both a and b are None.

Also tests should be added to show that _eval_interval now works as expected (fixing the issue).

karan1276 · 2016-12-04T06:22:43Z

Humm, i may be wrong but here is what i think:

(a < b) is not False and (a < b) == True both guard against undecided a < b, since undecided == True will become False and if statements will not be executed.
at (this commit)[https://github.com/Expr: _eval_interval checks both limits(left and right) #11889/commits/d643660db4c567a10d88f2e29a43a40fbd2b12a4] Aaron Meurer mentions "Yes, the typical way is if (a < b) == True (be careful, don't use is, since it gives sympy.true, not True). The issue is that if you just use if a < b then if the inequality is symbolic (can't be evaluated, like if a and b are symbols), then it will raise a TypeError."
if a == b does _eval_interval ever give a wrong answer ?(just curious). If there is no case then we may not need to add that condition.

If you are sure (a < b) is not False is better approach then let me know, we will just go with that.

karan1276 · 2016-12-04T06:36:44Z

Okay one correction, if a == b, then _eval_interval does give false limit in cases when left and right limit are not same and a,b is that point at which limit is not defined, for example:
sympy.integrate(sympy.log(0.5-x), (x, 0.5, 0.5))
gives:
0
should raise an error.

jksuom · 2016-12-04T08:47:32Z

(a < b) is not False and (a < b) == True both guard against undecided a < b, since undecided == True will become False and if statements will not be executed.

I think we should have the undecided case treated in the same way as when a < b is true. That will apparently not happen if we use (a < b) == True. It looks like we could use (a < b) != False instead. That will also cover the case when a < b is S.false, in which case my earlier suggestion (using is not) will fail.

The tests at the two endpoints should be compatible in the undecided case. (So we should have + at a and - at b.) For example, exactly the same test could be used for both. (a < b also at b.)

sympy.integrate(sympy.log(0.5-x), (x, 0.5, 0.5))

It is true in general that integrals over a set of measure zero have value zero (even if the integrand is not finite). Therefore I think we should return zero.

karan1276 · 2016-12-04T09:47:41Z

It is true in general that integrals over a set of measure zero have value zero (even if the integrand is not finite). Therefore I think we should return zero.

Is it true even when the function(or it's limit) is not defined at that point?

Right, ill make chages

jksuom · 2016-12-04T10:27:48Z

Is it true even when the function(or it's limit) is not defined at that point?

If the function is not defined on a set of measure zero, then its value can be set arbitrarily. That will not affect the integral. In general, if two functions differ only on a set of measure zero, their integrals have the same value. (This is how the Lebesgue integral behaves, the Riemann integral is only defined under fairly restrictive conditions.)

karan1276 · 2016-12-18T07:08:01Z

all test seem to pass here but test_issue_4199() seems to fail on my personal computer when i run sympy.test("expr"), any clue of why that is happening?

karan1276 · 2016-12-18T07:10:59Z

Sorry that got resolved. The case a==b should be checked after verifying if both a and b are not null. I was checking it before.

jksuom · 2016-12-18T09:12:34Z

If the order of the limits cannot be decided, for example, if we integrate from a = 0 to b = L, where L is a symbol, then neither a < b nor b < a is True and both limits would be taken on the left hand side. It is probable that the user has intended that the limit at 0 be taken on the right and the limit at L on the left. (Otherwise he would have written the integral from L to 0, though we can only guess.) In any case, the conditions should be such that the limits are always taken on opposite sides.

It seems to me that the algorithm should be as follows: If it can be decided that b < a, then take the left limit at a and the right limit at b. Otherwise (i.e, it is False or undecided), take the right limit at a and the left limit at b.

This means that exactly the same condition should be used at both endpoints. It could be (b < a) == True, or equivalently (a < b) != False.

The case a == b should be checked before computing A and B, right after seeing that not both a and b are None.

karan1276 · 2016-12-19T14:46:40Z

@jksuom Yup, thats the algorithm we are using right now. Altho the exact same condition is not used, the logic remains the same because if and else blocks are swapped. Currently things work the same way you described in your algorithm.

Aldo i added a test, please review.

jksuom · 2016-12-19T16:50:28Z

sympy/core/expr.py

@@ -792,7 +800,11 @@ def _eval_interval(self, x, a, b):
        else:
            B = self.subs(x, b)
            if B.has(S.NaN, S.Infinity, S.NegativeInfinity, S.ComplexInfinity):
-                B = limit(self, x, b)
+                if (b < a) != False:


This should also be a < b (and then "-" in the limit call) to make sure that the limits are always taken on opposite sides.

jksuom · 2016-12-20T08:23:02Z

sympy/core/tests/test_expr.py

+
+def test_issue_11877():
+    x = symbols('x')
+    assert (long(integrate(log(0.5-x), (x, 0, 0.5))) == long(-0.846573590279973))


This test probably shows that there is no imaginary part, but otherwise it does not say much:

In [3]: long(-0.846573590279973) Out[3]: 0

It is usually best to use rational numbers in tests, since floating point comparisons may be problematic. In this case, integrate(log(S(1)/2 - x), (x, 0, S(1)/2)) could be used.

jksuom · 2016-12-20T08:24:24Z

This looks good otherwise, but the test should be improved.

karan1276 · 2016-12-24T12:09:10Z

humm, i am thinking of a simpler function to use as a test case, how about something like Abs(x)/-x ? we can check limit from 0 to 1

jksuom · 2016-12-24T13:28:51Z

I think this would be the proper test for the issue:

In [3]: integrate(log(S(1)/2 - x), (x, 0, S(1)/2)) == -S(1)/2 -log(2)/2
Out[3]: True

karan1276 · 2016-12-24T17:10:42Z

sympy/core/tests/test_expr.py

@@ -1768,4 +1768,5 @@ def test_issue_10755():

 def test_issue_11877():
    x = symbols('x')
-    assert (long(integrate(log(0.5-x), (x, 0, 0.5))) == long(-0.846573590279973))
+    y = integrate(x**2/(-x*x), (x, 0, 1))


@jksuom i changed the test to one you suggest, but this one is a little shorter and easy to understand should we rather use this test?

Not that it really matters but just saying.

jksuom · 2016-12-25T13:30:05Z

I think this looks good. The error must be unrelated to the changes. I'll try to investigate.

jksuom · 2016-12-25T13:39:56Z

#11980 should fix the error. I'll restart the failed doctests.

karan1276 · 2016-12-25T16:57:39Z

Awesome 🎉 , Thanks for your patience @jksuom :)

Expr: _eval_interval checks both limits(left and right)

ebc2cae

Expr._eval_interval only evaluates right limit and left limit, checks if they are equal, gives error if theyare not equal, else behaves normaly.

Revert "Expr: _eval_interval checks both limits(left and right)"

2889e20

This reverts commit ebc2cae.

Expr: _eval_interval will takes limit from correct side

d643660

Expr._eval_limit now compute the limit from the side where the interval lies. This is crutial since left and right limit need not be same.

jksuom reviewed Nov 30, 2016

View reviewed changes

Replace '__lt__' with '<' for better support

8faf8bd

Guarding against symbolic a < b

92626e9

dding case a == b, other minor fix

141a760

jksuom mentioned this pull request Dec 14, 2016

Use Meijerg G-function methods for integrating sinc #11916

Closed

case a == b is checked only if both a and b are not null

40a15db

added test

a5149ee

jksuom reviewed Dec 19, 2016

View reviewed changes

using same inequality

a536b46

jksuom reviewed Dec 20, 2016

View reviewed changes

jksuom mentioned this pull request Dec 21, 2016

modified expr.py, so that _eval_interval() now takes limit from the left #11966

Closed

karan1276 added 2 commits December 24, 2016 22:03

Adding a cleaner test

5d0628e

Correct test case

5a1dd7a

karan1276 commented Dec 24, 2016

View reviewed changes

jksuom merged commit 3677e37 into sympy:master Dec 25, 2016

jksuom mentioned this pull request Dec 25, 2016

integrate(log(0.5-x), (x, 0, 0.5)) wrongly produces imaginary part #11877

Closed

jksuom mentioned this pull request Dec 28, 2016

Use left-handed limit for integration with discontinuity at right end of interval #11978

Closed

jksuom mentioned this pull request Feb 7, 2017

integrate(1/x**2,(x,-oo,oo)) give 0 #12133

Closed

Expr: _eval_interval checks both limits(left and right) #11889

Expr: _eval_interval checks both limits(left and right) #11889

Conversation

karan1276 commented Nov 30, 2016

jksuom commented Nov 30, 2016

karan1276 commented Nov 30, 2016

karan1276 commented Nov 30, 2016

jksuom commented Nov 30, 2016

karan1276 commented Nov 30, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

karan1276 commented Dec 1, 2016

jksuom commented Dec 2, 2016

karan1276 commented Dec 3, 2016

jksuom commented Dec 3, 2016

jksuom commented Dec 3, 2016

karan1276 commented Dec 4, 2016

karan1276 commented Dec 4, 2016

jksuom commented Dec 4, 2016

karan1276 commented Dec 4, 2016 • edited

jksuom commented Dec 4, 2016

karan1276 commented Dec 18, 2016

karan1276 commented Dec 18, 2016

jksuom commented Dec 18, 2016

karan1276 commented Dec 19, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jksuom commented Dec 20, 2016

karan1276 commented Dec 24, 2016

jksuom commented Dec 24, 2016

Choose a reason for hiding this comment

jksuom commented Dec 25, 2016

jksuom commented Dec 25, 2016

karan1276 commented Dec 25, 2016

karan1276 commented Dec 4, 2016 •

edited