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

Abs([-1,1] range real function - 1) -> redundant Abs #21359

Open
smichr opened this issue Apr 20, 2021 · 5 comments
Open

Abs([-1,1] range real function - 1) -> redundant Abs #21359

smichr opened this issue Apr 20, 2021 · 5 comments
Labels

Comments

@smichr
Copy link
Member

smichr commented Apr 20, 2021

Abs(cos(Dummy(real=1)) + 1) -> arg without abs; the same for sin and tanh whose ranges are in [-1, 1]

@smichr
Copy link
Member Author

smichr commented Apr 20, 2021

Here is a possible solution to have Abs recognize this:

diff --git a/sympy/functions/elementary/complexes.py b/sympy/functions/elementary/complexes.py
diff --git a/sympy/functions/elementary/complexes.py b/sympy/functions/elementary/complexes.py
index b34ca3a7de..7deddeb713 100644
--- a/sympy/functions/elementary/complexes.py
+++ b/sympy/functions/elementary/complexes.py
@@ -7,10 +7,11 @@
 from sympy.core.numbers import pi, I, oo
 from sympy.core.relational import Eq
 from sympy.functions.elementary.exponential import exp, exp_polar, log
+from sympy.functions.elementary.hyperbolic import tanh
 from sympy.functions.elementary.integers import ceiling
 from sympy.functions.elementary.miscellaneous import sqrt
 from sympy.functions.elementary.piecewise import Piecewise
-from sympy.functions.elementary.trigonometric import atan, atan2
+from sympy.functions.elementary.trigonometric import atan, atan2, cos, sin
 
 ###############################################################################
 ######################### REAL and IMAGINARY PARTS ############################
@@ -589,9 +590,18 @@ def eval(cls, arg):
             return exp(re(arg.args[0]))
         if isinstance(arg, AppliedUndef):
             return
-        if arg.is_Add and arg.has(S.Infinity, S.NegativeInfinity):
-            if any(a.is_infinite for a in arg.as_real_imag()):
-                return S.Infinity
+        if arg.is_Add:
+            if arg.has(S.Infinity, S.NegativeInfinity):
+                if any(a.is_infinite for a in arg.as_real_imag()):
+                    return S.Infinity
+            else:
+                c, t = arg.as_independent(cos, sin, tanh)
+                if t:
+                    m, t = t.as_independent(cos, sin, tanh)
+                    if isinstance(t, (cos, sin, tanh)) and t.is_real:
+                        if arg.subs(t, Dummy(nonnegative=True) - 1).is_nonnegative:
+                            return arg
+                    return
         if arg.is_zero:
             return S.Zero
         if arg.is_extended_nonnegative:

related to #21351

@Psycho-Pirate
Copy link
Member

I would like to work on this. Will implementing the above solution fix this issue and #21351?
@smichr

@oscarbenjamin
Copy link
Contributor

oscarbenjamin commented Apr 22, 2021

The proposed diff seems too much to me. It should inspect the args but not call subs or compute any new expressions I think. If it does make sense to do this at all then I think it should be done by making is_nonnegative and is_ge handle this rather than as a special case in Abs:

In [25]: t = Symbol('t', real=True)

In [26]: e = 1+cos(t)

In [27]: e
Out[27]: cos(t) + 1

In [28]: e.is_nonnegative

I think that maybe is_ge could have a handler for Add that can translate is_ge(cos(t) + 1, 0) to is_ge(cos(t), -1) and then a handler for cos that can evaluate that. Or perhaps Add.is_nonnegative (_eval_is_extended_nonnegative) should recognise a 2-term Add with a Number and call is_ge. We need to be careful about circularity and infinite recursion here. I'm not sure if is_ge is built on top of is_nonnegative or vice-versa.

Maybe is_ge could be used here (instead of s = v + c):

sympy/sympy/core/add.py

Lines 789 to 798 in b76553a

def _eval_is_extended_nonnegative(self):
from sympy.core.exprtools import _monotonic_sign
if not self.is_number:
c, a = self.as_coeff_Add()
if not c.is_zero and a.is_extended_nonnegative:
v = _monotonic_sign(a)
if v is not None:
s = v + c
if s != self and s.is_extended_nonnegative:
return True

I'm not sure if is_ge would end up recursing back to Add.is_nonnegative though...

@vitarka101
Copy link

I think that maybe is_ge could have a handler for Add that can translate is_ge(cos(t) + 1, 0) to is_ge(cos(t), -1) and then a handler for cos that can evaluate that.

sympy/sympy/core/relational.py

Lines 1235 to 1244 in 05f5021

if retval is not None:
return retval
else:
n2 = _n2(lhs, rhs)
if n2 is not None:
# use float comparison for infinity.
# otherwise get stuck in infinite recursion
if n2 in (S.Infinity, S.NegativeInfinity):
n2 = float(n2)
return n2 >= 0

In the following code above for any of the expression like cos(t) + 1 the value of n2 will be None,So should a different if condition be made for such cases or something else. I couldn't understand what @oscarbenjamin meant by handler.

@smichr
Copy link
Member Author

smichr commented May 6, 2021

meant by handler.

if you look in the relational.py file you will see handlers/functions that get dispatched to for different types of arguments.

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