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

Fix exp(x + num)/exp(x) not evaluating #1828

Merged
merged 4 commits into from Sep 3, 2021
Merged

Fix exp(x + num)/exp(x) not evaluating #1828

merged 4 commits into from Sep 3, 2021

Conversation

isuruf
Copy link
Member

@isuruf isuruf commented Aug 27, 2021

No description provided.

@rikardn
Copy link
Contributor

rikardn commented Aug 28, 2021

I am curious about the rationale for doing this auto-simplification. To me it feels like a special case. Should also pi**(x + n) / pi**n be auto-simplified in the same way? And should we do exp(x + n)/exp(x) = exp(n) if n is an exact number automatically? I can see that this change follows what sympy does, I just cannot understand why.

@isuruf
Copy link
Member Author

isuruf commented Aug 28, 2021

pi**(x + n) / pi**x is already simplified to be pi**n as does E**(x + n) / E**x to E**n. What this PR is doing is,
it's simplifying E**n when possible, but in master E**(x + 0.2) / E**x is given as E**0.2, but E**0.2 is evaluated when constructed directly. This PR fixes this inconsistency.

@rikardn
Copy link
Contributor

rikardn commented Aug 28, 2021

I'm sorry @isuruf, it was sympy that had the inconsistency I was referring to. I got my imports mixed up when testing this.

I guess we would still have the following inconsistency:

symengine.pi**2.0
Out[1]: pi**2.0

symengine.E**2.0
Out[2]: 7.38905609893065

Are we sure that we want to do the auto-evaluation in the second example? Then we loose the option of evaluating the expression to a higher precision. Even if 2.0 might be a RealDouble it would be nice to have the option of evaluating to a higher precision. Even if using RealMPFR(2.0) I see that symengine is auto-evaluating to some precision. To me it seems as if e**2.0 should be kept as it is.

@isuruf
Copy link
Member Author

isuruf commented Aug 28, 2021

We should evaluate pi**2.0 too, but that's outside the scope of this PR.
We evaluate things like sin(2.0). Reason is that 2.0 is only accurate to 53 bits, so it doesn't make sense to evaluate sin(2.0) to more precision than the precision of 2.0. That's one reason to do it. The other is that it prevents explosion of the symbolic tree.
So, the policy that symengine uses is that if you send in floating point number to a function like (sin, exp, etc) you get back a floating point number.

@rikardn
Copy link
Contributor

rikardn commented Aug 28, 2021

Ok, this makes sense. Also using RealMPFR we get auto-evaluation to the precision we have requested.

symengine.E**symengine.RealMPFR(1.0, prec=200)
Out[1]: 2.7182818284590452353602874713526624977572470936999595749670

Then there are other cases we should auto-evaluate like:

symengine.E*2.0
Out[2]: 2.0*E

I can open an issue to remember these.

@isuruf isuruf added this to the 0.8.0 milestone Aug 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants