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

New market API uses doubles #144

Closed
vikramrajkumar opened this issue Jan 18, 2017 · 5 comments
Closed

New market API uses doubles #144

vikramrajkumar opened this issue Jan 18, 2017 · 5 comments

Comments

@vikramrajkumar
Copy link
Contributor

vikramrajkumar commented Jan 18, 2017

From @theoreticalbts on January 7, 2016 19:22

The new market API implemented as part of cryptonomex/graphene#503 uses doubles. Use of doubles in any financial system is a worst practice, and this situation should be corrected. If #65 is finally implemented we should isolate the double's into their own API.

An alternative version is to refactor the JSON code to take a parse_float function pointer, like Python's JSON parser, and then have the floats parsed as some kind of rational or string class that doesn't lose information.

Copied from original issue: cryptonomex/graphene#504

(The description is updated to link to correct issue numbers)

@abitmore
Copy link
Member

abitmore commented Nov 9, 2017

Since those API's have been heavily used by 3rd-parties, we've missed the best time to change them. If we really want accurate data with similar functionalities, we need new API's without using doubles.

@abitmore
Copy link
Member

abitmore commented Nov 9, 2017

Just noticed that doubles will be jsonified as strings, so we can still fix the API's.

@abitmore abitmore self-assigned this Nov 13, 2017
@abitmore abitmore added this to the Future Non-Consensus-Changing Release milestone Nov 27, 2017
@oxarbitrage
Copy link
Member

we have the easy option to change the doubles to long doubles but that will not be a definitive solution. i was browsing around and found that int can be used to work around the rounding effects of floats and doubles but unsure how to implement yet. i saw the price type in our project but unsure if it will be for our needs either. any input on implementation will be very appreciated. we can replace the doubles but the question will be with what.

@abitmore
Copy link
Member

abitmore commented Dec 8, 2017

@oxarbitrage the issue is doubles are not accurate even when have high resolution. For example, in #455 (review), the test data you wrote, "amount": "1664.17390000000000327" is simply wrong and may confuse users. I'm not sure what's the best solution so far, but I tend to store and calculate with Integers and rationals, and convert them to strings (do rounding in our own code) at the last step.

@abitmore abitmore modified the milestones: Future Non-Consensus-Changing Release, 201802 - Non-Consensus-Changing Release Dec 8, 2017
abitmore added a commit to abitmore/bitshares-core that referenced this issue Jan 23, 2018
abitmore added a commit to abitmore/bitshares-core that referenced this issue Jan 23, 2018
abitmore added a commit to abitmore/bitshares-core that referenced this issue Jan 25, 2018
@oxarbitrage
Copy link
Member

closing by #594

@abitmore abitmore removed their assignment Mar 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants