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

Support fixed-point math #522

Open
stonemonk opened this issue Nov 9, 2017 · 40 comments
Open

Support fixed-point math #522

stonemonk opened this issue Nov 9, 2017 · 40 comments
Assignees
Labels

Comments

@stonemonk
Copy link

You want the most precision possible when dealing with a users money. In particular, methods for placing orders and doing deposits/withdrawals should accept decimal.Decimal for the relavant arguments, return values and should be used internally for any calculations they perform.

@kroitor kroitor self-assigned this Nov 9, 2017
@kroitor kroitor changed the title Support decimal.Decimal in APIs that deal with a users actual money. Support fixed-point math Nov 9, 2017
@kroitor
Copy link
Member

kroitor commented Nov 9, 2017

Thanks, for your suggestion! We agree completely! As you may know, this library is under active development. Switching to fixed point math is on our roadmap. Related issue: #393.

By the way, your PRs and contributions are welcome! We will be happy if you could help us with that. But please note that apart from Python we support a few other languages as well. So, we have to make it portable. Therefore this is a little more involved than just switching to decimal.Decimal (more about it).

We hope to deliver it soon. Will let you know as soon as fixed-point math is integrated into it.
Stay tuned!

@kroitor kroitor removed the question label Nov 9, 2017
@stonemonk
Copy link
Author

So I was proposing that CCXT switches from python float (which being backed by C double type, is 64bit on nearly every platform) to decimal.Decimal which is 128bit and would be suffient precision for working with currencies and is a more natural way of working with inherently floating point amounts (compared to trying to convert to fixed point).

I take it that you have a preference for using fixed point math, but this seems like it could be challenging if any calculations are required and because different symbols may require a different base unit. For BTC based symbols, you could use https://en.bitcoin.it/wiki/Satoshi_(unit) ints for example, but then what about symbols based on ETH or USD?

Wondering about your general thoughts about how to approach this topic. I dunno about the transpiling and other langs, but to use decimal.Decimal in the native python version would be pretty simple to switch to.

@kroitor
Copy link
Member

kroitor commented Nov 11, 2017

I take it that you have a preference for using fixed point math, but this seems like it could be challenging if any calculations are required and because different symbols may require a different base unit. For BTC based symbols, you could use https://en.bitcoin.it/wiki/Satoshi_(unit) ints for example, but then what about symbols based on ETH or USD?

We avoid rate conversions inside the library, therefore we don't really care about the base unit. I think we should approach it mathematically, not financially. Therefore a fixed-point solution seems to fit our needs better than big integers. Whatever way we resolve this, we still believe that the fix should be independent of any particular currency.

I dunno about the transpiling and other langs, but to use decimal.Decimal in the native python version would be pretty simple to switch to.

Not really, because there's a lot of exchanges (up to a half) that do return fp-numbers in JSON... So we will have to convert from float to decimal.Decimal, and that in itself introduces some problems due to float-rounding and precision issues.

@stonemonk
Copy link
Author

stonemonk commented Nov 11, 2017

Therefore a fixed-point solution seems to fit our needs better than big integers.

I don't understand this, "fixed-point" and "big integer" mean the same thing to me. Fixed-point normally refers to use of integer math by working with numbers at a certain scale. e.g. multipling all prices and quantities by 10 million, so you can represent it as a big integer and thus avoid using floating points and thereby avoiding float precision issues. I was suggesting that this would be difficult because you might not be able to find an appropriate base scale for all currencies (and for the quanitity/amount field for orders!)

Personally I would stick to a floating point solution, but just upgrade the datatype from a 64bit float to 128bit decimal.Decimal.

Not really, because there's a lot of exchanges (up to a half) that do return fp-numbers in JSON... So we will have to convert from float to decimal.Decimal, and that in itself introduces some problems due to float-rounding and precision issues.

You can work around this easily. The standard JSON parser has a way of customizing the float handling, specifically:

response.json(parse_float=lambda f: f)

This tells it to not try to parse unquoted floats as float, but rather just return it as a string. Then you can just call float(str_dec_field) or decimal.Decimal(str_dec_field) to preserve the original precision. Or if you want fixed point math, something like this:

scale = decimal.Decimal("100000000")
int_field = int(decimal.Decimal(str_dec_field) * scale)

If you wanted, you could also do that with string manipulation (detect and shift the decimal point character in the string, then convert directly to int) to avoid the use of decimal.Decimal completely. But again I would urge you to stick with floating point math and just upgrade the precision by using a 128bit floating point type.

@kroitor
Copy link
Member

kroitor commented Nov 11, 2017

I don't understand this, "fixed-point" and "big integer" are the same to me.

I may be using the wrong term here, but what I mean under "fixed-point" is being able to do the following string-wise operation:

"0.0000000000000000000000000000000000000000000000000000000001"
x
"1234567891234567891234567891234567891234567891234567891234.0"

