Skip to content
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

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

Open
switchYello opened this issue Jun 27, 2022 · 12 comments

Comments

@switchYello
Copy link

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
Copy link
Owner

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
Copy link
Author

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

@josdejong
Copy link
Owner

josdejong commented Jun 29, 2022

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

I've documented this in the docs now: 297d2e0

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
Copy link
Author

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
Copy link
Owner

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
Copy link
Contributor

I'd like to work on that

@josdejong
Copy link
Owner

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

@jakubriegel
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Owner

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
Copy link

kamal-telus commented Mar 17, 2023

  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
Copy link
Owner

josdejong commented Mar 17, 2023

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

@gwhitney gwhitney changed the title math.floor return wrong value math.floor returns apparently wrong value for BigNumbers; possibly implement distinct epsilon for BigNumber operations? Oct 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants