Shopify / measured

Encapsulate measurements and their units in Ruby and Ruby on Rails.
MIT License
337 stars 28 forks source link

Use Rational instead of BigDecimal? #63

Open thegedge opened 7 years ago

thegedge commented 7 years ago

There's no loss in operating with either, but there are pros and cons we need to weight against each other:

Representation

Rational has the benefit of describing infinitely repeating rationals exactly (e.g., 1/3, 4/7), but cannot represent infinity.

String parsing

Rational and BigDecimal have different profiles based on the type of string being parsed. Rational is a little faster for integral values, but much slower for fractional values.

Integral

Benchmark.ips do |x|
  FOO = "10127398"
  x.report("to_r") { FOO.to_r }
  x.report("to_d") { FOO.to_d }
  x.compare!
end
Warming up --------------------------------------
                to_r   117.596k i/100ms
                to_d   100.975k i/100ms
Calculating -------------------------------------
                to_r      1.913M (± 5.4%) i/s -      9.643M in   5.058549s
                to_d      1.488M (± 5.4%) i/s -      7.472M in   5.038070s

Comparison:
                to_r:  1912752.0 i/s
                to_d:  1488150.4 i/s - 1.29x  slower

Non-integral

Benchmark.ips do |x|
  FOO = "10.127398"
  x.report("to_r") { FOO.to_r }
  x.report("to_d") { FOO.to_d }
  x.compare!
end
Warming up --------------------------------------
                to_r    58.234k i/100ms
                to_d    97.379k i/100ms
Calculating -------------------------------------
                to_r    715.125k (± 4.3%) i/s -      3.611M in   5.058701s
                to_d      1.447M (± 3.8%) i/s -      7.303M in   5.056448s

Comparison:
                to_d:  1446574.0 i/s
                to_r:   715125.3 i/s - 2.02x  slower

Arithmetic

Rational has much better performance for arithmetic.

Benchmark.ips do |x|
  VALUE = "123.512"  
  R = VALUE.to_r
  BD = VALUE.to_d

  x.report("rational") { R + 10*R }
  x.report("bigdecimal") { BD + 10*BD }
  x.compare!
end
Warming up --------------------------------------
            rational    84.517k i/100ms
          bigdecimal    40.148k i/100ms
Calculating -------------------------------------
            rational      1.166M (± 4.1%) i/s -      5.832M in   5.008772s
          bigdecimal    506.024k (± 3.5%) i/s -      2.529M in   5.004639s

Comparison:
            rational:  1166439.0 i/s
          bigdecimal:   506023.8 i/s - 2.31x  slower
kmcphillips commented 7 years ago

I'm completely 👍 for representing as rational internally. Helps us avoid precision loss. And is much more in spirit with the lazy conversion. Do the changes/math just in time.

thegedge commented 7 years ago

And is much more in spirit with the lazy conversion. Do the changes/math just in time.

Could you expand on what you mean by "lazy" here? Do you mean we'd store given values and only convert to Rational when first necessary?

kmcphillips commented 7 years ago

The other way around. We store all factors/conversions in rational, and only convert to BigDecimal or whatever when terminal.

thegedge commented 7 years ago

I'm removing the 2.0 milestone. Doing this is backwards compatible in the sense that no contract has been established about exactly what we return here. Our README calls out that it could be either, so people will (hopefully) be writing code that can work with both.