So we don't have to choose the base unit or to convert to integers or whatever. The only thing we need is an arbitrary-precision arithmetics util. We don't really care whether it stores those values as integers inside the util or not, the only thing we care about is getting adequate string representations of arbitrary-precision floats and having the ability to do basic arithmetics with those representations. To me, the underlying mechanics looks like something that isn't based on integers solely. We don't need BigInts, we want BigNumbers.

Fixed-point normally refers to use of integer math by working with numbers at a certain scale. e.g. multipling all prices and quantities by 10 million, so you can represent it as a big integer and thus avoid using floating points and thereby avoiding float precision issues.

We believe, that mathematically, having a solution for arbitrary-precision numbers, we don't need to choose the scaler. This really should be currency-agnostic. The added complexity for arithmetics will definitely slow us down, so our main concern here is the efficiency of the math-util in question.

@stonemonk
Copy link
Author

Ahh I see, I hadn't even considered that option. Well that would certainly work if you can find a suitable library. But using 128bit decimals would also be sufficient and is a built in native for python so it seems like a more natural choice. I guess JS and PHP do not have 128bit float support built in though.

Maybe this is a suitable JS lib which could be transpiled?: https://github.com/MikeMcl/big.js/

It might be best to always use strings for CCXT API input/output as to hide the underlying details, thus allowing the user to chose a suitable representation in their own code.

You mention efficiency, but I don't think its a concern as you only really want to use it when dealing with a users money (create orders, fetch orders, fetch balances). For tickers, candles, etc there is no real reason to use higher precision.

@stonemonk
Copy link
Author

stonemonk commented Nov 15, 2017

Just want to say that I was incorrect when I said that Python's built in decimal.Decimal type was 128bit (I was thinking about the mongo Decimal type).

decimal.Decimal is in fact arbitrary precision (defaults to 28 decimal places but is adjustable), and would thus be a more reasonable choice for the Python version of CCXT than using a third party library.

@kroitor
Copy link
Member

kroitor commented Nov 15, 2017

@stonemonk , we have a concern on decimal.Decimal, because from Py2 docs it looks like it uses a global context in a dirty manner. If there's no other way to specify rounding/truncation rules, that global context might be a showstopper:

screen shot 2017-11-15 at 19 50 37

What do you think of the above?

@stonemonk
Copy link
Author

I am not working in Py2, but I don't think it's a problem. Any body else's code using decimal should be setting precision already prior to use. But if you are concerned about this you can always save the original precision first then restore it afterwards.

If the quantitize() method works in Py2, you might be better off using the default context precision of 28 decimals and only quantitizing down when needed..

@kroitor
Copy link
Member

kroitor commented Nov 15, 2017

@stonemonk looks like we can use decimal.Decimal in both Pythons with default precision + quantize, it should cover all values, hopefully. Thx!

@Bomper
Copy link
Contributor

Bomper commented Dec 19, 2017

Have you considered using BigNum for the JavaScript library? Floats are dangerous (precision loss) and strings lead to concatenation instead of addition (e.g. order.size + 10 may be "3010" if order.size was "30").

@kroitor
Copy link
Member

kroitor commented Dec 19, 2017

@Bomper yes, in fact, all languages are subject to that, so we are preparing a generalized solution to this issue. Will let you know upon progress.

@gaardiolor
Copy link

Would realy like this to be implemented too, using float all my calculations go wrong. Now I have to cast al floats to decimals, and back to float again when trading...

@gaardiolor
Copy link

python module simplejson supports decimal import / exports. Easy workaround:

import ccxt
import simplejson as json

e = ccxt.bittrex()
decimal_order_book = json.loads(json.dumps(e.fetch_order_book('ETH/BTC')), use_decimal=True)

@kroitor this should be pretty easy to implement in ccxt (at least for python sync / async) ?

  • use simplejson instead of json
  • make use_decimal configurable

@kroitor
Copy link
Member

kroitor commented Dec 23, 2017

@gaardiolor if I do this without changing all the code that works with precision for order submission, withdrawal submission, not only the parsing, but also all other aspects of the library, this would mostly break it unusable. There isn't an "easy one-liner" to fix for this, otherwise I would have done it three times already. It requires a more thorough design, and this is the core code of the lib, therefore it has to be designed with double care. We need to have base methods for arbitrary precision arithmetics first.

@gaardiolor
Copy link

OK, totally trust your judgement of course, I'm pretty new to programming. Awesome piece of work this library.

@damian123
Copy link

Floats should never be used for money but ccxt forces it, will this task be finished?

@frosty00
Copy link
Member

@damian123 you can use exchange.number = str or exchange.number = decimal.Decimal it is almost finished

@timolson
Copy link
Contributor

timolson commented Mar 31, 2022 via email

@pietrodn
Copy link
Contributor

pietrodn commented Mar 31, 2022

In what sense do some exchanges use float? The REST APIs use JSON numbers, that can be (de)serialized as Decimal, maintaining precision.

The only issue would be if they used binary-encoded IEEE 754 numbers, but I don’t know if any exchange that does that…

@kroitor
Copy link
Member

kroitor commented Mar 31, 2022

@pietrodn it's more about the performance of native JSON parsers like JSON.parse() vs a custom parser that would be required to treat JSON numbers as text-strings, which is why we just quote long numbers and feed them into the native parser as quoted strings.

