Fix #222: Wrong result for e.g. √(2.25) - 1.5 #222

[guihuaz]

Fixes #222, #309, #574

Description of the changes:

• add a helper function _snaprat to check if a result RAT should be zero with given precision. Apply _snaprat to addrat/subrat/lograt.
• The root causes of issues #222, #309, #574 are the same. For an irrational number result, the approximation introduces small errors beyond precision required, which is expected. The issue was Ratpak tried to take "precision" digits from the calculation result regardless where the result came from, and it exposed digits from approximation errors. Take sqrt(2.25) - 1.5 for example. sqrt(2.25) might be approximated to 1.499...998... depending on the precision. When this number subtracted 1.5, the Ratpak got 0.000...001.... Ratpak took up to precision number of digits and converts to scientific format, which resulted -1.2566515751494548126925332972115e-47. This PR checks the magnitude difference between the result and the input operands. Since the result is too close to 47 magnitude smaller than original subtraction operands 1.499...998... and 1.5, it would be snapped to 0.
• The same fix applies to lograt as well.
• One further optimization could be a new sqrt function specifically checking perfect square first before using Taylor series approximation.

How changes were validated:

• Add 1 test case in CalculatorManagerTestStandard to verify addrat.
• Add 2 test cases in CalculatorManagerTestScientific to verify addrat and lograt
• Manual test the cases in the linked issues.
• Manual test log case: log(sqrt(2.25)/1.5)

[guihuaz]

@guihuaz please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.

@microsoft-github-policy-service agree [company="{your company}"]

Options:

• (default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.

@microsoft-github-policy-service agree

• (when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.

@microsoft-github-policy-service agree company="Microsoft"

Contributor License Agreement

@microsoft-github-policy-service agree

[guihuaz]

@tian-lt, are you the maintainer of this repo? Could you please take a look at the PR?

[tian-lt]

@tian-lt, are you the maintainer of this repo? Could you please take a look at the PR?

@guihuaz Sure. Thank you for proposing the fix. Let me trigger a CI run for this PR. I'll review the changes and get back to you later.

[tian-lt]

@guihuaz Shall we adjust the result in the Rational class or a higher context? Because treating the epsilon as zero is more like an application requirement rather than a correct behavior of semi-numerical analysis.

[guihuaz]

@guihuaz Shall we adjust the result in the Rational class or a higher context? Because treating the epsilon as zero is more like an application requirement rather than a correct behavior of semi-numerical analysis.

@tian-lt I thought about adding the change in Rational for "+/-" and RationalMath for Log. I think within Ratpack is more appropriate.

1. _snaprat uses rat_smallest which is private to Ratpack implementation. Rational is a just wrapper, which doesn't know exactly when to snap.
2. More fundamentally, the snapping is not really just treating a small value as zero. Ratpack (and the Calculator app) should display the numbers within precision no matter how small it is. Take another example, 2e-47 - 1e-47, the Ratpack should return and Calculator app should display 1e-47, although the number is small. In the case of sqrt(2.25) - 1.5, the result -1.2566515751494548126925332972115e-47 is wrong answer from Ratpack, not just application requirement. Please see the PR description for more details.
3. Will Ratpack be used by other applications? If not, the risk of changing in Rational and RationalMath is similar as directly changing with Ratpack. By the way, Ratpack, Rational, and RationalMath handle both rational and irrational numbers. So their naming is not accurate.

[tian-lt]

The changes look good to me. @HuanglinXu21 @guominrui, could you help take a look?

[HuanglinXu21]

The changes look good to me. @HuanglinXu21 @guominrui, could you help take a look?

Looks good to me, too. I tested several cases, and they all passed😉