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

BigDecimal with division and other operations which need to round #13

Open
littledan opened this issue Nov 13, 2019 · 18 comments
Open

BigDecimal with division and other operations which need to round #13

littledan opened this issue Nov 13, 2019 · 18 comments

Comments

@littledan
Copy link
Member

Division is a bit of a conundrum with arbitrary-sized decimal types. This was discussed in the 2017 TC39 presentation. There are two basic approaches I've seen in the ecosystem:

  • Java-like: Require that the user provide the precision and/or rounding mode as a parameter when performing division
  • Ruby-like: Just choose an arbitrary amount of extra decimal places to use for the division result
  • The implicit third option is to avoid division, and instead have a div/mod operation, or a "partition" operation (to split a BigDecimal into an integer number of buckets as equally as possible, at the precision of the BigDecimal)

I'm not sure what to do here. I've even considered whether this is the straw that breaks the camel's back, requiring 128-bit IEEE 754 decimal. What are your thoughts?

@qzb
Copy link

qzb commented Nov 17, 2019

I'm not entirely sure if it's a good idea, but there is another option (or rather a variant of Java-like approach) - use precision based on product of precisions of the multiplication factors:

1m / 3m === 0m
1.00m / 3m === 0.33m
1.00m / 3.00m === 0.3333m

I'm not sure if it's intuitive enough, but it's for sure more convenient than using method instead of operator, it gives more control than Ruby-like approach, and it's a nice parallel to multiplication.

@littledan
Copy link
Member Author

littledan commented Nov 17, 2019

@qzb Thanks for including this option; we should definitely be considering this option among the three others above. It makes sense formally, considering the precision rules for other operators.

Personally, I find 1m / 3m === 0m to be confusing enough (for a feature which is all about preserving decimal precision) to cast some significant doubt on this version.

@qzb
Copy link

qzb commented Nov 17, 2019

@littledan Even worse, according to these rules 1m / 2m === 0m, even thought it have not only finite but also short decimal expansion. Maybe some compromise can be made, e.g.: quotient precision could be a maximum of some predefined value (let say 10) and of sum of precisions of dividend and divisor:

1.000m / 3m = 0.333333333m // max(10, 3 + 0) = 10
1.000m / 3.0000000000m = 0.3333333333333m // max(10, 3 + 10) = 13

It gives some control over result's precision, works with existing operator, but starts to be pretty convoluted...


BTW, it's worth mention that we can have both Java-like and Ruby-like operations, one using .divide() method and other / operator. In my opinion Java-like approach should be implemented, the only question is what method should be used for / operator (if any).

@littledan
Copy link
Member Author

@qzb This may be a reasonable compromise. I think this is what Ruby does. The ugly part is that we have to arbitrarily choose this constant (e.g., 10).

@domenic
Copy link
Member

domenic commented Nov 17, 2019

FWIW, here was my short thought process upon hearing of this conundrum:

  • It would be bonkers if you couldn't divide decimals. What if I want to split an amount of money between two people?
  • OK, but I see there is a clearly a precision problem. I guess the easiest solution is to require specifying the precision, e.g. 1m.divide(2m, { precision: 10 }).
    • I wonder what units that precision value has... bits? Decimal places? Significant figures? Ah well, someone smart will figure that out.
  • But it's so un-ergonomic to have to drop down to .divide() for a common-ish operation like this. / should really work. I guess the only way out of this is to pick a default precision value which / uses, i.e. to explain that xDec / yDec is shorthand for xDec.divide(yDec, { precision: SOME_DEFAULT }).

@MaxGraey
Copy link

What about static field which could read / write in BigDecimal namespace like BigDecimal.MIN_DECIMAL_PRECISION?

@qzb
Copy link

qzb commented Nov 17, 2019

@domenic Actually splitting money is not a good use case for dividing. Let's assume you are trying to split 10$ between three people, you have to end up with one person which pays 3.34$ instead 3.33$. When comes to splitting money you want some kind of div/mod or partition operation described by littledan in 3rd point of this issue. It could work like this:

const [ q, r ] = (10m).divmod(3m, { precision: 2 }) // q === 3.33m, r === 0.01m
const [ a, b, c ] = (10m).partition(3n, { precision: 2 }) // a === 3.33m, b === 3.33m, c === 3.34m

@qzb
Copy link

qzb commented Nov 17, 2019

@MaxGraey AFAIK currently specification doesn't include even single (writable) constant like this and adding one would probably be pretty controversial. Also it sounds like a method for producing really nasty bugs, since any of your dependencies could change it.

If some minimal precision is chosen we should definitely add constant like this, Number already have 8, but it should not be writable.

@littledan
Copy link
Member Author

Yeah, I don't think we can have a global setting for these sorts of things. For one, primitives live outside of any particular JS global object, so they couldn't even reference that. But stepping back a bit, global settings are just the enemy of composability of different pieces of code, which is what the JS ecosystem is based on.

@mmis1000
Copy link

mmis1000 commented Nov 18, 2019

Just for curious, why not big fraction (someone actually implemented that with big int) instead?
BigDecimal is effectively fraction but allow only 10 based denominator.

If non 10 based denominator is allowed.