@timolson
Copy link
Contributor

timolson commented Mar 31, 2022 via email

@frosty00
Copy link
Member

frosty00 commented Apr 1, 2022

@timolson the exchanges sends a json string that contains numbers. At this point the numbers are still strings, we simply feed those strings to the .number constructor without parsing it as a float first, hope that clears things up for you!

@gshpychka
Copy link

gshpychka commented Jul 19, 2022

@damian123 you can use exchange.number = str or exchange.number = decimal.Decimal it is almost finished

I've tried it, but I'm getting an error when trying to create an order with Decimal price and size:

File "/usr/local/lib/python3.9/site-packages/ccxt/okcoin.py", line 2071, in create_order
    request['price'] = self.price_to_precision(symbol, price)
  File "/usr/local/lib/python3.9/site-packages/ccxt/base/exchange.py", line 2952, in price_to_precision
    return self.decimal_to_precision(price, ROUND, market['precision']['price'], self.precisionMode, self.paddingMode)
  File "/usr/local/lib/python3.9/site-packages/ccxt/base/decimal_to_precision.py", line 39, in decimal_to_precision
    assert(isinstance(precision, float) or isinstance(precision, numbers.Integral))
AssertionError

@frosty00 passing this function without the assertions to exchange seems to work. Maybe a good idea to remove them, or account for Decimal?

@ttodua ttodua added the important Higher priority label Jul 22, 2022
@gshpychka
Copy link

@kroitor would it make sense to adjust that check to support Decimal?

@gshpychka
Copy link

@ttodua what's required to progress this issue? Right now the error above is the only thing that's keeping me with using Decimal in Python.

@samgermain samgermain self-assigned this Aug 16, 2022
@samgermain
Copy link
Member

@ttodua what's required to progress this issue? Right now the error above is the only thing that's keeping me with using Decimal in Python.

I'll push a PR later today that does a lot of the changes, and after that, for all the exchanges that you want to use, I would make sure that all the arithmetic on prices uses methods like Precise.stringAdd, Precise.stringMul... instead of +,* ...

@samgermain samgermain linked a pull request Aug 16, 2022 that will close this issue
@samgermain
Copy link
Member

Changes like the ones in this PR need to be made across all exchanges. I can get all the one that use Math. pretty easily by using a search, but catching all the +, -, * / could be a bit tricky and might require some Beta use

@gshpychka
Copy link

@samgermain not sure I understand completely - assigning Decimal to exchange.number already means that number operations will be fixed-point, doesn't it? It seems to work fine, but the error I posted above is what's still broken.

@gshpychka
Copy link

@samgermain can you explain?

@samgermain
Copy link
Member

@samgermain can you explain?

Don't worry about it, I've been working on adding string math across the library, should hopefully be done soonish

@gshpychka
Copy link

gshpychka commented Sep 16, 2022

@samgermain can you explain?

Don't worry about it, I've been working on adding string math across the library, should hopefully be done soonish

The link leads to a 404 for me.

Can you address my question above - assigning Decimal to exchange.number already enables fixed-point math, but the error I posted above is still broken.

@gshpychka
Copy link

@samgermain can you please address the question?

@samgermain
Copy link
Member

samgermain commented Sep 26, 2022

@damian123 you can use exchange.number = str or exchange.number = decimal.Decimal it is almost finished

I've tried it, but I'm getting an error when trying to create an order with Decimal price and size:

File "/usr/local/lib/python3.9/site-packages/ccxt/okcoin.py", line 2071, in create_order
    request['price'] = self.price_to_precision(symbol, price)
  File "/usr/local/lib/python3.9/site-packages/ccxt/base/exchange.py", line 2952, in price_to_precision
    return self.decimal_to_precision(price, ROUND, market['precision']['price'], self.precisionMode, self.paddingMode)
  File "/usr/local/lib/python3.9/site-packages/ccxt/base/decimal_to_precision.py", line 39, in decimal_to_precision
    assert(isinstance(precision, float) or isinstance(precision, numbers.Integral))
AssertionError

@frosty00 passing this function without the assertions to exchange seems to work. Maybe a good idea to remove them, or account for Decimal?

I created a pull request that should fix your specific issue. If you run into any more issues regarding this topic please let me know and I can try to solve them.

Using decimal.Decimal will not work on every exchange yet. There are a number of PR's that need to be merged before that will be possible.

After all these PRs are merged, and after finding any remaining bugs, using decimal precision will be an option across the library

@samgermain
Copy link
Member

@samgermain can you please address the question?

The fix to your problem was merged, so you should be able to use the Decimal type with okcoin. Using the Decimal type will not be completely possible for all exchanges however until the PRs listed above get merged.

@samgermain
Copy link
Member

PRs related to this issue are linked in https://github.com/orgs/ccxt/projects/52

Once all PRs in that project are merged, bugs related to this issue will be intermittant, and solved as they are discovered, but string math should be universally available

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

Successfully merging a pull request may close this issue.