josdejong / mathjs

An extensive math library for JavaScript and Node.js
https://mathjs.org
Apache License 2.0
14.35k stars 1.24k forks source link

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

Open switchYello opened 2 years ago

switchYello commented 2 years ago

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'

josdejong commented 2 years ago

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
switchYello commented 2 years ago

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

josdejong commented 2 years ago

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?

switchYello commented 2 years ago

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 ( ̄︶ ̄)↗ 

josdejong commented 2 years ago

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.

jakubriegel commented 1 year ago

I'd like to work on that

josdejong commented 1 year ago

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

jakubriegel commented 1 year ago

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 commented 1 year ago

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.

josdejong commented 1 year ago

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?

kamal-telus commented 1 year ago
  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

josdejong commented 1 year ago

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