Re: Issues with rand()

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

On Wednesday, December 29, 2010, Juhász Péter wrote:
> > it would be easy enough to do the same
> > thing in specfun.c that is already done in internal.c and standard.c
> >   internal.c:#define pop(x) pop_or_convert_from_string(x)
> >   standard.c:#define pop(x) pop_or_convert_from_string(x)
> 
> I tried this and it works nicely. If we were to put this definition at
> the beginning of the file, then it may affect other functions, a fact
> that requires further testing. 

I think it's safe enough.  All of the calls to pop() in that file are
expecting a numerical value, so they should all benefit equally from
better handling of a string value passed by mistake or in the expectation
that it will be converted.

> > %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
> > --- 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.
> > 
> 
> Seems OK. 

I've made this change, since it fixes an error (rand(0) stalling out
on a single value).  But the points below still need some thought.
It may well be OK the way it is, but perhaps there's a subtle error.
2^31 possible seeds is probably enough :-)

> However, a sentence or two should go into the documentation
> mentioning that only the integer part(s) of the seed are meaningful.
> 
> >> 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.
> 
> Simply because the highest bit stores the sign in a signed integer, and
> the integer part of struct value is a signed int? 
>