[Random] uniform_int vs. uniform_real vs. uniform_01
I'm having a few problems dealing with uniform_real and uniform_01. The basic issue is that uniform_real does not always produce values between its min and its max. The problem consistently occurs when I'm using minstd_rand as my generator (one of the linear_congruential variants). For instance, if I write: template<typename RandomNumberGenerator> void foo(RandomNumberGenerator& gen) { uniform_int<int> rand_int(some_min_int, some_max_int); // Okay, some_min_int <= x <= some_max_int int x = rand_int(gen); uniform_real<double> rand_double(some_min_double, some_max_double); // Uh-oh, some_min_double <= y <= some_max_double doesn't always hold double y = rand_double(gen); } Looking at the source to uniform_real, the problem is obvious: result_type operator()(Engine& eng) { return eng() * (_max - _min) + _min; } uniform_real assumes that the engine returns a value in [0, 1], but doesn't check that. If this is correct (I hope it isn't!), the "obvious" answer is to use uniform_01<RandomNumberGenerator...> as input to uniform_real. However, uniform_01 is also broken in several ways: 1) It isn't a random distribution because its operator() does not accept an engine. Instead, the engine is embedded in the uniform_01 object. 2) uniform_01 stores the incoming random number generator by _value_, so the original generator "gen" does not advance. You can't even fake it with uniform_01<RandomNumberGenerator&>, because that creates reference-to-reference problems. Below is a small patch to make uniform_real<...> behave nicely. As for uniform_01: we should deprecate it in 1.33.0 and remove it afterwords, because it is hopelessly broken and not even included in the TR. Doug Index: uniform_real.hpp =================================================================== RCS file: /cvsroot/boost/boost/boost/random/uniform_real.hpp,v retrieving revision 1.14 diff -u -r1.14 uniform_real.hpp --- uniform_real.hpp 27 Jul 2004 03:43:32 -0000 1.14 +++ uniform_real.hpp 30 Dec 2004 22:11:33 -0000 @@ -50,7 +50,11 @@ void reset() { } template<class Engine> - result_type operator()(Engine& eng) { return eng() * (_max - _min) + _min; } + result_type operator()(Engine& eng) { + return static_cast<result_type>(eng() - eng.min()) + / (eng.max() - eng.min()) + * (_max - _min) + _min; + } #if !defined(BOOST_NO_OPERATORS_IN_NAMESPACE) && !defined(BOOST_NO_MEMBER_TEMPLATE_FRIENDS) template<class CharT, class Traits>
Doug Gregor wrote:
I'm having a few problems dealing with uniform_real and uniform_01.
The basic issue is that uniform_real does not always produce values between its min and its max.
uniform_real was meant to be used with variate_generate<> only. Reviewing the language in the ISO C++ TR draft seems to miss one requirement: That the Engine used as input for uniform_real() always returns numbers in the range [0..1[, if floating-point numbers are requested. This is one of the features of variate_generate<>: it wraps the original Engine so that this is always the case. Note that the current ISO C++ TR draft already requires that the input Engine produce numbers of the "correct" type (floating-point or integer).
Looking at the source to uniform_real, the problem is obvious:
result_type operator()(Engine& eng) { return eng() * (_max - _min) + _min; }
Yes. Fixed.
However, uniform_01 is also broken in several ways:
uniform_01, at this time, is just an implementation helper for variate_generator<> and thus should move to the details namespace ASAP. Since it's hideously broken for general use right now, I'm inclined to perform the move without any deprecation period. Jens Maurer
On Dec 31, 2004, at 10:51 AM, Jens Maurer wrote:
Doug Gregor wrote:
I'm having a few problems dealing with uniform_real and uniform_01. The basic issue is that uniform_real does not always produce values between its min and its max.
uniform_real was meant to be used with variate_generate<> only. Reviewing the language in the ISO C++ TR draft seems to miss one requirement: That the Engine used as input for uniform_real() always returns numbers in the range [0..1[, if floating-point numbers are requested. This is one of the features of variate_generate<>: it wraps the original Engine so that this is always the case.
Interesting. Looks like I should be using variate_generator more often.
Note that the current ISO C++ TR draft already requires that the input Engine produce numbers of the "correct" type (floating-point or integer).
Looking at the source to uniform_real, the problem is obvious: result_type operator()(Engine& eng) { return eng() * (_max - _min) + _min; }
Yes. Fixed.
Thanks!
However, uniform_01 is also broken in several ways:
uniform_01, at this time, is just an implementation helper for variate_generator<> and thus should move to the details namespace ASAP. Since it's hideously broken for general use right now, I'm inclined to perform the move without any deprecation period.
Sounds good to me. Doug
On Dec 31, 2004, at 10:51 AM, Jens Maurer wrote:
Looking at the source to uniform_real, the problem is obvious: result_type operator()(Engine& eng) { return eng() * (_max - _min) + _min; }
Yes. Fixed.
It doesn't seem to be completely fixed, although it might be because I am wrong. In the fix I'd sent, eng() - eng.min() would first be casted to the result_type (a real type, of course), the we divide by eng.max() - eng.min() and adjust to uniform_real's range. In the code in CVS, the cast isn't performed until after the division. So when the engine returns integer values (as linear_congruential does), we don't get proper results. Is it wrong to use engines returning integer values with uniform_real? If so, we should have a static assert in there. Doug
participants (3)
-
Doug Gregor -
Douglas Gregor
-
Jens Maurer