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

[Bug] Simple division Expression returns invalid value #310

Closed
darxis opened this issue Nov 22, 2023 · 14 comments
Closed

[Bug] Simple division Expression returns invalid value #310

darxis opened this issue Nov 22, 2023 · 14 comments
Labels
Milestone

Comments

@darxis
Copy link

darxis commented Nov 22, 2023

MathParser.org-mXparser verssion: v.5.2.1
Framework: .NET Framework 4.7.2

I came into a bug today, please see the following test case that I created to experience the bug:

[TestMethod]
public void testExpr1403(){
    TestCommonTools.testExprSettingsInit();
    bool testResult = false;
    String expStr = "0.0000004566969933 / 36370.51937825058";
    TestCommonTools.consolePrintTestExprStart(1, expStr);
    Expression testExp = new Expression(expStr);
    double value = testExp.calculate(); // it returns 1E-11 instead of 1.2556790530000044E-11
    double reg = 0.0000004566969933 / 36370.51937825058;
    if (value == reg)
        testResult = true;
    TestCommonTools.consolePrintTestExprEnd(value, reg, testResult, testExp);
    Assert.IsTrue(testResult);
}

The bug is that the division returns 1E-11 instead of 1.2556790530000044E-11

Please fix this, I will add a PR for the test case

@mariuszgromada
Copy link
Owner

Thanks.

Have you been trying switching off smart rounding settings? Below the link tot he section in the tutorial:

https://mathparser.org/mxparser-tutorial/smart-rounding-settings/

Best regards

@darxis
Copy link
Author

darxis commented Nov 22, 2023

@mariuszgromada I didn't change any default settings

mXparser.checkIfAlmostIntRounding(); // true
mXparser.checkIfUlpRounding(); // false
mXparser.checkIfCanonicalRounding(); // true

I encountered this bug when I upgraded from 4.4.2 to 5.2.1. On version 4.4.2 this bug doesn't occur but when I upgraded to 5.2.1 this bug is present.
I didn't found any breaking changes in the change log, on the nuget package additional details it says: API of versions 5.x are fully backward compatible with all 4.x. Technical migrating to 5.x should be hassle free.. But it isn't as I can see :/

@mariuszgromada
Copy link
Owner

I belive there was a bug in 4.4 with default settings. Can you switch off all the roundings?

@darxis
Copy link
Author

darxis commented Nov 22, 2023

@mariuszgromada On both 4.4.2 and 5.2.1 only AlmostIntRounding and CanonicalRounding are enabled by default.

Version 4.4.2
mXparser.checkIfAlmostIntRounding(); // true
mXparser.checkIfUlpRounding(); // false
mXparser.checkIfCanonicalRounding(); // true

Version 5.2.1
mXparser.checkIfAlmostIntRounding(); // true
mXparser.checkIfUlpRounding(); // false
mXparser.checkIfCanonicalRounding(); // true

Also I can see that this bug is present since version v5.0.7, on v5.0.6 the bug is not present.

When I call mXparser.disableCanonicalRounding() the bug disappears.
Why with CanonicalRounding enabled it gives 0.0000004566969933 / 36370.51937825058 = 0.00000000001?

@mariuszgromada
Copy link
Owner

mariuszgromada commented Nov 22, 2023

Can you run this code for me?

mXparser.disableAlmostIntRounding();
mXparser.disableCanonicalRounding();
mXparser.disableUlpRounding();
String expStr = "0.0000004566969933 / 36370.51937825058";
Expression testExp = new Expression(expStr);
double value = testExp.calculate();
double reg = 0.0000004566969933 / 36370.51937825058;
System.Console.WriteLine("value = " + value);
System.Console.WriteLine("reg = " + reg);

@mariuszgromada
Copy link
Owner

Can you also make a test using more fresh .net framework?

@darxis
Copy link
Author

darxis commented Nov 22, 2023

Can you run this code for me?

mXparser.disableAlmostIntRounding();
mXparser.disableCanonicalRounding();
mXparser.disableUlpRounding();
String expStr = "0.0000004566969933 / 36370.51937825058";
Expression testExp = new Expression(expStr);
double value = testExp.calculate();
double reg = 0.0000004566969933 / 36370.51937825058;
System.Console.WriteLine("value = " + value);
System.Console.WriteLine("reg = " + reg);

It returns the following output, clearly disabling CanonicalRounding fixes this issue.

value = 1,2556790530000044E-11
reg = 1,2556790530000044E-11

@mariuszgromada
Copy link
Owner

Ok, now pls see my result when canonical rounding is enabled

mXparser.disableAlmostIntRounding();
mXparser.enableCanonicalRounding();
mXparser.disableUlpRounding();
String expStr = "0.0000004566969933 / 36370.51937825058";
Expression testExp = new Expression(expStr);
double value = testExp.calculate();
double reg = 0.0000004566969933 / 36370.51937825058;
System.Console.WriteLine("value = " + value);
System.Console.WriteLine("reg = " + reg);

Result

value = 1.2556791E-11
reg = 1.2556790530000044E-11

I am using a newer version of .net, I suspect the implementation difference in .net between 4.7.2 (your older version) and others. Can you test this on e.g. .net6 or some other version?

@darxis
Copy link
Author

darxis commented Nov 22, 2023

I am executing the code you sent me in the mXparser source UnitTests projects.

I can see that it fails only for .NET Framework and .NET Standard libraries, for .NET Core it works properly.

I created another test case for the code you just sent to execute it in the mXparser source UnitTests projects (4-Unit-Tests-Core, 4-Unit-Tests-Net, 4-Unit-Tests-Stand)

[TestMethod]
public void testExpr1405()
{
    mXparser.disableAlmostIntRounding();
    mXparser.enableCanonicalRounding();
    mXparser.disableUlpRounding();
    String expStr = "0.0000004566969933 / 36370.51937825058";
    Expression testExp = new Expression(expStr);
    double value = testExp.calculate();
    double reg = 0.0000004566969933 / 36370.51937825058;
    System.Console.WriteLine("value = " + value);
    System.Console.WriteLine("reg = " + reg);
}

The code you sent gives the different outputs for .NET Core and .NET Framework/Standard.

4-Unit-Tests-Core
value = 1,2556791E-11
reg = 1,2556790530000044E-11

4-Unit-Tests-Net
value = 1E-11
reg = 1,255679053E-11

4-Unit-Tests-Stand
value = 1E-11
reg = 1,255679053E-11

@mariuszgromada
Copy link
Owner

@mariuszgromada
Copy link
Owner

I am using to string conversion in canonical rounding.

@darxis
Copy link
Author

darxis commented Nov 22, 2023

I am using to string conversion in canonical rounding.

That could be the cause, I think we should take a look at commits between v5.0.6 and v5.0.7 because the bug is not present in v5.0.6 and is present in v5.0.7.

@mariuszgromada
Copy link
Owner

No need. I have fixed the bug in the canonical rounding that was present since 4.4.2.

As of now please disable canonical rounding and you are good to go.

I will try do identify how to solve the difference between frameworks.

Best regards

@mariuszgromada mariuszgromada added this to the v.6.0 Picon milestone May 12, 2024
@mariuszgromada
Copy link
Owner

Fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants