Re: [gfortran] PATCH -- Fix for PR 15619

[Steve Kargl <sgk at troutmask dot apl dot washington dot edu>]

On Sat, May 29, 2004 at 01:15:50PM +0100, Paul Brook wrote:
> > Sigh.  I really didn't want to become an expert on PRNG.  I only
> > want to use them. :-)  So, here's my current solution.
> 
> +#ifdef __FreeBSD__
> +#  include <sys/uio.h>
> +#  include <unistd.h>
> +#endif
> 
> Definitely not. Why is this needed? It should be a proper configure check.

Sorry about that.  This is a leftover from hacking on the Mersenne
Twister.  Lars was initializing the MT by reading /dev/urandom.
read(), open() and close() need proper prototypes.
> 
> +const GFC_INTEGER_4
> +  kiss_default_seed[4] = {123456789, 362436069, 521288629, 916191069};
> +static GFC_INTEGER_4
> +  kiss_seed[4]         = {123456789, 362436069, 521288629, 916191069};
> 
> Shouldn't these seed values use unsigned types? You're doing right shifts on 
> the, so it will affect the value.

I changed everything to GFC_UINTEGER_4.

> 
> +/* kiss_random_kernel() returns an integer value in the range of
> +   [- GFC_INTEGER_4_HUGE, GFC_INTEGER_4_HUGE].  The distribution of 
> +   pseudorandom numbers should be uniform.  */
> 
> I suspect this is wrong. DOesn't it return a value in the range
> [- (GFC_INTEGER_4_HUGE + 1), GFC_INTEGER_4_HUGE] ?

Fixed.  The range is [0, GFC_UINTEGER_4_HUGE] where I'm not sure
if 0 is a possible value. 

> 
> +void prefix(random_r4) (GFC_REAL_4 *x)
> +{
> +
> +  *x = (GFC_REAL_4) kiss_random_kernel();
> +  *x = ((*x < 0) ? - *x : *x) / ((GFC_REAL_4) (GFC_INTEGER_4_HUGE) + 1.);
> +}
> 
> This is wrong.  (-x == x && x < 0) when x = 1u<<31.
> 
> Also ((GFC_REAL_4) (GFC_INTEGER_4_HUGE) + 1.) == GFC_INTEGER_4_HUGE
> because of limited precision, so it can return 1.0. I'd prefer it if you 
> used/copied the existing routines to do this.

Fixed.

> These also return in the range [0, 1], but that can be fixed by iterating 
> until we generate a value <1.0.

Fixed.

> Please add a comment saying why we #if 0'd the existing implementation. ie. 
> randomness of values depends on seed quality, get/put don't work properly, 
> mention PR15619.

Fixed.

See attached.
fortran.diff is applied to gcc/fortran.
random.c.diff is applied to random.c

-- 
Steve

Attachment: random.c.diff
Description: Text document

Attachment: fortran.diff
Description: Text document