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

sympify("E") unexpectedly returns something else than Symbol("E") #24397

Open
qdriven opened this issue Dec 16, 2022 · 9 comments
Open

sympify("E") unexpectedly returns something else than Symbol("E") #24397

qdriven opened this issue Dec 16, 2022 · 9 comments

Comments

@qdriven
Copy link

qdriven commented Dec 16, 2022

  1. Running following python script:
from sympy import symbols, sympify
expr_str = 'E*A*B*C*D*44/12'
A,B,C,D,E = symbols('A B C D E')
expr = sympify(expr_str)
expected = 10.2 * 43.070 * 19.41 * 0.99 * 44 / 12
print("expected result:", expected)
result = expr.evalf(subs={'A': '10.2', 'B': '43.070', 'C': '19.41', 'D': '0.99', 'E': '1'})
print("actual result:",result)
  1. get the result:
expected result: 30953.317606200002
actual result: 84139.8407794549
  1. So:
    expr.evalf get a wrong result.
@bjodah
Copy link
Member

bjodah commented Dec 16, 2022

>>> print(sympify('A') == Symbol('A'))
True
>>> sympify('E') == Symbol('E')
False
>>> type(sympify('E'))
sympy.core.numbers.Exp1

consider using parse_expr rather than sympify, if you know what symbols might exist you can pass them as local_dict:

>>> type(parse_expr('E', local_dict={'E': Symbol('E')}))
sympy.core.symbol.Symbol

That said, I've also been bitten by 'E' mapping to Exp1, so I agree that it's unexpected. Perhaps an alternative to standard_transformations without this mapping could be useful.

@bjodah bjodah changed the title expr.evalf returns wrong value sympify("E") unexpectedly returns something else than Symbol("E") Dec 16, 2022
@1e9abhi1e10
Copy link
Contributor

The main problem is that expression, E is automatically treated as an exp and not only with E but also with I and S as well , the same problem occurs.
As in I
expected result: 30953.317606200002
actual result: 30953.3176062*I it gives that
and in the case of S, it gives this TypeError: unsupported operand type(s) for *: 'SingletonRegistry' and 'Symbol'
because this all variables are pre-defined in sympy

@smichr
Copy link
Member

smichr commented Dec 18, 2022

This is a known issue: QOSINE are the clashing single letters (as documented in abc.py).

@asmeurer
Copy link
Member

I'd say this should be closed as intended behavior. This is also documented in the sympify docstring (see the "locals" section https://docs.sympy.org/latest/modules/core.html#sympy.core.sympify.sympify).

@asmeurer asmeurer closed this as not planned Won't fix, can't repro, duplicate, stale Dec 18, 2022
@smichr
Copy link
Member

smichr commented Dec 18, 2022

closed as intended behavior

But it's a pretty big gotcha. I wonder why E instead of e (or I instead of i) was chosen as the representation. But either way, being so early in the alphabet it is susceptible to confusion.

Perhaps an alternative to standard_transformations without this mapping could be useful.

Maybe something like as escape_all could be used where unless a symbol in QOSINE is prefaced with a backslash it will be treated like a symbol.

@smichr smichr reopened this Dec 18, 2022
@asmeurer
Copy link
Member

But it's a pretty big gotcha. I wonder why E instead of e (or I instead of i) was chosen as the representation. But either way, being so early in the alphabet it is susceptible to confusion.

Because uppercase letters are much less common than lowercase letters for user defined variables. If we used i for instance, it would be shadowed every time someone did for i in .... If we used those this problem would be much more common.

Also note that this gotcha only exists if you're using string parsing. If you define expressions using normal Python syntax where you are explicit about defining symbols, it doesn't happen.

A,B,C,D,E = symbols('A B C D E')
expr = E*A*B*C*D*44/12

As confusing as this gotcha is, the default behavior of sympify is consistent. If we instead tried to use the calling namespace, the behavior would differ depending on the context where it was called from. Similarly, if we tried to "guess" somehow whether E means Symbol('E') or exp(1), it would vary depending on how good the guess is. We could not include these symbols, but then sympify() wouldn't always be the same as just executing an expression in from sympy import *.

Personally I would have avoided including so many single variable name symbols in SymPy in the first place. IMO we don't really need N (already a method, actually two methods), E (can just be exp(1)), or O (a nice shorthand but could have also have just been called order()). Q and S are convenient shorthands but I don't know if they're worth it. I is the only one that seems worth it to me. But they already exist so that's really a moot point now. I'm not sure if this is a big enough issue to try deprecating any of these names at this point.

@oscarbenjamin
Copy link
Contributor

I'm not sure if this is a big enough issue to try deprecating any of these names at this point.

I don't think we should deprecate the names but there should be a straight-simple way to disable all of them in parse_expr. The problem actually goes further than single letter names because e.g.:

In [69]: parse_expr('2*beta')
...
TypeError: unsupported operand type(s) for *: 'Integer' and 'FunctionClass'

Everything in the whole of from sympy import * which is about 1000 different names will potentially clash with something that should be a symbol in the string to be parsed. Of course users will expect some things like pi etc but there should be a straight-forward way to control the namespace that is used.

@smichr
Copy link
Member

smichr commented Dec 19, 2022

Great points added by both of you. Thanks for entertaining the idea of a better way. Note, too, that abc provides some guidance:

The module also defines some special names to help detect which names clash
with the default SymPy namespace.
_clash1 defines all the single letter variables that clash with
SymPy objects; _clash2 defines the multi-letter clashing symbols;
and _clash is the union of both. These can be passed for locals
during sympification if one desires Symbols rather than the non-Symbol
objects for those names.

>>> from sympy import S
>>> from sympy.abc import _clash1, _clash2, _clash
>>> S("Q & C", locals=_clash1)
C & Q
>>> S('pi(x)', locals=_clash2)
pi(x)
>>> S('pi(C, Q)', locals=_clash)
pi(C, Q)

@sylee957
Copy link
Member

sylee957 commented Jun 4, 2023

Instead, sympify('e'), sympify('i') gives Symbol than e, i, which are neither good defaults though.

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

7 participants