BigDecimal icon indicating copy to clipboard operation
BigDecimal copied to clipboard

Adapt polyfill to current state of decimal proposal?

Open jessealama opened this issue 2 years ago • 11 comments

Jesse here, one of the co-champions of the decimal proposal! As I am preparing to give an update to the TC39 plenary, I noticed your polyfill. This looks great! There was a time when the champions were considering an arbitrary-precision model for decimals, but after a lot of discussion we've settled on a fixed bit-width approach, IEEE 754 Decimal128 in particular. Would you be interested in updating your polyfill to reflect those changes? FWIW I have an npm library that implements Decimal128 (or, more precisely, the part of Decimal128 that's relevant to the proposal as it currently stands).

jessealama avatar Jan 19 '24 14:01 jessealama

Hello Jesse! It's great to hear from you. Some thoughts:

I see that you have something like: Decimal.add(x, y[, rounding]) Decimal.subtract(x, y[, rounding]) Decimal.multiply(x, y[, rounding]) Decimal.multiplyAndAdd (x, y[, rounding]) Decimal.divide(x, y[, rounding]) Decimal.remainder(x, y[, rounding]) // where only rounding.roundingMode will be used Decimal.abs(x) Decimal.cmp(x, y) Decimal.prototype.isNegative Decimal.prototype.isNaN Decimal.prototype.coefficient Decimal.prototype.exponent Decimal.prototype.round(precision, roundingMode) Decimal.prototype.toString() Decimal.prototype.toFixed(precision[, roundingMode = "half-up"]) Decimal.prototype.toPrecision(precision[, roundingMode = "half-up"]); Decimal.prototype.toExponential(fractionDigits[, roundingMode = "half-up"]) btw, spec misses some args for toExponential, and misses toPrecision completely. Also, where is the unary negation? and why isNegative, not sign ?

I vote for something much shorter, like dec128.mul or d128.mul, otherwise it may look too bulky. What are the performance requirements? well, if it is 110 bits, then perhaps, internal usage of the BigInt to store the significand is not bad. Because Bigint has some overhead, and double-double arithmetic could be faster in case 106 bits are needed. Well... may be there are some other smart ways to implement it.

Yaffle avatar Jan 19 '24 19:01 Yaffle

@jessealama what should .coefficient/.exponent return when the value is +-0, +-Infinity, NaN ?

Yaffle avatar Jan 23 '24 09:01 Yaffle

@jessealama , why do you use Decimal.cmp not Decimal.compare if you have long names for everything else

Yaffle avatar Jan 23 '24 20:01 Yaffle

@jessealama , I have done some changes, but there are still a lot of work if to support multiplyAndAdd, reminder, and numbers without normalization

Yaffle avatar Jan 23 '24 23:01 Yaffle

@jessealama , non-normalized decimals looks pretty natural, well. i have not tested all cases

Yaffle avatar Jan 26 '24 20:01 Yaffle

@jessealama I have run the tests from https://github.com/jessealama/decimal128/tree/main/tests/dectest against my code and fixed few bugs, I see some tests fails when the result is rounded to infinity or not rounded, not sure if it is a bug of test suite or my code's bug

Yaffle avatar Feb 01 '24 09:02 Yaffle

Awesome! Thanks for taking a look. Yes, supporting non-normalized values is a bit tricky. Is there anything you need some help with? I'd be happy to take a look at a PR, for instance.

jessealama avatar Feb 16 '24 07:02 jessealama

@jessealama what should .coefficient/.exponent return when the value is +-0, +-Infinity, NaN ?

Those properties were intended to be private -- no need to worry about that!

jessealama avatar Feb 22 '24 12:02 jessealama

There's just toString, no more toFixed or toExponential.

jessealama avatar Feb 22 '24 12:02 jessealama

I vote for something much shorter, like dec128.mul or d128.mul, otherwise it may look too bulky.

why do you use Decimal.cmp not Decimal.compare if you have long names for everything else

Absolutely agree with it, filled the same ideas here https://github.com/tc39/proposal-decimal/issues/105 This is great that proposal become easy to polyfill!

munrocket avatar Mar 13 '24 20:03 munrocket

Awesome! Thanks for taking a look. Yes, supporting non-normalized values is a bit tricky. Is there anything you need some help with? I'd be happy to take a look at a PR, for instance.

not entirely true, support for non-normalized values ​​according to the ІЕЕЕ754 standard seems quite natural, I didn’t even have to change the code, the only place is exact division: If value is represented as significand*10**exponent, where significand is integer : For the sum: a+b the exponent to set for the result is min(a.exponent, b.exponent); For multiplication - sum(a.exponent, b.exponent); For division - diff(a.exponent, b.exponent) When that value of exponent cannot be kept just keep as small as possible. See examples at https://github.com/Yaffle/BigDecimal/blob/master/tests.js#L80

Yaffle avatar Mar 28 '24 19:03 Yaffle