Closed sophiec20 closed 6 years ago
Tracked this down:
boost::maths::lgamma throws an exception due to an internal numeric overflow. In detail I found the following problems:
scipy
and std::lgamma
successfully compute them (they seem to have a better implementation, but I haven't looked for the details), note that bigger values are properly workingignore_error
policy, avoids the exception and should set a proper value, but it sets the value to -inf
while in this case it should be set to +inf
at best, -inf
is problematic as it breaks the code further down when choosing the min from a set of variablesAs C++11 provides std::lgamma
I think this is the best and easiest fix (There seem to be more boost math functions that can be replaced by C++11 functions, I will create a follow-up issue for them).
Notes:
std::lgamma
is a very tiny difference faster.
std::lgamma
successfully compute them
Are you sure this exists in Visual Studio 2013? I did a quick search and could only find it in Visual Studio 2015, but maybe I'm wrong - Microsoft has made it extra hard to search the docs for old Visual Studio versions.
If Visual Studio 2013 doesn't have std::lgamma
then the fix would be for 7.0 only.
Regarding handling policies. I think I generally prefer the throw on error. We can usually take better action than trying to handle infinities in later code: you tend to quickly run into nan explosion.
I'd be completely happy to migrate to std::lgamma if it is available on all platforms and if it is the same on all platforms; we'd need to test this. Also, I think we can probably also fix this particular case using Stirling's approximation (which given the overflow is likely to be very accurate). This is something we could perhaps wrap up as a safe implementation ourselves.
I haven't checked platform support yet, it might very well be that the incomplete C++11 support for VS fooled me once again. If it's there I would be very surprised if it behaves different.
Anyway, this was a quick heads up on the issue, I am still working on it. What seems clear to me: The current try catch covers to many lines. The 1st step - if std::lgamma
is not available - is to wrap lgamma
into a separate method to improve error handling (for 7.0 this implementation would of course use std::lgamma
). At least for this case I am almost certain that we do not need any custom implementation as we do not need the lgamma value.
Found in 6.3.0
"build" : { "hash" : "a47564f", "date" : "2018-04-09T21:45:51.347364Z" },
The following job configuration gives multiple
Failed to compute BIC gain
errors.Logged 28 times (excl repeats) for the job, where: