-
Notifications
You must be signed in to change notification settings - Fork 226
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
lgamma slow with defaults #241
Comments
We need to look at this - when that function was written (and the library started), there was negligible performance difference between double and the old x87 long double, so it was worth using internal promotion for accuracy. However, as you rightly point out, things have moved on and there is now a very significant difference between x64 floating point and x87 code. It's a one line change to change the default, but it would break all our tests as the expected errors are predicated on internal promotion taking place. What does everyone else feel about this as a breaking change? |
If your thought is to use (performance issues with threaded programs using |
No there's too much variation between platforms IMO. |
I think it should be done, and there should be a slow but steady commitment to doing the same with every function in the library. I don't think the breaking change will really affect many people-half ULP accuracy which can be achieved with long double promotion is not an expectation many people have. |
uh... I misread... you are suggesting to change the default policy to avoid the promotion. Sounds good to me given that functions still work fine. (FYI, the digamma functions runs about 2x faster) |
The
boost::math::lgamma
implementation is about 2-3x slower than thestd::lgamma
implementation when used with standarddouble
s.When turning off the do the propagation from
double
tolong double
the boost implementation is comparable in speed tostd::lgamma
, but still 10-20% slower.However, when turning off propagation to
long double
problems with early overflow are the price to pay... see the next issue.The text was updated successfully, but these errors were encountered: