fermat.js icon indicating copy to clipboard operation
fermat.js copied to clipboard

roundTo is probably wrong

Open oldrich-svec opened this issue 3 years ago • 2 comments

Calling roundTo(103.35, 0.1) gives 103.30000000000001 where it should give 103.4.

Original function:

/** Round a number `n` to the nearest multiple of `increment`. */
export function roundTo(n: number, increment = 1) {
  return Math.round(n / increment) * increment;
}

This function looks to work properly:

function roundTo(n: number, increment = 1) {
  const v1 = Math.round(n / increment ) * increment 
  const v2 = Math.round(n / increment + 1e-10) * increment 
  const v3 = Math.round(n / increment - 1e-10) * increment 
  return Math.abs(v1 - v2) > increment / 10 ? v2 : v3
}

oldrich-svec avatar Mar 25 '21 08:03 oldrich-svec

I'm not sure if it is worth adding this performance overhead to deal with floating-point edge cases, but we'll definitely keep it in mind for future updates.

In this example, if you want to round to a certain number of decimal places, you could instead just use the round() function which should not have this issue:

import {round} from '@mathigon/fermat`
console.log(round(103.35, 1)); // => 103.4

plegner avatar Mar 25 '21 21:03 plegner

That is true, but in our case we need to use roundTo function. 0.1 increment was just an example 😉.

PS: For me it is more important that the functions that I use are correct and I can always rely on them and only after that comes speed. 👍

oldrich-svec avatar Mar 26 '21 07:03 oldrich-svec