mathjs icon indicating copy to clipboard operation
mathjs copied to clipboard

math.floor returns apparently wrong value for BigNumbers; possibly implement distinct epsilon for BigNumber operations?

Open switchYello opened this issue 3 years ago • 12 comments

hello,math floor maybe wrong

case 1: math.floor(math.bignumber("1234567890123456.9951").mul( math.bignumber(100)),0).toString()

is wrong , accept '123456789012345699' but '123456789012345700'

case2: math.floor(math.bignumber("1234567890123456.9951").mul( math.bignumber(100)),1).toString() is ok , value is : '123456789012345699.5'

switchYello avatar Jun 27 '22 10:06 switchYello

The reason is that the functions floor, ceil, fixand equality functions have a mechanism to deal with round-off errors, for example:

math.equal(0.99999999999999, 1) // true, because it is 'nearly equal'

This behavior is configurable with the config value epsilon, which is 1e-12 by default. This default makes sense for numbers, but is too low when working with BigNumbers (having a precision of 64 digits by default, which is also configurable).

To solve this, you can configure a smaller epsilon, like:

math.config({ epsilon: 1e-60 })
math.floor(math.bignumber("1234567890123456.9951").mul( math.bignumber(100)),0).toString()
// '123456789012345699' as expected

josdejong avatar Jun 28 '22 09:06 josdejong

I misunderstood the function usage , Thank you for your answer , it helps me a lot ^____^ ^____^

switchYello avatar Jun 29 '22 06:06 switchYello

Well, it's not your fault really, it's simply not working ideal.

I've documented this in the docs now: https://github.com/josdejong/mathjs/commit/297d2e0c558ec6f18efc9f06ddc83f4871ce0fac

But I have the feeling that we should think this through and come up with a solution that prevents this issue in the same place. Both number and BigNumber can be used together, and both need a different epsilon. So I think we should split this into two separate config options. Also, the epsilon for BigNumbers should maybe be coupled with the configured precision (64 by default), it should be close to the configured precision.

Anyone interested into thinking this though and come up with a solution?

josdejong avatar Jun 29 '22 08:06 josdejong

I use java more , I suggestion can reference design of java。 we can providing a method like 'setScale(int scale,int roundingMode)' on BigNumber , manual control discard strategy and bits.

you say number and BigNumber use different epsilon is good idea. by default BigNumber's epsilon should long.

besides maybe we can providing one boole type switch to close the approximate comparison feature。 once we close this feature, all operations are highly accurate, no longer allowed 0.9999...9999 == 1 .

I am not very proficient in JS, sad not to provide help code ( ̄︶ ̄)↗ 

switchYello avatar Jun 30 '22 03:06 switchYello

I think mathjs has more or less the same , it has a config(...) function to set configuration. However, mathjs does not only support BigNumber, but a set of different data types, so that is a bit different.

It makes sense to introduce a config option to enable/disable nearly equal comparisons, thanks for your suggestion.

josdejong avatar Jul 01 '22 11:07 josdejong

I'd like to work on that

jakubriegel avatar Jan 11 '23 08:01 jakubriegel

Thanks @jakubriegel for picking this up! I think we should make this idea more concrete. Can you do a proposal?

josdejong avatar Jan 13 '23 13:01 josdejong

I want to first investigate the implementation and check if it can be done via some already present config or is there a need for custom logic

jakubriegel avatar Jan 18 '23 19:01 jakubriegel

I think the problem is that complicated and see three options:

  1. Add additional epsilons for each type (like bigNumberEpsilon) to the config. This would be an explicit solution to rounding problems, but requires change in every epsilon use case and demand better library underrating for the user setting it.
  2. Adding epsilon as a param to floor(), ceil() etc. This will allows users to fine tune behaviour of the library and move the need of storing configuration away from maths.
  3. Leave everything as is. It reduces possible functionalities but also does not increase complexity of the tool.

I did a quick research on how it is done in NumPy and it turns out they do allows users to set the epsilon in any way.

jakubriegel avatar Mar 01 '23 19:03 jakubriegel

Thanks for thinking this through.

I have a preference for the first option, implementing a separate epsilon for bignumbers. I had a look at the code, and implementing that may actually not be that complicated: there are only 28 occurrences of config.epsilon in the code base, and most of them look like the following two cases:

nearlyEqual(x, y, config.epsilon)
bigNearlyEqual(x, y, config.epsilon) // <-- here we can use a different epsilon

Thinking aloud here: we could go for two separate options { epsilon: 1e-12, epsilonBigNumber: 1e-60}. Or we could extend the existing epsilon to be either a number applied to both numbers and BigNumbers (current behavior), or a nested object like { epsilon: { number: 1e-12, BigNumber: 1e-60 } }. Then it would be a non-breaking change I think, and it would be extensible for new numeric types. Thoughts?

josdejong avatar Mar 09 '23 09:03 josdejong

  1. console.log(Math.floor(5.89 *100)/100); //expected output is 5.89

  2. console.log(Math.floor(4.89 *100)/100); //expected output is 4.89 but getting 4.88

Why Math.floor is changing its behaviour with value '4.89' >

any one can explain to understand it

kamal-telus avatar Mar 17 '23 11:03 kamal-telus

If you execute 5.89 *100 and 4.89 *100 in your browser you'll discover why. This is all plain JavaScript though and not related to mathjs.

EDIT: when using mathjs, the output is as you expect:

console.log(mathjs.floor(4.89 * 100) / 100) // 4.89

// or simply:
console.log(mathjs.floor(4.89, 2)) // 4.89

josdejong avatar Mar 17 '23 14:03 josdejong