Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:119214
Message-ID: <91e74741-3841-bdda-5331-e5175e825d44@bastelstu.be>
Date: Thu, 29 Dec 2022 14:28:54 +0100
MIME-Version: 1.0
Content-Language: en-US
To: Hans Henrik Bergan <divinity76@gmail.com>, Go Kudo <g-kudo@colopl.co.jp>
Cc: internals@lists.php.net
References: <CAMs36HdC6k0pjvjiXJDJhnUjSPeOnGSUFMn0FmyE2e0fZ1cCdQ@mail.gmail.com>
 <CAJmy8YF-63tQAujWYrjQCaH6V=ZcrdUzvdQ==bNB8MjARFuKjA@mail.gmail.com>
 <CAJmy8YHp6anOqwDOZ6x2AHEBeLdJNAG2gbtkv5Kkmiz125-P9A@mail.gmail.com>
In-Reply-To: <CAJmy8YHp6anOqwDOZ6x2AHEBeLdJNAG2gbtkv5Kkmiz125-P9A@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Subject: Re: [PHP-DEV] ext-random: add random_float() ?
From: tim@bastelstu.be (=?UTF-8?Q?Tim_D=c3=bcsterhus?=)

Hi

On 12/20/22 09:15, Hans Henrik Bergan wrote:
> btw while we're on the topic, does anyone know if this function gives
> biased results/is-safe or not? i honestly don't know:
> function random_float(float $min, float $max): float
> {
>      if ($min > $max) throw new \InvalidArgumentException("min must be
> less than max");
>      if ($min === $max) return $min;
>      return $min + random_int(0, PHP_INT_MAX) / PHP_INT_MAX * ($max - $min);
> }
> 
> 

To give a clear answer on this one:

For a full evaluation the reader lacks information: What is this 
function supposed to do? For floats, the definition of the boundaries 
matter (that's why it's a parameter for Randomizer::getFloat()). Your 
function is implemented on the closed interval [0, 1] (i.e. both 
including 0 and 1). As mentioned in my email a few minutes ago, I 
consider the ClosedClosed behavior to not be the most useful in the 
general case, but in your case it might be the correct behavior.

Nonetheless, the function is definitely broken:

1. A IEEE 754 double precision float as used by PHP only has 53 Bits of 
precision, but you generate a random 64 Bit integer. Thus rounding will 
occur and might cause some values to be returned more often than other.

To improve the function you should only generate 53 Bits of randomness 
(i.e. use 2**53 as the divisor and either 2**53-1 or 2**53 as the dividend).

2. Using the (min + rand * (max - min)) construction will generate 
biased results (see also my other email).

PHP 8.3's Randomizer::getFloat() method uses the γ-section algorithm 
proposed by Prof. Goualard to generate unbiased results for arbitrary 
ranges. This algorithm cannot reasonably be implemented in userland, 
because the necessary "building blocks" (obtaining the next/previous 
float for a given float) are not available for userland code. The best 
you can do in PHP 8.2 userland is what Randomizer::nextFloat() does.

TLDR: Yes, the function is broken.

Best regards
Tim Düsterhus