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 · 13 comments

Comments

Projects
None yet
6 participants
@pashoo2

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 added the option label Dec 5, 2015

@epoberezkin epoberezkin changed the title from multipleOf to multipleOf precision Dec 5, 2015

@epoberezkin

This comment has been minimized.

Owner

epoberezkin commented Dec 5, 2015

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

This comment has been minimized.

Collaborator

blakeembrey commented Dec 5, 2015

@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

This comment has been minimized.

Owner

epoberezkin commented Dec 5, 2015

@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

This comment has been minimized.

Collaborator

blakeembrey commented Dec 5, 2015

@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

This comment has been minimized.

Owner

epoberezkin commented Dec 6, 2015

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

This comment has been minimized.

aaronshaf commented Dec 29, 2015

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

@epoberezkin

This comment has been minimized.

Owner

epoberezkin commented Dec 29, 2015

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

This comment has been minimized.

Palisand commented Dec 20, 2017

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

This comment has been minimized.

Owner

epoberezkin commented Dec 20, 2017

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

@Palisand

This comment has been minimized.

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

This comment has been minimized.

Owner

epoberezkin commented Dec 20, 2017

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

This comment has been minimized.

Contributor

Delagen commented Feb 8, 2018

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

@epoberezkin

This comment has been minimized.

Owner

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.

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