-
Notifications
You must be signed in to change notification settings - Fork 18k
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
math: Gamma(x) for -ve half integral values < -170.5 returns +Inf not 0 #11441
Comments
This ended up being a small fix. See: https://go-review.googlesource.com/#/c/17236/ The Gamma function was checking for values < -170.5 and always returning +Inf instead of zero. Removing this check fixed the issue. For sanity, I also compared the results to tgamma from cmath with the following code: #include <iostream>
#include <cmath>
int main(int argc, char **argv)
{
double a;
for(a = 299.5; a > -300.5; a = a - 1.0)
{
std::cout << "gamma(" << a << ") = " << tgamma(a) << std::endl;
}
return 0;
} You can see the results from my change here: https://docs.google.com/spreadsheets/d/14gBcKZxs0Oh4km40BatucqDrxqEgUREpFajCUJoLf1E/edit#gid=0&vpid=A1 We now match cmath's gamma for the 600 values tested. |
CL https://golang.org/cl/17236 mentions this issue. |
On the CL it has been pointed out that there are some values that are still incorrect, namely we know of at least two values that are tested for in CPython's math test cases (https://github.com/python/cpython/blob/1fe0fd9feb6a4472a9a1b186502eb9c0b2366326/Lib/test/math_testcases.txt#L275) that fail:
I looked deeper at the Cephes C library that the Go code was based and it too incorrectly returns NaN for these values:
The current Go 1.5.1 implementation always returns +Inf for values < -170.5. |
Sorry, we lost this during the Go 1.7 cycle. |
CL https://golang.org/cl/30146 mentions this issue. |
This fix broke the 387 builder:
|
That is actually the correct value according to python!
|
@ncw thanks for that clue. |
CL https://golang.org/cl/30540 mentions this issue. |
@rsc looks perfect now - well done :-) |
@ncw How did this come up? Just curious. |
@rsc I've been porting python and its runtime to go. It is a mad plan I know ;-) I discovered the issue while running the python unit tests with my interpreter. I hope to actually release it soon, but other stuff keeps coming up! |
Se here for a demo: http://play.golang.org/p/4npToJjm79
Returns (trimmed)
The text was updated successfully, but these errors were encountered: