coblox / nectar

GNU General Public License v3.0
0 stars 1 forks source link

Improve calculations #16

Closed D4nte closed 4 years ago

D4nte commented 4 years ago

Improve mathematics by avoiding floating point calculations and state expectations around precision.

TODO: property testing.

D4nte commented 4 years ago

Before I actually jump in and review the maths, I wonder if it would be possible to use something like this or this to avoid some of the complexity of handling floats. Did you already consider that? Would it help at all?

I considered it but the value we receive from Kraken may not be a rational fraction or ratio. Also I felt it would be faster to roll out what I did then learn the limitations and usage of the quoted libs.

D4nte commented 4 years ago

All this floating point arithmetic is really scary :/

I don't understand what you are talking about. This PR moves most arithmetic to unsigned integer.

luckysori commented 4 years ago

I considered it but the value we receive from Kraken may not be a rational fraction or ratio.

num_rational does provide https://docs.rs/num-rational/0.3.0/num_rational/struct.Ratio.html#method.from_float and https://docs.rs/num-rational/0.3.0/num_rational/struct.Ratio.html?search=#method.approximate_float to deal with that. And so does the other crate.

Also I felt it would be faster to roll out what I did then learn the limitations and usage of the quoted libs.

If you're confident writing that stuff then that is fair, but I'm certainly not very confident in my ability to review it :face_with_head_bandage:

D4nte commented 4 years ago

Will use existing crate.

D4nte commented 4 years ago

BigRational is not an option because it loses precision.

D4nte commented 4 years ago

Can we discuss the idea of not converting to a floating point rate at all?

I do not understand how you intend to manipulate decimal numbers without using floating point representations.

We have two requirements that uses decimal numbers:

Please write the (pseudo) code to demonstrate how you would implement this requirements without dealing with floating points.

thomaseizinger commented 4 years ago

Can we discuss the idea of not converting to a floating point rate at all?

I do not understand how you intend to manipulate decimal numbers without using floating point representations.

We have two requirements that uses decimal numbers:

* The value/rate from Kraken.

* Applying a spread.

Please write the (pseudo) code to demonstrate how you would implement this requirements without dealing with floating points.

To my understanding, the "don't use floating point" advice applies to doing multiplication and other arithmetic. We can't do anything about the fact that kraken sends us a floating point value.

What we can do though is limit the usage as soon as possible, use datatypes that store exact values (u64 or u128) and use sub-cent accounting to minimize the impact of precision loss in a controlled way.

Kraken gives the amount of DAI for 1 BTC. DAI is basically USD, so it comes with 2 decimal places of precision that matter at the end of the day.

First, we have to decide on how accurate we want to be in terms of cents, let's assume that "millicents" are good enough.

  1. Multiply the value we get from kraken by 100 * 1000 (100 to get cents, 1000 to get millicents). Store this value in a u64.
  2. To apply a 3% spread on that value, we:
    • divide the u64 value by 100 to get 1%.
    • multiply the result by 3
    • add the value to the original amount

If we want sub-percent spreads, we need to use per mille instead of per cent, potentially go down to microcents etc.

We do lose precision here as well, simply because there will always be divisions what have a remainder. To my understanding, the important part is that we use datatypes that store exact values and do sub-cent accounting to make sure the precision-loss is controlled and predictable.

Edit 1: Fix 10 -> 100.

D4nte commented 4 years ago

I initially asked for the review to be focused on the WorthIn part but instead the comments were made on code that was not yet ready (the PR was initially marked as draft) and still using float arithmetic.

The approach explained above is exactly what I was doing from day one.

I shall continue on the initial approach and will mark the PR ready once everything is done.

D4nte commented 4 years ago

Will open fresh one once done.