libc++: Implementation of std::poisson_distribution::operator()

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

libc++: Implementation of std::poisson_distribution::operator()

Richard Smith via cfe-dev

Back in 2010, the algorithm for this function was updated, adding the following clause:

 

            if (__x < 10)

            {

                const result_type __fac[] = {1, 1, 2, 6, 24, 120, 720, 5040,

                                             40320, 362880};

                __px = -__pr.__mean_;

                __py = _VSTD::pow(__pr.__mean_, (double)__x) / __fac[__x];

            }

 

In this case, result_type is the single template parameter to the instantiation of std::poisson_distribution.

In the case that std::poisson_distribution is instantiated with a 16-bit signed integral type, __fac[8] and __fac[9] are out of range of result_type, and are truncated or result in a change of sign.

 

Since the only use of __fac is as an operand to a double-precision floating point division, would it not make sense to instead declare it ‘double’ instead of ‘result_type’, and then modify the values populating it to be doubles?

 

Regards,

J.B. Nagurne

Code Generation

Texas Instruments


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: libc++: Implementation of std::poisson_distribution::operator()

Richard Smith via cfe-dev
On Wed, Jan 10, 2018 at 11:25 AM, Nagurne, James via cfe-dev <[hidden email]> wrote:

Back in 2010, the algorithm for this function was updated, adding the following clause:

 

            if (__x < 10)

            {

                const result_type __fac[] = {1, 1, 2, 6, 24, 120, 720, 5040,

                                             40320, 362880};

                __px = -__pr.__mean_;

                __py = _VSTD::pow(__pr.__mean_, (double)__x) / __fac[__x];

            }

 

In this case, result_type is the single template parameter to the instantiation of std::poisson_distribution.

In the case that std::poisson_distribution is instantiated with a 16-bit signed integral type, __fac[8] and __fac[9] are out of range of result_type, and are truncated or result in a change of sign.

 

Since the only use of __fac is as an operand to a double-precision floating point division, would it not make sense to instead declare it ‘double’ instead of ‘result_type’, and then modify the values populating it to be doubles?



------------------------------------------------------------------------
r322556 | marshall | 2018-01-16 06:54:36 -0800 (Tue, 16 Jan 2018) | 7 lines
Changed paths:
   M /libcxx/trunk/include/random

Change an internal table of constants for the poisson distribution from
type 'result_type' to 'double'. The only thing that we ever do with
these numbers is to promote them to 'double' and use them in a division.
For small result_types, the values were getting truncated, skewing the
results. Thanks to James Nagurne for the suggestion.
 


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev