#6059 closed Bugs (fixed)
uniform_real_distribution fails when _min = _max
Reported by: | 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 , 11 years ago
comment:2 by , 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 , 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 , 10 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:5 by , 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:8 by , 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 , 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?
min == max is illegal, because the range [min, max) is empty. Thus there are no legal values for the distribution to produce.