[PATCH] abs(sin(1)) simplifies to sin(1)

[Fabian Seoane]

Before of this patch, this calculation returned an AssertionError
(see issue #1051).

Adopted approach is to not use the assumptions framework for this
calculation. This is faster, but also, since we are slowly moving
the assumption system out of the core, this will have to be how
things are done inside the core.
---
sympy/functions/elementary/complexes.py | 10 ++++++----
sympy/functions/elementary/tests/test_complexes.py | 10 ++++++++--
2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/sympy/functions/elementary/complexes.py b/sympy/functions/elementary/complexes.py
index 250d808..b331f08 100644
--- a/sympy/functions/elementary/complexes.py
+++ b/sympy/functions/elementary/complexes.py
@@ -1,5 +1,6 @@

-from sympy.core.basic import Basic, S, C, sympify
+from sympy.core.basic import Basic, S, C
+from sympy.core.numbers import Real
from sympy.core.function import Function
from sympy.functions.elementary.miscellaneous import sqrt

@@ -232,9 +233,10 @@ def canonize(cls, arg):
def eval(cls, arg):
if arg is S.NaN:
return S.NaN
- if arg.is_zero: return arg
- if arg.is_positive: return arg
- if arg.is_negative: return -arg
+ _evalf = arg.evalf()
+ if isinstance(_evalf, Real):
+ if _evalf >= 0: return arg
+ else: return -arg
coeff, terms = arg.as_coeff_terms()
if coeff is not S.One:
return cls(coeff) * cls(C.Mul(*terms))
diff --git a/sympy/functions/elementary/tests/test_complexes.py b/sympy/functions/elementary/tests/test_complexes.py
index 28d97a9..5ee4314 100644
--- a/sympy/functions/elementary/tests/test_complexes.py
+++ b/sympy/functions/elementary/tests/test_complexes.py
@@ -1,6 +1,6 @@
from sympy import symbols, Symbol, sqrt, oo, re, nan, im, sign, I, E, log, \
- pi, arg, conjugate, expand
-from sympy.utilities.pytest import XFAIL
+ pi, arg, conjugate
+from sympy.functions import sin, cos


def test_re():
@@ -97,6 +97,12 @@ def test_abs():
n = Symbol('n',integer=True)
assert x**(2*n) == abs(x)**(2*n)

+def test_abs_sin_cos():
+ assert abs(sin(1)) == sin(1)
+ assert abs(sin(0.1)) == sin(0.1)
+ assert abs(sin(4)) != sin(4)
+ assert abs(cos(0.2)) == cos(0.2)
+
def test_abs_real():
# test some properties of abs that only apply
# to real numbers
--
1.6.1.2

[Ondrej Certik]

Yep, now it's +1, thanks!

[Fredrik Johansson]

On Fri, Mar 13, 2009 at 9:53 AM, Fabian Seoane <fab...@fseoane.net> wrote:
>
> Before of this patch, this calculation returned an AssertionError
> (see issue #1051).

The new version is worse, IMO. Instead of just asking whether the
argument is negative, it now needs to worry about details about the
evalf implementation. Also, this version is less general.

The right thing to do is to check arg.is_negative or use a function
is_negative(arg). It should be up to is_negative to use assumptions,
evalf, or any other method to determine whether arg is negative (or
whether to do nothing at all), and the details should be hidden behind
the is_negative abstraction.

Fredrik

[Fabian Seoane]

Thanks for the feedback, Fredrik. You are right that this version is
worse in the sense that it's evalf-dependent,
so I'm not pushing this in and leaving the issue unassigned.

Just wanted to say for future reference that i don't think that this is
less general because in the future we should use
refine to simplify this expression in case of symbolic values
(refine(abs(sin(x)), Assume(x, 'real')) -> sin(x)), that is why
in this case I would only do simplifications in the numeric case.
> Fredrik
>
> >
>
>

[Fabian Seoane]

maybe a nicer implementation would be:
if (arg >= 0) == True: return arg
if (arg < 0) == True: return -arg

that way we don't care about the return type of evalf ...