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

Int to float conversion bug #84

Open
Stranger6667 opened this issue May 23, 2020 · 3 comments
Open

Int to float conversion bug #84

Stranger6667 opened this issue May 23, 2020 · 3 comments

Comments

@Stranger6667
Copy link
Owner

In some numeric validators, we have a bug due to float conversion.(1u64 << 54) - 1 is 18014398509481983 and ((2u64 << 53) - 1) as f64 is 18014398509481984.0 which is +1. This causes various bugs on the corner cases:

{"minimum": 18014398509481984u64} should fail on 18014398509481984u64 - 1, because the latter is smaller. but it doesn't because the latter rounds to 18014398509481984 and the validation fails. Interestingly that 18014398509481984u64 - 2 doesn't fail and after the power of 53 we have these ranges that should fail but they don't:

  • 2 ^ 54 - 2 ^ 0 -> 2 ^ 54 - 1
  • ...
  • 2 ^ 63 - 2 ^ 9 -> 2 ^ 63 - 1

I.e with having the right part as the minimum value all values to the left part will pass validation

@Stranger6667
Copy link
Owner Author

Stranger6667 commented May 23, 2020

Oh ... even worse:

9007199254740995f64 is 9007199254740996.0, the same for Python ... so, it is basically how floating-point numbers are represented. Probably we can skip them

@macisamuele
Copy link
Contributor

Ideally we should try to reduce as much as possible floating point operations (https://rust-lang.github.io/rust-clippy/master/index.html#float_cmp).

At the current stage we rely on floats on few validators (min, max, const and multipleOf), we might want to explore ways to have more accuracy without a big hit in performance.

@Stranger6667
Copy link
Owner Author

Indeed, I found multiple cases when was testing the Python bindings in #54 with Hypothesis - some of them are inevitable like 9007199254740995f64 which has the same behavior in Python but for the others, we need to avoid it and I believe that the performance penalty won't be big. Also, it is great that we can have a lint for it :)

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