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

Possible wrong result from calculate function #178

Closed
IdanVT opened this issue May 13, 2019 · 2 comments
Closed

Possible wrong result from calculate function #178

IdanVT opened this issue May 13, 2019 · 2 comments

Comments

@IdanVT
Copy link

IdanVT commented May 13, 2019

Issue title:

Possible wrong result from calculate function

If bug/question:

  • MathParser.org-mXparser verssion: v.4.3.3
  • Framework:.net, 4.6.1

Wrong boolean operators calculation order

`
Consider the following code:

            List<Argument> testArgs = new List<Argument>();
            testArgs.Add(new Argument("X1",0));
            testArgs.Add(new Argument("X2",1));
            testArgs.Add(new Argument("X3",1));
            testArgs.Add(new Argument("X4",1));
            testArgs.Add(new Argument("X5",1));
            testArgs.Add(new Argument("X6",0));
            Expression testExp = new Expression("(X1 & X2) | (X3 & X4) | X5 & X6", testArgs.ToArray()); 
            testExp.setVerboseMode();
            double ans = testExp.calculate();
            bool ans2 = (false && true) || (true && true) || true && false;

`
ans2 which is the equivalent expression return true
ans return 0 (false)

if i setVerboseMode I'm getting a wrong calculation order

Verbose output:

[][(X1 & X2) | (X3 & X4) | X5 & X6] Starting ...
[][(X1 & X2) | (X3 & X4) | X5 & X6] X1 = 0
[][(X1 & X2) | (X3 & X4) | X5 & X6] X2 = 1
[][(X1 & X2) | (X3 & X4) | X5 & X6] X3 = 1
[][(X1 & X2) | (X3 & X4) | X5 & X6] X4 = 1
[][(X1 & X2) | (X3 & X4) | X5 & X6] X5 = 1
[][(X1 & X2) | (X3 & X4) | X5 & X6] X6 = 0
[][(X1 & X2) | (X3 & X4) | X5 & X6] Starting calculation loop
[][(X1 & X2) | (X3 & X4) | X5 & X6] Parsing (0, 4) ---> ( 0 & 1 ) ... ---> ( 0 ) | ( 1 & 1 ) | 1 & 0 ... done
[][(X1 & X2) | (X3 & X4) | X5 & X6] Parsing (0, 2) ---> ( 0 ) ... ---> 0 | ( 1 & 1 ) | 1 & 0 ... done
[][(X1 & X2) | (X3 & X4) | X5 & X6] Parsing (2, 6) ---> ( 1 & 1 ) ... ---> 0 | ( 1 ) | 1 & 0 ... done
[][(X1 & X2) | (X3 & X4) | X5 & X6] Parsing (2, 4) ---> ( 1 ) ... ---> 0 | 1 | 1 & 0 ... done
[][(X1 & X2) | (X3 & X4) | X5 & X6] Parsing (0, 6) ---> 0 | 1 | 1 & 0 ... ---> 1 | 1 & 0 ... done
[][(X1 & X2) | (X3 & X4) | X5 & X6] Parsing (0, 4) ---> 1 | 1 & 0 ... ---> 1 & 0 ... done
[][(X1 & X2) | (X3 & X4) | X5 & X6] Parsing (0, 2) ---> 1 & 0 ... ---> 0 ... done
[][(X1 & X2) | (X3 & X4) | X5 & X6] Calculated value: 0
[][(X1 & X2) | (X3 & X4) | X5 & X6] Exiting

as you can see there is a wrong order calculation in lines:
[][(X1 & X2) | (X3 & X4) | X5 & X6] Parsing (0, 6) ---> 0 | 1 | 1 & 0 ... ---> 1 | 1 & 0 ... done
[][(X1 & X2) | (X3 & X4) | X5 & X6] Parsing (0, 4) ---> 1 | 1 & 0 ... ---> 1 & 0 ... done

while it should be
[][(X1 & X2) | (X3 & X4) | X5 & X6] Parsing (0, 6) ---> 0 | 1 | 1 & 0 ... ---> 0 | 1 | 0 ... done
[][(X1 & X2) | (X3 & X4) | X5 & X6] Parsing (0, 6) ---> 0 | 1 | 0 ... ---> 1 | 0 ... done
[][(X1 & X2) | (X3 & X4) | X5 & X6] Parsing (0, 4) ---> 1 | 0 ... ---> 1 ... done
[][(X1 & X2) | (X3 & X4) | X5 & X6] Calculated value: 0
[][(X1 & X2) | (X3 & X4) | X5 & X6] Exiting

It also damage the performance, Because logically you could abort the calculation in line:
[][(X1 & X2) | (X3 & X4) | X5 & X6] Parsing (0, 6) ---> 0 | 1 | 1 & 0 ... ---> 1 | 1 & 0 ... done
and return 1 as result

@IdanVT IdanVT changed the title wrong Possible wrong result from calculate function May 13, 2019
@mariuszgromada
Copy link
Owner

Thank you, but mXparser does not follow .net/java precedence. In general math there is no clear precedence defined for logical operators, the same is in case of natural language, thus you should always use parenthesis. I will consider precedence introduction.

Best regards

@mariuszgromada mariuszgromada added this to the 4.4 Gemoni milestone Jan 1, 2020
mariuszgromada added a commit that referenced this issue Jan 3, 2020
1) Negation
2) AND related
3) OR related
4) IMPL related
@mariuszgromada
Copy link
Owner

Fixed

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

2 participants