Boost C++ Libraries: Ticket #10110: boost::random::negative_binomial_distribution unnecessary restriction to IntType for parameter k

attachment set

Tue, 10 Jun 2014 20:24:12 GMT

attachment → negbin_test.cpp

Example of double to int conversion in negative_binomial_distribution

Steven Watanabe — Thu, 26 Feb 2015 01:02:41 GMT

I'm on the fence about this. This generalization is reasonable, but it is not consistent with std::negative_binomial_distribution and the difference is subtle enough that it could result in non-obvious bugs if someone were to try replacing boost::random with std.

attachment set

Tue, 10 Mar 2015 22:21:33 GMT

attachment → test.cpp

Test paramter assertions

attachment set

Tue, 10 Mar 2015 22:23:40 GMT

attachment → test.2.cpp

Test paramter assertions

attachment set

Tue, 10 Mar 2015 22:26:31 GMT

attachment → comparison.txt

Comparison of assertion tests

Tue, 10 Mar 2015 22:29:19 GMT

Hi Steven,

thanks for your reply. I think that this implicit cast can lead to non-obvious bugs as well, since it changes the distribution in case for non integer parameter k. The standard only describes the negative binomial distribution for trials, e.g. integer parameters. So the behavior for a non integer value is unspecified by the standard. In practice, e.g. claim number modeling, real positive parameters are widely used. To avoid the implicit cast bug it would be possible to delete the constructor for non integer types. Then still no real parameters are usable (in fact they are not proper right now) but the compiler will reject it if someone tries.

Regarding the issue about the assertions. I did some tests comparing std::negative_binomial_distribution and the one from boost. Summarizing, the parameter checks are not very satisfying either, see the attached comparison.txt.

In my opinion it makes no sense that a negative_binomial_distribution with invalid parameters can be constructed and then may or may not fail during the sampling process. Especially in the case k=0 were -2^{31 is produced which can happen if someone calculates with the (in principal) valid parameter k = 0.99.}

I produced these results with g++-4.9.real (Ubuntu 4.9.1-16ubuntu6) 4.9.1 and compiled the attached example with: g++ -D_GLIBCXX_DEBUG -std=c++11 -o test test.cpp and without the _GLIBCXX_DEBUG flag.

It would be great if both implementations of the negative binomial distribution will improve.

Thank you, Max

Steven Watanabe — Tue, 10 Mar 2015 22:52:46 GMT

Replying to Maximilian Kleinert <kleinert.max@…>:

The standard only describes the negative binomial distribution for trials, e.g. integer parameters. So the behavior for a non integer value is unspecified by the standard.

Not correct. The standard specifies that it takes an integer parameter. It also states that a floating point number can be converted to an integer. Therefore the behavior when passing a floating point number is specified even if it isn't useful.

Steven Watanabe — Wed, 11 Mar 2015 02:28:21 GMT

k = 0 and p = 1 are degenerate cases which should be supported.

Wed, 11 Mar 2015 08:07:42 GMT

Replying to steven_watanabe:

Replying to Maximilian Kleinert <kleinert.max@…>:

The standard only describes the negative binomial distribution for trials, e.g. integer parameters. So the behavior for a non integer value is unspecified by the standard.

Not correct. The standard specifies that it takes an integer parameter. It also states that a floating point number can be converted to an integer. Therefore the behavior when passing a floating point number is specified even if it isn't useful.

Ok. So the only possibility would be to add an extra distribution, e.g. Polya distribution, which is essentially the same except that it allows a real positive parameter k?

Wed, 11 Mar 2015 08:28:41 GMT

Replying to steven_watanabe:

k = 0 and p = 1 are degenerate cases which should be supported.

The case p = 1 (regardless of k) is not supported by the boost version: The gamma distribution fails because the parameter beta = (1-p)/p is not greater than zero. The standard version fails if compiled with -D_GLIBCXX_DEBUG using g++.

The working draft standard (N4926) page 965 requires: 0 < p <= 1 and 0 < k.

Boost and standard version (except compiled with -D_GLIBCXX_DEBUG) return in the case p = 0 the sample -2147483648 which is not very useful.