Proposal: Use shopspring/decimal instead of bespoke decimal interface

[sdcoffey]

Shopspring's decimal is undoubtedly a better interface than my own big I propose we migrate away from big towards the better decimal impelmentation

[tomarus]

Hey, i've just ported this package to use shopspring/decimal but i run into some issues.

1. The shopspring pkg does not support Sqrt() yet, but a PR is currently open and being worked on so this will probably be ready in a few weeks. See Added Sqrt and SqrtRound functionality shopspring/decimal#130 I just copied the (apparently broken) Sqrt() from the PR and it seems to pass all tests lol.

2. Shopspring doesn't support Inf. I've fixed this using the following but i have no idea if that's mathematically even the correct way to do this:

var plusInf = decimal.NewFromInt(math.MaxInt64)
var minInf = decimal.NewFromInt(-math.MaxInt64

3. There is a HUGE increase in resources using shopspring/decimal, especially memory allocation. This is especially noticable in reecursive calculations, like MMA. I noticed this immediately when i had my own app ported. In fact it is also noticable by just running the tests.

This is the time for the tests to run on your own big package:

$ time go test -cover
PASS
coverage: 98.5% of statements
ok  	github.com/sdcoffey/techan	0.040s

real	0m0.978s
user	0m2.532s
sys	0m0.549s

But when converted to shopspring/decimal it's more like this:

$ time go test -cover
PASS
coverage: 98.0% of statements
ok  	github.com/tomarus/techan	20.757s

real	0m21.720s
user	0m26.858s
sys	0m1.660s

So that's like 20 times slower and 20 times the memory usage (don't have real stats from that).

I tried messing around with the MMA calculations a bit but it probably needs to be rewritten completely, somehow without recursion or so, But i'm really not knowledgeable about this.

Also i'm not sure where to go from here. I really like shopspring/decimal but not at this cost. Just wanted to let you know above findings. And thanks a lot for this package, it's really useful!

Here's my port if anyone wants to check it out: https://github.com/tomarus/techan/tree/decimal

[sdcoffey]

Wow, this is super insightful @tomarus! Thanks for the in-depth review. I think this would be part of a larger overhaul as a move toward v1, and definitely not something we'd do on a whim. Will definitely keep this in mind going forward