Re: Issues with rand()

[Ethan M. <merritt@u.washington.edu>]

On Tuesday, December 28, 2010, Daniel J Sebald wrote:
> Ethan Merritt wrote:

> > It's fair to consider acceptance of a seed value (0 < seed < 1) as a
> > bug, since it will immediately be converted to an integer value 0
> > which is not a valid seed.  This might be an adequate fix:
> > %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
> > --- gnuplot/src/specfun.c       2010-10-21 22:28:24.000000000 -0700
> > +++ gnuplot-cvs/src/specfun.c   2010-12-28 13:23:20.000000000 -0800
> > @@ -1098,7 +1098,7 @@ ranf(struct value *init)
> >  
> >      /* Construct new seed values from input parameter */
> >      /* FIXME: Ideally we should allow all 64 bits of seed to be set */
> > -    if (real(init) > 0.0) {
> > +    if (real(init) > 1.0) {
> >         if (real(init) >= (double)(017777777777UL))
> >             int_error(NO_CARET,"Illegal seed value");
> >         if (imag(init) >= (double)(017777777777UL))
> > %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
> > Note that the FIXME comment is out of date.  You can indeed set 64 bits
> > of seed by using rand(i + j*{0,1}) as noted above.
> 
> The "value" type is double floats for real and imaginary, then?  Well, that should hold 32 bits within a 52 bit fractional.  (seed1 and seed2 are longs.)

Yes.  The number of mantissa bits kept by a (double) is greater than 32,
so it's OK to hold a 32-bit integer value in a double.

> A couple odd things however.  Why is the limiting value 017777777777UL?  
> Is that some bad way of obtaining the maximum 32 value of 2^32-1, i.e., 4294967295?  
I think the intent is to make sure that the integer
value didn't overflow before being converted to (double).
Off the top of my head, I can't explain why the test isn't
against 037777777777 instead.


> If not, then it should be 
> 
> 	if (real(init) >= (double)(04294967295UL))
> 	    int_error(NO_CARET,"Illegal seed value");
> 	if (imag(init) >= (double)(04294967295UL))
> 	    int_error(NO_CARET,"Illegal seed value");

Why?  All bits set is easy to remember; the numerical value 4294967295 - not so easy.

 > And what is with this?
> 
> 	seed1 = (int)real(init);
> 	seed2 = (int)imag(init);
> 
> That's a potential bug for compilers in which the default integer is 16 bits.  First casting to int and then casting to long could lose something.

We no longer support 16-bit.
I don't know if that particular bit of code was ever a problem
back in the days when we did.  But you are correct that the explicit cast is not needed.

> Regarding the out of date comment, did it mean that seed1 and seed2 should be 64-bit, 
> i.e., long long?

Upon reflection, I think the comment is pointing out that since we just
disallowed any values with the high bit set you can only set the low 31
bits of each half.  I.e. only 62 of the 64 bits can be set.