1.0040 is just 10040 / 10000.
1.0040 / 3 would just be 10040 / 30000.

Not got rounded (Because you don't need to)

Allowing non 10 based fraction will also makes something 1 / 3 + 1 / 3 + 1 / 3 works as anyone would expect(the computation will be more expensive though).

@qzb
Copy link

qzb commented Nov 18, 2019

@mmis1000 It's called Rational, and it's discussed in issue #6

@Skalman
Copy link

Skalman commented Feb 5, 2020

I haven't seen it proposed elsewhere, so here goes. You could have a "use ..." directive.

"use decimalDivision fractionDigits:2 round:down";

1m / 3m === 0.33m

In order to use the / operator with BigDecimals, you have a "use decimalDivision ..." directive. If there are multiple such (possibly conflicting) directives in the same function/file, use the first one.

This syntax is quite ugly though, and quite verbose. And I suppose it'd be impossible to support something similar for your own implementations if operator overloading is ever standardized.

@AnyhowStep
Copy link

AnyhowStep commented Feb 28, 2020

I wonder what units that precision value has... bits? Decimal places? Significant figures? Ah well, someone smart will figure that out.

This is really important.

Particularly for division, siginificant figures makes more sense.

In SQL,

  • precision = integer digits + fractional digits
  • scale = fractional digits
  • scale = decimal places

I quickly browsed through the README but it didn't describe "precision" well.

Using scale/fractional digits/decimal places for division doesn't sound like a good idea,

//This is just 0.00001m
const x = 1.00m / 100000.00m

Using multiplication of decimal places gives us 4 decimal places
With 4 decimal places, we end up with 0.0000m


Whereas, if we went with significant figures, we'd get 0.00001m

The question of "how many significant figures?" then comes up.

Maybe something like for a / b, we use a.fractionalDigits + b.fractionalDigits significant figures in the result.


Even though "significant figures" usually does not mean trailing zeroes, maybe it might be more intuitive to include trailing zeroes for the definition here.

The result will have a.fractionalDigits + b.fractionalDigits + number of leading zeroes in fractional part number of decimal places.

So, in the above example, maybe 0.00001000m would be a good result for 1.00m / 100000.00m

2 fractional digits + 2 fractional digits + 4 leading zeroes = 8 fractional digits in result


So, while figuring out the decimal places for addition, subtraction, multiplication is easy and intuitive, for division, it might be better to not think in decimal places but in terms of significant figures (including trailing zeroes).

@caiolima
Copy link
Collaborator

I think we can consider another option there, that is to use Ruby-like approach (i.e choose an arbitrary amount of extra decimal places) when the result is a non-terminated expansion (e.g. 1m / 3m), but then give the precise result when it is possible to do so. This allows us to use / operator and if any specific rounding is necessary, it's possible to use BigDecimal.divide(...,{fractionalDigits: 10, roundingMode: ...).

What are your thoughts on this?

@ericsedlar-zz
Copy link

The simplest use case I know of to think about decimal arithmetic is TPC-H Query 1 (simplified):
SELECT ... sum(l_extendedprice*(1-l_discount)*(1+l_tax)) as sum_charge... FROM lineitem

If you think about how a store will do this, they will round each price in your order to the nearest penny after applying discount & tax, and then sum up the list of stuff you bought. They aren't going to retain fractional cents on each item to accumulate in the sum. So generally, you want to do decimal arithmetic with some fixed precision & scale across a set of operations (for currency, scale = 2). I think it's likely that you want to specify the precision either for some large scope (eg. everything nested within this block, global/thread scope, etc.) or you specify it when you create / declare a variable (so you can create a currency "type" with scale = 2 for arithmetic on currency). Specifying the precision on each divide operation is really horrible, and likely to create bugs for cases where you really want this large class of objects to be operated on with a particular precision/scale.

@TehShrike
Copy link

Fixed-precision division is a real brain-bender.

But sometimes, I really want it.

Given these two realities, I would be totally happy with a .divide(number, precision) method – it means I have the ability to divide, and we don't have to come up with some new inscrutable syntax or magical behavior around precision.

@JounQin
Copy link

JounQin commented Apr 12, 2023

1m/3m + 1m/3m + 1m/3m = // ?

What's the result here?

@jessealama
Copy link
Collaborator

jessealama commented Aug 30, 2023

1m/3m + 1m/3m + 1m/3m = // ?

What's the result here?

It would be 0.99...999, with the number of 9s determined by some kind of context paramter/second argument. (Of course,)

Fixed-precision division is a real brain-bender.

But sometimes, I really want it.

Given these two realities, I would be totally happy with a .divide(number, precision) method – it means I have the ability to divide, and we don't have to come up with some new inscrutable syntax or magical behavior around precision.

I agree! With division, the need for a second parameter is clear. That makes things a bit bulkier with the syntax, but so much clearer semantically that it's worth the payoff, I'd say.

(By the way, for the Decimal128 approach to decimals -- which, I realize, this thread is not about -- division does work without an extra "number of digits" argument. The number of significant digits you get in the result is bounded from above by 34, which is the number of significant digits that can be represented in a Decimal128 value.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests