MikeMcl / bignumber.js

A JavaScript library for arbitrary-precision decimal and non-decimal arithmetic
http://mikemcl.github.io/bignumber.js
MIT License
6.63k stars 743 forks source link

toNumber does not respect ROUNDING_MODE #360

Closed bakkot closed 8 months ago

bakkot commented 8 months ago

Consider:

BigNumber.config({ DECIMAL_PLACES: 1000, ROUNDING_MODE: BigNumber.ROUND_HALF_UP });
console.log(BigNumber('9007199254740993').toNumber());

The mathematical value 9007199254740993 is precisely between two JS number values (9007199254740992 and 9007199254740994). So I would expect that when converting the BigNumber representing 9007199254740993 to a JS number value using the ROUND_HALF_UP rounding mode, it would round up. But it doesn't.

In fact, x.toNumber() appears to get the string representation of x and then rely on the JS engine to parse that string to a number, ignoring ROUNDING_MODE entirely.

When there are fewer than 20 significant digits, JS engines are required by the JS specification to use ROUND_HALF_EVEN ("This procedure corresponds exactly to the behaviour of the IEEE 754-2019 roundTiesToEven mode").

But when there are more than 20 significant digits, in some cases it's actually up to the engine which of the two nearest neighbors to use, so this isn't a well-defined operation.

shuckster commented 8 months ago

Type 9007199254740993 into a JavaScript REPL. What do you get?

bakkot commented 8 months ago

JavaScript uses ties-to-even semantics. Here I've configured this library to use half-up semantics, and I expected that to apply to this operation.

MikeMcl commented 8 months ago

As per the documentation, x.toNumber() does not use ROUNDING_MODE and is the equivalent to Number(x).

bakkot commented 8 months ago

Shouldn't it? Is there a reason it doesn't? It seems like if you're doing your operations with a particular rounding mode, and then trying to get a Number at the end, you'd probably want the rounding mode to apply to the process of getting the Number as well. I did, at least.

shuckster commented 8 months ago

Type 9007199254740993 into a JavaScript REPL. What do you get?

The reason I asked this is because the language itself cannot, using its native number-type, represent the number you wish to calculate.

To represent numbers that the native JavaScript number-type cannot, you must use strings and a library like BigNumber that can work with them.

The problem cannot be worked-around. If you want an accurate decimal number type (at this time) in JavaScript, it's strings or nothing.

bakkot commented 8 months ago

I'm not asking to represent the mathematical value 9007199254740993 in JS. I'm asking that, when I represent that value using this library, and convert it to a JS number (which necessarily entails rounding), the conversion to a JS number should respect the rounding mode configured in this library.

shuckster commented 8 months ago

Press F12 to open the Chrome console.

Type the following:

9007199254740993 === 9007199254740992

If you can explain how a library like BigNumber.js is able to solve this problem, please let us know so we can write a Pull Request to fix it.

bakkot commented 8 months ago

I know how numbers in JavaScript work. BigNumber('9007199254740993') is not a JavaScript number. As long as the library is configured with enough precision, it precisely represents the mathematical value 9007199254740993. It is perfectly possible to write an algorithm which converts that value to a JavaScript number under any choice of rounding mode.

shuckster commented 8 months ago

BigNumber('9007199254740993') is indeed not a JavaScript number, but you did say you want to convert it to one, right? In such a case, the library must consider more cases than just this single number.

Perhaps I'm missing the point, but the assumption I've been working under is that the whole purpose of BigNumber et al. is to avoid the pitfalls of IEEE-754 and MAX_SAFE_INTEGER.

Apologies, but I'm not very clear on the use-case of implementing such a library and then converting its results to the very thing we're trying to save ourselves from.

MikeMcl commented 8 months ago

I think it's a reasonable enhancement proposal on the face of it, but ultimately just isn't worth implementing.

Although straightforward, it would only make a difference when converting values with more than 15 significant digits that do not decimal-binary-decimal round-trip, so I am not convinced its limited utility justifies the extra code weight and implementation effort required. It would also be considerably slower than the current method as it would involve converting the value to binary before rounding.

There is also the issue of what should be the result of converting your example value, 9007199254740993, when the rounding mode is ROUND_HALF_EVEN, which is defined here as:

Rounds towards nearest neighbour. If equidistant, rounds towards even neighbour.

when 9007199254740992 and 9007199254740994 are both even!

bakkot commented 8 months ago

Fair enough. Might be worth documenting how toNumber chooses which JS value to use when the true value is not precisely representable?

There is also the issue of what should be the result of converting your example value, 9007199254740993, when the rounding mode is ROUND_HALF_EVEN, which is defined here as:

True! I think of ROUND_HALF_EVEN as meaning IEEE's ties-to-even, which refers to the evenness of the significand (and so it would be 9007199254740992). But that's not quite the meaning here.