Bug 473669 - KRunner can give erroneous output when evaluating equations involving implicit mutiplication
Summary: KRunner can give erroneous output when evaluating equations involving implici...
Status: CONFIRMED
Alias: None
Product: krunner
Classification: Plasma
Component: calculator (show other bugs)
Version: 5.27.7
Platform: Ubuntu Linux
: NOR normal
Target Milestone: ---
Assignee: Plasma Bugs List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-08-23 07:26 UTC by Aaron Rainbolt
Modified: 2023-08-25 18:26 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Aaron Rainbolt 2023-08-23 07:26:51 UTC
SUMMARY
KRunner's calculator fails to apply the order of operations properly when dealing with implicit multiplication. It appears to use libqalculate's default Adaptive parsing mode, which is able to apply implicit multiplication before applying any other operation (even if this violates PEMDAS). This probably comes in handy for expressions involving variables, like "5/2x" (where 2x is meant to be taken as it's own thing), but for expressions consisting solely of known values (like what KRunner's calculator probably usually handles), it can result in extremely odd output. For instance, 6/2(2+1) = 9 according to PEMDAS, but KRunner outputs a 1 as the implicit multiply of 2(2+1) gets done before the division.

STEPS TO REPRODUCE
1. Open KRunner by pressing Alt+F2.
2. Type "=6/2(2+1)"

OBSERVED RESULT
1

EXPECTED RESULT
9

SOFTWARE/OS VERSIONS
Linux/KDE Plasma: Kubuntu 22.04 LTS
(available in About System)
KDE Plasma Version: 5.27.7
KDE Frameworks Version: 5.104.0
Qt Version: 5.15.3

ADDITIONAL INFORMATION
I believe this can be fixed by using libqalculate's "conventional" parsing mode on all calculations. This should be as easy as adding one line of code into https://invent.kde.org/plasma/plasma-workspace/-/blob/master/runners/calculator/qalculate_engine.cpp in the QalculateEngine::evaluate function.
Comment 1 Nate Graham 2023-08-23 16:21:14 UTC
Sounds good! Wanna submit a merge request to do that?
Comment 2 Aaron Rainbolt 2023-08-23 17:18:09 UTC
I was working on it last night, but isma pointed out (in the Plasma dev room) that KRunner supports unknown variables when solving equations (for instance you can type =2x=5 and it will tell you that x = 2.5). Setting conventional mode would cause the expression 1/2x to be parsed as 1/(2x) rather than (1/2)x.

Perhaps this isn't actually a bug? If a user doesn't want the implicit multiply to be applied first, they can make it explicit by inserting a * where it belongs, or they can use extra parentheses.

Worthy of note, libqalculate's Adaptive mode (which I *think* we're using now) allows an easy way of disambiguating this kind of expression: 1/2x is parsed as 1/(2x), but 1/2 x (notice the space) is parsed as (1/2)x. However when inserting a space into the equation in KRunner (as =6/2 (2+1)), the result is still 1. That may actually be a bug, however I'm unsure if fixing it would help matters much since I don't think most users will know about this interesting libqalculate behavior and be able to use it to their advantage.
Comment 3 Nate Graham 2023-08-25 18:26:49 UTC
Sounds like our options are to submit an MR or close this as RESOLVED INTENTIONAL. Your call. :)