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

multipleOf precision #84

Closed
pashoo2 opened this issue Dec 5, 2015 · 14 comments
Closed

multipleOf precision #84

pashoo2 opened this issue Dec 5, 2015 · 14 comments
Labels

Comments

@pashoo2
Copy link

pashoo2 commented Dec 5, 2015

For example, the result of ‌‌53.198098 % 0.0000001 is ‌4.068961201910684e-15. And this causes an errors with the "multipleOf" keyword.
Please, use something another instead of the modulo ("%") operator for validation of this keyword.

@epoberezkin epoberezkin changed the title multipleOf multipleOf precision Dec 5, 2015
@epoberezkin
Copy link
Member

It is also the case in v8 that 53.198098 / 0.0000001 = 531980980.00000006, although the precise value is 531980980.

There is no efficient way to improve integer arithmetics. Also I am not sure what is the practical reason to use "multipleOf": 0.0000001. If the number of decimal digits is really important maybe you can use a string with pattern instead...

Also you probably could define modulo precision for multipleOf as an option. It can be an option multipleOfPrecision: N (N is a positive integer). And in case it is defined, it will check that the modulo result is within -1e-N ... 1e-N range (instead of being exactly 0).

If that's what you want maybe you could make a PR with such option.

@blakeembrey
Copy link
Collaborator

@epoberezkin It might be worth looking at Number.EPSILON. That way the offset is factored into the calculation from the beginning (not sure if there's a drawback with using this, except maybe false positives on really tiny cases). This is only a slightly related comment in case you weren't aware of it.

@epoberezkin
Copy link
Member

@blakeembrey thanks, it's interesting. Never heard of it :) The disadvantages are that it's ES6, so won't work in node 0.10 (we still use it...) and in some supported browsers, and that the above would fail anyway: 53.198098 % 0.0000001 is greater than Number.EPSILON.

@blakeembrey
Copy link
Collaborator

@epoberezkin Right, you'd use it to check the difference so you probably can't use it here (maybe, bad at math, sorry!). Also, it's just a number so I'm sure it's fine to polyfill. E.g. Math.pow(2, -52) === Number.EPSILON.

Edit: Thought, probably wrong, but you could do 53.1980435435 % 0.0000001 < Math.pow(10, -7) where -7 is the number of decimal places of accuracy.

@epoberezkin
Copy link
Member

yes, 1e-7 will work too (it's compiled so can be x > -1e-{{=precision}} && x < 1e-{{=precision}} - modulo is negative for negative numbers)

@aaronshaf
Copy link

Not sure if this is related, but multipleOf 0.01 fails to validate 4.18, etc.

@epoberezkin
Copy link
Member

Yes, same story here. 4.18 / 0.01 = 417.99999999999994. 4.18 % 0.01 is 0.01which is really bad... (multipleOf doesn't use modulo though, it compares the result of the division with parseInt() of this result).

The option I suggested would resolve this issue and probably it should be set to 12 by default...

@Palisand
Copy link

For the following property:

    "latitude": {
      "type": "number",
      "maximum": 85,
      "minimum": -85,
      "multipleOf": 1e-8
    }

Validation will fail for the value 40.7593657 even when multipleOfPrecision is set to 8.

@epoberezkin
Copy link
Member

It passes with 6. multipleOfPrecision is explained in docs, see the docs on the information about this option.

@Palisand
Copy link

Palisand commented Dec 20, 2017

I read the option description in the docs before posting that comment. It makes no mention of 6 being the upper limit and states this option can be set "to some positive integer N". Did you get 6 from Math.abs(Math.round(division) - division) < 1e-N?

@epoberezkin
Copy link
Member

You can't really compute this N, it just determines how close the result of division should be to a nearest integer number. The number of correct fractional digits depends on the number of digits in the integer part. So the correct N would depend on the range of the division results. With the divider 1e-8 you can't have too many fractional digits as the result is quite large...

There can be a better algorithm than Math.abs(Math.round(division) - division) < 1e-N, e.g. see #652 submitted today too.

@Delagen
Copy link
Contributor

Delagen commented Feb 8, 2018

For example 555555555555.22/0.01 = 55555555555521.99
Cause not works ((

@epoberezkin
Copy link
Member

epoberezkin commented Feb 11, 2018

It would pass with multipleOfPrecision equal to 1 or 2. multipleOf is not really created to determine the number of digits in the fractional format. If you really need to enforce a certain number of fractional digits you can consider using a string and patterns to validate it, rather than a number.
With a number, particularly a large one, multipleOf will not give you a precise result. It's the limitation of JavaScript number precision.

@kapouer
Copy link
Contributor

kapouer commented May 13, 2022

NB: the algorithm might trigger less floating points issues if it was just making sure that
(value * 1eN) / (multipleOf * 1eN) is an integer, where N is the number of decimals in multipleOf or the fixed multipleOfPrecision number.

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

No branches or pull requests

7 participants