leongersen/noUiSlider

Float precision errors

xeruf opened this issue · 10 comments

xeruf commented

When I use noUiSlider.set to update the value to an int, I still sometimes get errors where JS creates a bad float when handing the value back to me:
image

Is there a way to avoid that, is there something you can fix, or do I have to live with it and always do rounding?

xeruf commented

I get this for the format and the tooltips to function seemingly at random.

The library is probably doing some math in which floating point imprecision is having this effect.

I think the library ought to be changed so that if step == 1 then the value is rounded off before being passed to the user.
All integers in the range [-2^53, 2^53 ] can be exactly represented in javascript IEEE 754. The library should take that into account.

The methods format always get the raw slider value. If you need these to be integers, they need to be rounded/floored/ceiled. Whether the input values for range and step are integers isn't relevant here.

xeruf commented

But how can this be if I set the raw slider value to a rounded integer?

Because values aren't internally kept as integers.

All values in javascript are stored as 64 bit floats. So the library's internal representation is really not a factor -- it's the same as the user's representation. Yes, doing math on floats can lead to imprecision, but if the user has told the library that he will be working only with "integers" (ie step == 1, or step == 2, ...) then the library shouldn't be returning floats -- it should round off to the nearest step requested by the user (except when the user provided a float value in spite of step == 1, or when smooth-steps is enabled).

The step parameter is relevant because it limits the slider handle's possible positions. When I set step to 1, and drag the slider between 10 and 11, the slider handle snaps from 10 to 11... never to 10.000001

Since all values are of type Number, (not int or float), whichever representation is used to initiate a slider is not the API for setting formatting.

The core problem really isn't about formatting, the problem is rather about constraining the actual value of the slider. I really can't think of a better API -- returning non-integer values when { step: <integer>, start: <integer> } is semantically incorrect, and requiring anything additional to { step: <integer>, start: <integer> } to specify that we want only integers is redundant.

The lack of an explicit int type in the language is not an obstacle here. Number is a floating-point type (as per the language specification) capable of distinctly storing both integer and non-integer values. Simply use Number.isSafeInteger() to find out if it's storing an integer value.

If the programmer configures the slider with { step: 1.0, start: 10.0, smoothSteps: false }, then the slider value should never be 10.00001. You could prevent that within the library internals by rounding the computed slider value when the handle is moved.

If either step or start are non-integers (or if smoothSteps is enabled), all bets are off -- perfectly accurate slider movement can no longer be guaranteed. But otherise, it can and I don't see any reason not to.

The problem is especially severe because it only happens some of the time. The programmer configures an integer-valued slider and prints the value to the screen somewhere, and it works 99% of the time. But if the range is just so then suddenly a field like "Persons attending event" becomes "9.99999999998". How silly. Yes, it can be mitigated by requiring the programmer to manually round the value, but the programmer has to be aware of this surprising behavior. Why require that? What downside could there be to rounding the value in such cases before providing it to the users of this library?

@andrewsm80 I'm open to a PR that sets the format option internally to Math.round if the arguments to start and step are integers.

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.