Incorrect result
Hi, apparently {{6 / 2 ( 2 + 1)}} is not the same as {{6 / 2 * (2 + 1)}}
Why?
Regards, Artur.
Comments (11)
-
repo owner -
reporter Probably. I've never heard about higher priority for 'implicit multiplication'. Apparently it's intentional, @polwel can I get a quick explanation why it's that way?
-
Some calculators do give higher priority to implicit multiplication (
1/2 pi
is not the same as1/2*pi
with Google calculator for instance), but AFAIK, this is not expected in SpeedCrunch.Let's wait for @polwel's opinion, but this looks like a bug to me, as I found nothing in the evaluator code about a higher priority for implicit multiplications, and the evaluator steps for both expressions seem to indicate we missed that case in the implementation of implicit multiplications.
-
Yep, definitely a bug, shouldn't happen.
-
This seems like a tough one. Right now, I don't see an easy fix. I'll have to think some more.
The bug only occurs in expressions of the sort
a/b (c)
. On the first pass, the evaluator will not reducea/b
, because it does not identify the implicit multiplication yet. It only seesb(
, which does not match the implicit multiplication rule, so it goes on. Only after processing)
, it applies the parenthesis rule to reduce the expression toa/b c
. By design, the parser will now work its way back, and hence it computesb*c
first.Basically our (primitive) parser cannot correctly handle such an expression without look-ahead.
Off the top of my head: maybe outright expressions that include
b(
whereb
is not a function is a viable way out that avoids rewriting most of the parser? Not liking this approach though...Or maybe just document this pecularity and call it a feature...
-
BTW, any other operator than
/
is concerned as well.2^2(2) = 16
2^2*(2) = 8
-
- changed status to resolved
Only exclude function calls from binary operator rule (fixes
#741)→ <<cset ac4983aef12e>>
-
Should be fixed in trunk. The issue was caused by a greedy exclusion condition which treated anything followed by an open parenthesis as a function call.
@amdrozd thanks for your report.
-
- changed status to closed
-
- changed component to parser
-
Issue
#825was marked as a duplicate of this issue. - Log in to comment
@polwel Do you think this has to do with commit a5f17a9? (CC @Teyut)