[Bug Sprint][random] overload resolution for constructors/seed
AMDG I'm looking at the following tickets: https://svn.boost.org/trac/boost/ticket/351 https://svn.boost.org/trac/boost/ticket/2887 https://svn.boost.org/trac/boost/ticket/2424 #351 and #2887 are effectively the same boost::mt19937 prng1; boost::mt19937 prng2(prng1); // does not call copy constructor #2424 is int seed = 0; boost::mt19937 prng1(seed); // doesn't compile #2424 can be fixed easily without breaking backwards compatibility. Fixing #351 and #2887 will silently change the meaning of the constructor call above. It will also be difficult to emulate the previous (documented) behavior. However, looking at the current standard draft, the constructor that causes these problems is not present, so the standard mersenne twister will have a well behaved copy constructor. Also, only lagged_fibonacci_01, mersenne_twister, and subtract_with_carry provide the offending constructor, so Boost.Random is not consistent. I'd like to have the copy constructor called. What do others think? When testing fixes, I found that rand48 behaves differently with an int64_t seed than with an int32_t seed. According to the current draft standard, they should be equivalent. Should rand48 be changed or should I leave it alone? In Christ, Steven Watanabe
-----Original Message----- From: boost-bounces@lists.boost.org [mailto:boost-bounces@lists.boost.org] On Behalf Of Steven Watanabe Sent: Tuesday, June 02, 2009 4:30 PM
Fixing #351 and #2887 will silently change the meaning of the constructor call above. It will also be difficult to emulate the previous (documented) behavior. However, looking at the current standard draft, the constructor that causes these problems is not present, so the standard mersenne twister will have a well behaved copy constructor. Also, only lagged_fibonacci_01, mersenne_twister, and subtract_with_carry provide the offending constructor, so Boost.Random is not consistent. I'd like to have the copy constructor called. What do others think?
I think the principal of least surprise dictates that it should be the copy constructor. Can't the current behavior be achieved with the following? boost::mt19937 prng1; boost::mt19937 prng2; prng2.seed(prng1);
When testing fixes, I found that rand48 behaves differently with an int64_t seed than with an int32_t seed. According to the current draft standard, they should be equivalent. Should rand48 be changed or should I leave it alone?
For the same reason as above, I think the behavior should be the same.
In Christ, Steven Watanabe
Keith Jeffery
AMDG Keith Jeffery wrote:
Fixing #351 and #2887 will silently change the meaning of the constructor call above. It will also be difficult to emulate the previous (documented) behavior. However, looking at the current standard draft, the constructor that causes these problems is not present, so the standard mersenne twister will have a well behaved copy constructor. Also, only lagged_fibonacci_01, mersenne_twister, and subtract_with_carry provide the offending constructor, so Boost.Random is not consistent. I'd like to have the copy constructor called. What do others think?
I think the principal of least surprise dictates that it should be the copy constructor. Can't the current behavior be achieved with the following?
boost::mt19937 prng1; boost::mt19937 prng2; prng2.seed(prng1);
Yes, but it will be inefficient, because it has to seed twice. I suppose that we don't expect to create many PRNGs so this is acceptable. I'm certain that the "correct" behavior is to call the copy constructor. The main question is whether it is worth breaking backwards compatibility.
When testing fixes, I found that rand48 behaves differently with an int64_t seed than with an int32_t seed. According to the current draft standard, they should be equivalent. Should rand48 be changed or should I leave it alone?
For the same reason as above, I think the behavior should be the same.
rand48 has a 32 bit return type, so according to the draft standard the seed should be static_cast<uint32_t>(x). The problem is that the state is a 64 bit number, so we don't really like to throw away the extra bits when given a uint64_t. This is not so much an issue of least surprise as of ease of testing and matching <random> (which has diverged a fair amount from Boost.Random anyway) In Christ, Steven Watanabe
participants (2)
-
Keith Jeffery -
Steven Watanabe