SymPy.jl icon indicating copy to clipboard operation
SymPy.jl copied to clipboard

Incorrect precedence of `//` in exponent when converting SymPy expression to Julia `Expr`

Open rben01 opened this issue 3 years ago • 3 comments

When converting a SymPy symbolic expression to a Julia Expr, if there is an integer-division in an exponent, the integer division is placed into the output Expr without parentheses to indicate precedence, which is incorrect.

To reproduce:

julia> convert(Expr, 1/(2*x*sympy.pi^(1//4)))
:((1 / 2) * pi ^ -1//4 * x ^ -1)

Expected result:

:((1 / 2) * pi ^ (-1//4) * x ^ -1)

As is, the returned Expr is interpreted as :(((1 / 2) * pi ^ -1)//4 * x ^ -1), which is incorrect.

rben01 avatar Aug 08 '22 16:08 rben01

Thanks for the report! A fix is coming soon.

jverzani avatar Aug 08 '22 17:08 jverzani

@jverzani Unfortunately I think the issue is more complicated than my example shows. 1//4 is evaluated as a Rational before being passed to the exponentiation, and although it's displayed as pi ^ -1//4 it's really pi ^ r where r is Rational(1, 4). So my example is not actually problematic.

However I did discover this issue when attempting to convert an expression to Expr, when 1//4 was passed in as integer division instead of as a Rational, and so I got the error (if my memory serves me) ERROR: MethodError: no method matching //(::Int64, ::Float64) (why 4 was being treated as a float, I don't now). I'll try to investigate further.

rben01 avatar Aug 08 '22 17:08 rben01

The fix I just merged basically throws in parentheses to evaluate ()^(). Before, without that the precedence rules were not consistent with the expression. Before tagging, can you send along your example that fails and I'll make sure this addresses that. Thanks!

jverzani avatar Aug 08 '22 18:08 jverzani