{"action":"created","issue":{"url":"https://api.github.com/repos/cosmos/cosmos-sdk/issues/2888","repository_url":"https://api.github.com/repos/cosmos/cosmos-sdk","labels_url":"https://api.github.com/repos/cosmos/cosmos-sdk/issues/2888/labels{/name}","comments_url":"https://api.github.com/repos/cosmos/cosmos-sdk/issues/2888/comments","events_url":"https://api.github.com/repos/cosmos/cosmos-sdk/issues/2888/events","html_url":"https://github.com/cosmos/cosmos-sdk/issues/2888","id":383627460,"node_id":"MDU6SXNzdWUzODM2Mjc0NjA=","number":2888,"title":"Decimal Multiplication/Quo Bug","user":{"login":"rigelrozanski","id":20132176,"node_id":"MDQ6VXNlcjIwMTMyMTc2","avatar_url":"https://avatars1.githubusercontent.com/u/20132176?v=4","gravatar_id":"","url":"https://api.github.com/users/rigelrozanski","html_url":"https://github.com/rigelrozanski","followers_url":"https://api.github.com/users/rigelrozanski/followers","following_url":"https://api.github.com/users/rigelrozanski/following{/other_user}","gists_url":"https://api.github.com/users/rigelrozanski/gists{/gist_id}","starred_url":"https://api.github.com/users/rigelrozanski/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rigelrozanski/subscriptions","organizations_url":"https://api.github.com/users/rigelrozanski/orgs","repos_url":"https://api.github.com/users/rigelrozanski/repos","events_url":"https://api.github.com/users/rigelrozanski/events{/privacy}","received_events_url":"https://api.github.com/users/rigelrozanski/received_events","type":"User","site_admin":false},"labels":[{"id":322349783,"node_id":"MDU6TGFiZWwzMjIzNDk3ODM=","url":"https://api.github.com/repos/cosmos/cosmos-sdk/labels/bug","name":"bug","color":"ff6666","default":true},{"id":969553658,"node_id":"MDU6TGFiZWw5Njk1NTM2NTg=","url":"https://api.github.com/repos/cosmos/cosmos-sdk/labels/core","name":"core","color":"fafad2","default":false},{"id":1038201125,"node_id":"MDU6TGFiZWwxMDM4MjAxMTI1","url":"https://api.github.com/repos/cosmos/cosmos-sdk/labels/if-have-time%20prelaunch","name":"if-have-time prelaunch","color":"f9d0c4","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":1,"created_at":"2018-11-22T18:07:51Z","updated_at":"2018-11-23T04:20:24Z","closed_at":null,"author_association":"MEMBER","body":"There is a patch fix here: \r\nhttps://github.com/cosmos/cosmos-sdk/pull/2825/files#r235796732\r\n\r\nThere is an edge case which doesn't really make sense to me yet, it likely has to do with precision rounding... the problem is effectively goes something like this: \r\n\r\n`(1000.00000 * 100.00000) / * 100.00000 == 1001.00000`\r\n\r\nCC: @ValarDragon \r\n"},"comment":{"url":"https://api.github.com/repos/cosmos/cosmos-sdk/issues/comments/441153818","html_url":"https://github.com/cosmos/cosmos-sdk/issues/2888#issuecomment-441153818","issue_url":"https://api.github.com/repos/cosmos/cosmos-sdk/issues/2888","id":441153818,"node_id":"MDEyOklzc3VlQ29tbWVudDQ0MTE1MzgxOA==","user":{"login":"ValarDragon","id":6440154,"node_id":"MDQ6VXNlcjY0NDAxNTQ=","avatar_url":"https://avatars1.githubusercontent.com/u/6440154?v=4","gravatar_id":"","url":"https://api.github.com/users/ValarDragon","html_url":"https://github.com/ValarDragon","followers_url":"https://api.github.com/users/ValarDragon/followers","following_url":"https://api.github.com/users/ValarDragon/following{/other_user}","gists_url":"https://api.github.com/users/ValarDragon/gists{/gist_id}","starred_url":"https://api.github.com/users/ValarDragon/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ValarDragon/subscriptions","organizations_url":"https://api.github.com/users/ValarDragon/orgs","repos_url":"https://api.github.com/users/ValarDragon/repos","events_url":"https://api.github.com/users/ValarDragon/events{/privacy}","received_events_url":"https://api.github.com/users/ValarDragon/received_events","type":"User","site_admin":false},"created_at":"2018-11-23T04:20:24Z","updated_at":"2018-11-23T04:20:24Z","author_association":"MEMBER","body":"I do agree with in general leveraging more open source software. However just because something exists, doesn't mean its good. In this case, I actually don't find this library good, especially relative to what we have. (I have this gripe in general for a lot of the simple golang data structure libraries)\r\n\r\nIt has one global variable for precision: https://github.com/shopspring/decimal/blob/master/decimal.go#L45. Theres no benchmarks on any of the meaningful functions. (w.r.t. to what we'd use) It is fundamentally designed for decimals with variable precision, our use case is many decimals with the same precision. I am fairly confident ours will be more efficient for that reason. In the future, I think we should strive for less re-allocation (e.g. big int style) as we re-allocate all over the place. (I'd ideally like to aim for a big-int pool / decimal pool) I also think utilizing SIMD would be cool for the operations. (Our 256 bit decimals fit perfectly into AVX2 SIMD registers, the underlying golang big int takes a perf hit by not getting to know max word size before the computation.) \r\n\r\nBack to this library, I fail to see what it does that our implementation doesn't already do. We already have something thats been tailored to our use case, I don't see a reason to switch to the more general case. IMO, that libraries core benefit would be implementations of functions we don't use atm. (Different rounds, math funcs, etc.) but that seems like bloat for the sdk type impl.\r\n\r\nThats an interesting edge case rigel! I'll take a look tomorrow."}}
{"actor":{"display_login":"ValarDragon"}}