Opened 11 years ago

Closed 10 years ago

Last modified 9 years ago

#6059 closed Bugs (fixed)

uniform_real_distribution fails when _min = _max

Reported by: dario.izzo@… Owned by: Steven Watanabe
Milestone: To Be Determined Component: random
Version: Boost 1.47.0 Severity: Problem
Keywords: Cc:

Description

Connected to BUG report #6053.

Even though all asserts require only that _min <= _max, the condition _min=_max seems to be not handled correctly. Consider the code

 #include<boost/random/uniform_real_distribution.hpp>
 #include<boost/random/lagged_fibonacci.hpp>
 #include<iostream>

 int main() {
 boost::lagged_fibonacci607 rng(32);
 double r = boost::random::uniform_real_distribution<double>(90,91)(rng);
 std::cout << r << std::endl;
 r = boost::random::uniform_real_distribution<double>(50,50)(rng);
std::cout << r << std::endl;
 return 0;
 }

the second std::cout will never execute as an infinite loop is encountered before. The problem is in boost/random/uniform_real_distribution.hpp and in particular at generate_uniform_real line:

if(result < max_value) return result;

If min_value == max_value the condition above is never met as result = min_value = max_value ALWAYS.

A solution (the one I am currently using) could be

if ( (result < max_value) || (max_value==min_value) ) return result;

Change History (9)

comment:1 by Steven Watanabe, 11 years ago

min == max is illegal, because the range [min, max) is empty. Thus there are no legal values for the distribution to produce.

comment:2 by dario.izzo@…, 11 years ago

OK, then if this is a design choice, it would be nice to have a warning or an exception be thrown rather then an infinite loop produced (also because the behavior has changed since boost 1.46 (and my code was counting on the previous behavior :)

NOTE: I can actually see many reasons to allow min==max and return min, but I guess this choice has already been made ....

Thanks for taking the time to answer so promptly

comment:3 by Edward Rudd <urkle@…>, 10 years ago

@steve_watanabe if min == max is illegal then shouldn't the ASSERT in the constructor be _min < _max instead of "<=" as it is now? This is rather diceiving as I expected it to throw some error in the condition of _min == _max (which in my case only occurred to due a bug in my code that calculated the range). Had the assert in uniform_real_distribution been correct I would have been able to find the error quicker.

comment:4 by Steven Watanabe, 10 years ago

Resolution: fixed
Status: newclosed

(In [82937]) Fix assertion for uniform_real. Fixes #6053. Fixes #6059.

comment:5 by harris.pc@…, 9 years ago

Why has this bug been fixed, but in boost 1.53.0, the assert remains the same as it was before (incorrect!)

See uniform_real_distribution.hpp

comment:6 by harris.pc@…, 9 years ago

Ah, the patch came after 1.53.0, I think.

comment:7 by pontus, 9 years ago

Still not fixed in 1.55.0

comment:8 by digigram@…, 9 years ago

this is a regression from 1.47 where min=max was allowed and returned the expected value. The regression breaks existing software. The logical return would simply be the max or min value when there is no range and so the proposed >= should be right.

More fundamental, if the numerator<=divisor and both are greater than 0 than result should always be between min and max (including max) and the for loop really does not make any sense unless max is not allowed to be part of the range, but I would not know why it shouldn't be. In matter of fact, I would say max should be part of the range simply because the max of a dice is 6 and the min is 1 and six can be thrown.

So I propose to remove the for loop altogether, and the if assertion as well.

comment:9 by dario.izzo@…, 9 years ago

I would also love to see this fixed allowing max==min (in which case min is returned). I had to patch all my code manually to allow this behaviour explicitly .... any hope to see this implemented?

Note: See TracTickets for help on using tickets.