shopspring / decimal

Arbitrary-precision fixed-point decimal numbers in Go
Other
6.37k stars 622 forks source link

Pow doesn't work for decimal exponents #55

Open alexandrinaw opened 7 years ago

alexandrinaw commented 7 years ago

The current implementation of Pow only looks at the IntPart of the exponent, so something like 4^2.5 returns 16 :(

I'm not sure why the exponent argument is supposed to be a decimal if it actually only works for ints.

l0k18 commented 6 years ago

I also discovered this bug today and it's a real downer because I wanted to use it for compound interest calculations.

victorquinn commented 6 years ago

Sorry all, would love a PR to help resolve this :)

I'll try to get to it but perhaps the community can get to it sooner as I'm currently swamped

martskins commented 5 years ago

Any pointers on how would you go about implementing this? Also, there's probably people using this believing that the result is correct, so I think some sort of measures should be taken. How do you feel about either panic-ing (as wrong as that sounds) when a non-int decimal is given as the exponent or returning an inexact value (maybe just use math.Pow?) while we wait for a new implementation?

mediocregopher commented 5 years ago

This is still a problem, and it bit me today. Reproduction is very easy:

    two := decimal.RequireFromString("2")
    onePointOne := decimal.RequireFromString("1.1")
    fmt.Printf("%v^%v = %v\n", two, onePointOne, two.Pow(onePointOne))

should output something like 2.14355..., but just outputs 2.

Is the underlying problem that decimal exponents can't always be represented to a non-infinite number of decimal places? If so, would it make sense to do something like what is done for division in this package, where it's possible to specify how much precision you want both on the package and method level (DivisionPrecision and DivRound)?

mediocregopher commented 5 years ago

Somewhat related: I implemented the ability to take the nth root of a number: https://github.com/shopspring/decimal/pull/153.

For performing decimal exponentiation, if you know that d2 is a fraction then you can combine Pow and Root. For example:

three, four, threeQuarters := decimal.New(3, 0), decimal.New(4, 0), decimal.RequireFromString("0.75")

// broken
three.Pow(threeQuarters)

// works
three.Pow(three).Root(four)
mwoss commented 4 years ago

Thanks for creating this issue @alexandrinaw. This "bug" should be at least documented in code, because as @martskins said many people use this believing that the result is correct. I'm adding this issue to the backlog and try o fix it before v1.2.0 release

bartmika commented 4 years ago

Issue still exists, I ran:

go get -u github.com/shopspring/decimal

I have v1.2.0 installed and this error still exists.

juby-gif commented 4 years ago

+1

zhengjiejiang commented 4 years ago

+1

mwoss commented 4 years ago

Yes, we know that it's still not a resolved issue. We weren't able to fix it with 1.2.0 release. Recently, I had a lot of o things on my mind and I wasn't able to focus on decimal development. Apologize for that. I will try to work on the issue this weekend. Please be patient

bartmika commented 4 years ago

@mwoss Hello how are you? I was wondering how the progress is coming along with this issue? Thank you!

mwoss commented 4 years ago

Hi @bartmika. This week I had my last uni assignments and other Master's degree related stuff and now I'll be free to invest some time into decimal library development. I have a draft implementation on my local branch. I need to test it correctly and refactor code overall before I create a pull request with it. Thank you all for your patient!

bartmika commented 4 years ago

Dziękuję bardzo!

mwoss commented 4 years ago

To inform everyone who is waiting for proper Pow implementation. Today I started my 3rd attempt to implement a satisfying solution. This time I will base the code on already existing Python and Java implementations. My two previous attempts failed miserably, they were too buggy and I couldn't figure out how I could fix all the issues. Both Python and Java implementation are battle-tested and were written by ultra-smart people, so hopefully, by using those resources I will be able to deliver working Pow solution.

tooolbox commented 4 years ago

Ouch, yeah this was first noticed 3 years ago. As far as I can tell this is the most popular decimal library for Go so would be good to have this working :)

Thanks for the update!

mwoss commented 4 years ago

Next update. I wasn't able to push the implementation any further this week as I'm preparing for coding interviews - yea potential job change. Hopefully, everything will go well and I will be able to get back to this issue next Friday. Stay tuned, and I apologize once again that you have to wait so long for this library feature.

Edit: Jeez, finding new job is more time-consuming than I thought. Still in progress. Hopefully, I will do some coding tomorrow, but for sure I won't be able to finish it yet.

Edit2: Last two weeks were a true rollercoaster. I hardly could find any time to work on personal projects/ideas. High hopes, next week would be more stable. (Is this issue slowly becoming my personal diary? :|)

RedClusive commented 4 years ago

There is a Java Math.BigDecimal Implementation of Core Mathematical Functions with a good theory explanation. Maybe this can help. (see "download" and "Ancillary files" on the right side)

https://arxiv.org/abs/0908.3030v3

mwoss commented 4 years ago

Thanks @RedClusive that will be helpful for sure! :D

rowanseymour commented 3 years ago

This bit us recently in production. I get that implementing this is hard but this limitation really needs to be flagged in the documentation. Or even better replace that method with a PowInt(int) until you have a working version with decimal exponents.

mwoss commented 3 years ago

I apologize for any inconvenience caused by the current Pow implementation. We will update the documentation at least for now.

bartmika commented 2 years ago

Hi @mwoss, just wanted to check in and see how this issue is coming along?

rowanseymour commented 2 years ago

Please if you're not going to fix it, just remove it. Better to break compatibility than let users unwittingly use a function we know is wrong.

l0k18 commented 2 years ago

I contributed this exact function in Go to rosetta code. Well, I can't find it in there, weirdly hard to find floating point exponentiation.

https://steemit.com/tutorial/@gopher23/power-and-root-functions-using-big-float-in-golang

Yes I wrote that. Based on a C version if I remember correctly. I think the same code can be switched to work with float32 and float64, for more performance less precision.

I am pretty sure somewhere I wrote one for fixed precision. Yes, here https://github.com/l0k18/float256/blob/master/float256.go

It was intended to be used as a deterministic cryptocurrency high precision math library.

mwoss commented 7 months ago

Hi all! I apologize it took me so long, but finally, I improved the Pow operation. Now it correctly calculates power operation for all decimals. In addition to that I've introduced three new methods - PowWithPrecision, PowInt32, and PowBigInt. You can take a look at the implementation - https://github.com/shopspring/decimal/pull/358.

It was quite a challenging task, to be honest, especially when we had to care about correctness and high precision. In addition to Pow improvements, I've implemented natural exponent and natural logarithm methods that you can also leverage in your workflows.

In days I will release the 1.4.0 lib version. I will need to work on small doc improvements and merge a few PRs created by the community.

Thanks for waiting!

mwoss commented 6 months ago

We released 1.4.0 a moment ago. You can now try a new Pow implementation :D