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

log(Abs) #21437

Open
proy87 opened this issue May 6, 2021 · 3 comments
Open

log(Abs) #21437

proy87 opened this issue May 6, 2021 · 3 comments

Comments

@proy87
Copy link

proy87 commented May 6, 2021

log(Abs) returns zoo, but the argument of log is FunctionClass.

skirpichev added a commit to skirpichev/diofant that referenced this issue May 12, 2021
@oscarbenjamin
Copy link
Contributor

The problem is that Basic subclasses pass through sympify so this can be fixed with:

diff --git a/sympy/functions/elementary/exponential.py b/sympy/functions/elementary/exponential.py
index bdedc455c2..8bf8972000 100644
--- a/sympy/functions/elementary/exponential.py
+++ b/sympy/functions/elementary/exponential.py
@@ -1,4 +1,5 @@
 from sympy.core import sympify
+from sympy.core.sympify import _sympify
 from sympy.core.add import Add
 from sympy.core.cache import cacheit
 from sympy.core.function import (
@@ -641,7 +642,7 @@ def eval(cls, arg, base=None):
         from sympy.sets.setexpr import SetExpr
         from sympy.functions.elementary.complexes import Abs
 
-        arg = sympify(arg)
+        arg = _sympify(arg)
 
         if base is not None:
             base = sympify(base)

That kind of change should be used throughout much of the codebase.

It is probably also good to change it so that sympify rejects Basic subclasses.

@asmeurer
Copy link
Member

I had thought we had an issue about replacing sympify with _sympify everywhere in library code, but I'm not finding it right now.

This is also related to #17280. Really FunctionClass the superclass should handle calling sympify. Classes shouldn't need to do this themselves in eval.

@oscarbenjamin
Copy link
Contributor

The proper fix would be to make sympify more strict:

diff --git a/sympy/core/sympify.py b/sympy/core/sympify.py
index ed5ba267d9..242cd2e6f3 100644
--- a/sympy/core/sympify.py
+++ b/sympy/core/sympify.py
@@ -346,10 +346,7 @@ def sympify(a, locals=None, convert_xor=True, strict=False, rational=False,
     if is_sympy is True:
         return a
     elif is_sympy is not None:
-        if not strict:
-            return a
-        else:
-            raise SympifyError(a)
+        raise SympifyError(a)
 
     if isinstance(a, CantSympify):
         raise SympifyError(a)

That leads to these failures in core:

================================================================== short test summary info ===================================================================
FAILED sympy/core/tests/test_containers.py::test_Tuple_Eq - assert Eq((1,), f(1)) is True
FAILED sympy/core/tests/test_expr.py::test_has_tuple - sympy.core.sympify.SympifyError: SympifyError: f
FAILED sympy/core/tests/test_function.py::test_diff_wrt_func_subs - assert Derivative(f(g(x)), g(x))*Derivative(g(x), x) == 2*Subs(Derivative(f(_xi_1), _xi...
FAILED sympy/core/tests/test_function.py::test_subs_in_derivative - assert Derivative(f(x, f(x, x)), f(x, x)) == Subs(Derivative(x + z, z), z, 2*x)
FAILED sympy/core/tests/test_function.py::test_Subs_Derivative - assert False
FAILED sympy/core/tests/test_function.py::test_issue_15947 - sympy.core.sympify.SympifyError: SympifyError: f
FAILED sympy/core/tests/test_match.py::test_derivative_bug1 - sympy.core.sympify.SympifyError: SympifyError: f
FAILED sympy/core/tests/test_match.py::test_derivative2 - sympy.core.sympify.SympifyError: SympifyError: f
FAILED sympy/core/tests/test_match.py::test_exclude - sympy.core.sympify.SympifyError: SympifyError: sin
FAILED sympy/core/tests/test_subs.py::test_trigonometric - assert exp(pi) == 0
FAILED sympy/core/tests/test_subs.py::test_functions_subs - assert g(y, x) + cos(x) == ((sin(y) + x) + cos(x))
FAILED sympy/core/tests/test_subs.py::test_derivative_subs - assert Derivative(g(x), g(x)) == Derivative(f(x), f(x))
FAILED sympy/core/tests/test_subs.py::test_Function_subs - assert Piecewise((g(f(x, y)), x < -1), (g(x), x <= 1)) == Piecewise((h(f(x, y)), x < -1), (h(x),...
FAILED sympy/core/tests/test_subs.py::test_recurse_Application_args - assert f(x, f(x, x)) == exp(2*x + 3*exp(5*x))
FAILED sympy/core/tests/test_sympify.py::test__sympify - sympy.core.sympify.SympifyError: SympifyError: f
================================================= 15 failed, 1193 deselected, 1 warning in 130.65s (0:02:10) =================================================

The main problem here seems to be that with that change things like sympify(sin) or sympify(Function('f')) fail

In [1]: sin(x).subs(sin, cos)
Out[1]: sin(x)

In [2]: f = Function('f')

In [3]: f(x).subs(f, cos)
Out[3]: f(x)

In [4]: sympify(f)
---------------------------------------------------------------------------
SympifyError                              Traceback (most recent call last)
<ipython-input-4-f6adfff4c072> in <module>
----> 1 sympify(f)

~/current/sympy/sympy/sympy/core/sympify.py in sympify(a, locals, convert_xor, strict, rational, evaluate)
    347         return a
    348     elif is_sympy is not None:
--> 349         raise SympifyError(a)
    350 
    351     if isinstance(a, CantSympify):

SympifyError: SympifyError: f:

skirpichev added a commit to skirpichev/diofant that referenced this issue Jun 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants