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

[FIX] tools: epsilon magnitude #35565

Closed
wants to merge 1 commit into from

Conversation

nim-odoo
Copy link
Contributor

@nim-odoo nim-odoo commented Aug 8, 2019

The following rounding is incorrect:

>>> float_round(6.6 * 0.175, precision_digits=2)
1.15

Indeed, 6.6 * 0.175 = 1.155 ≈ 1.16.

In this specific case, the epsilon computed is not sufficient. A
precision of 53 gives:
normalized_value = 115.49999999999997
epsilon = 1.2823075934420547e-14
=> new normalized_value = 115.49999999999999

Bad luck, this is just not enough to tip the value in the right
direction.

However, a precision of 52 is sufficient:
normalized_value = 115.49999999999997
epsilon = 2.5646151868841094e-14
=> new normalized_value = 115.5

The value of 53 was chosen from the binary64 number format precision.
In case of Python, the corresponding machine epsilon is 2^-52 [1].
Therefore, using 52 instead of 53 does make sense.

It is worth noting that the value of the machine epsilon
2^-52 = 2.2204460492503131e-16, which is still 2 orders of magnitude
below our dynamic estimation.

[1] https://en.wikipedia.org/wiki/Machine_epsilon
[2] numpy.finfo(float).eps = 2.2204460492503131e-16

opw-2047368

Description of the issue/feature this PR addresses:

Current behavior before PR:

Desired behavior after PR is merged:

--
I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr

The following rounding is incorrect:
```
>>> float_round(6.6 * 0.175, precision_digits=2)
1.15
```

Indeed, 6.6 * 0.175 = 1.155 ≈ 1.16.

In this specific case, the `epsilon` computed is not sufficient. A
precision of 53 gives:
    normalized_value = 115.49999999999997
    epsilon = 1.2823075934420547e-14
    => new normalized_value = 115.49999999999999

Bad luck, this is just not enough to tip the value in the right
direction.

However, a precision of 52 is sufficient:
    normalized_value = 115.49999999999997
    epsilon = 2.5646151868841094e-14
    => new normalized_value = 115.5

The value of 53 was chosen from the `binary64` number format precision.
In case of Python, the corresponding machine epsilon is 2^-52 [1].
Therefore, using 52 instead of 53 does make sense.

It is worth noting that the value of the machine epsilon
2^-52 = 2.2204460492503131e-16, which is still 2 orders of magnitude
below our dynamic estimation.

[1] https://en.wikipedia.org/wiki/Machine_epsilon
[2] `numpy.finfo(float).eps = 2.2204460492503131e-16`

opw-2047368
@nim-odoo nim-odoo self-assigned this Aug 8, 2019
@robodoo robodoo added seen 🙂 CI 🤖 Robodoo has seen passing statuses labels Aug 8, 2019
@C3POdoo C3POdoo added the OE the report is linked to a support ticket (opw-...) label Aug 8, 2019
@nim-odoo
Copy link
Contributor Author

robodoo r+

robodoo pushed a commit that referenced this pull request Aug 20, 2019
The following rounding is incorrect:
```
>>> float_round(6.6 * 0.175, precision_digits=2)
1.15
```

Indeed, 6.6 * 0.175 = 1.155 ≈ 1.16.

In this specific case, the `epsilon` computed is not sufficient. A
precision of 53 gives:
normalized_value = 115.49999999999997
epsilon = 1.2823075934420547e-14
=> new normalized_value = 115.49999999999999

Bad luck, this is just not enough to tip the value in the right
direction.

However, a precision of 52 is sufficient:
normalized_value = 115.49999999999997
epsilon = 2.5646151868841094e-14
=> new normalized_value = 115.5

The value of 53 was chosen from the `binary64` number format precision.
In case of Python, the corresponding machine epsilon is 2^-52 [1].
Therefore, using 52 instead of 53 does make sense.

It is worth noting that the value of the machine epsilon
2^-52 = 2.2204460492503131e-16, which is still 2 orders of magnitude
below our dynamic estimation.

[1] https://en.wikipedia.org/wiki/Machine_epsilon
[2] `numpy.finfo(float).eps = 2.2204460492503131e-16`

opw-2047368

closes #35565

Signed-off-by: Nicolas Martinelli (nim) <nim@odoo.com>
@robodoo
Copy link
Contributor

robodoo commented Aug 20, 2019

Merged at 0f878e4, thanks!

@robodoo robodoo closed this Aug 20, 2019
@nim-odoo nim-odoo deleted the 11.0-opw-2047368-epsilon-nim branch August 20, 2019 07:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI 🤖 Robodoo has seen passing statuses OE the report is linked to a support ticket (opw-...)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants