From a7e19a809fb9d9624daeb6ab83dde1af8d54ec3f Mon Sep 17 00:00:00 2001
From: Nicolas DUBIEN <github@dubien.org>
Date: Mon, 9 Jan 2023 20:29:59 +0100
Subject: [PATCH] =?UTF-8?q?=F0=9F=94=A5=20Clean=20internals=20of=20uniform?=
 =?UTF-8?q?=20distribution=20(#515)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 .../UnsafeUniformIntDistributionInternal.ts   | 45 ++++---------------
 ...safeUniformIntDistributionInternal.spec.ts | 31 -------------
 2 files changed, 8 insertions(+), 68 deletions(-)

diff --git a/src/distribution/internals/UnsafeUniformIntDistributionInternal.ts b/src/distribution/internals/UnsafeUniformIntDistributionInternal.ts
index 5de8f18c..c4c3bd1a 100644
--- a/src/distribution/internals/UnsafeUniformIntDistributionInternal.ts
+++ b/src/distribution/internals/UnsafeUniformIntDistributionInternal.ts
@@ -2,48 +2,19 @@ import { RandomGenerator } from '../../generator/RandomGenerator';
 
 /**
  * Uniformly generate number in range [0 ; rangeSize[
+ * With rangeSize <= 0x100000000
  * @internal
  */
 export function unsafeUniformIntDistributionInternal(rangeSize: number, rng: RandomGenerator): number {
   const MinRng = -0x80000000;
   const NumValues = 0x100000000;
 
-  // Range provided by the RandomGenerator is large enough
-  if (rangeSize <= NumValues) {
-    const MaxAllowed = NumValues - (NumValues % rangeSize);
-    let deltaV = rng.unsafeNext() - MinRng;
-    while (deltaV >= MaxAllowed) {
-      deltaV = rng.unsafeNext() - MinRng;
-    }
-    return deltaV % rangeSize; // Warning: we expect NumValues <= 2**32, so diff too
+  // Range provided by the RandomGenerator is large enough,
+  // given rangeSize <= 0x100000000 and RandomGenerator is uniform on 0x100000000 values
+  const MaxAllowed = NumValues - (NumValues % rangeSize);
+  let deltaV = rng.unsafeNext() - MinRng;
+  while (deltaV >= MaxAllowed) {
+    deltaV = rng.unsafeNext() - MinRng;
   }
-
-  // Compute number of iterations required to have enough random
-  // to build uniform entries in the asked range
-  let FinalNumValues = NumValues * NumValues;
-  let NumIterations = 2; // At least 2 (at this point in the code)
-  while (FinalNumValues < rangeSize) {
-    FinalNumValues *= NumValues;
-    ++NumIterations;
-  }
-  const MaxAcceptedRandom = rangeSize * Math.floor((1 * FinalNumValues) / rangeSize);
-
-  let largeDeltaV = unsafeComputeValue(rng, NumIterations, NumValues, MinRng);
-  while (largeDeltaV >= MaxAcceptedRandom) {
-    largeDeltaV = unsafeComputeValue(rng, NumIterations, NumValues, MinRng);
-  }
-  const inDiff = largeDeltaV - rangeSize * Math.floor((1 * largeDeltaV) / rangeSize);
-  return inDiff;
-}
-
-/**
- * Aggregate multiple calls to next() into a single random value
- */
-function unsafeComputeValue(rng: RandomGenerator, NumIterations: number, NumValues: number, MinRng: number): number {
-  let value = 0;
-  for (let num = 0; num !== NumIterations; ++num) {
-    const out = rng.unsafeNext();
-    value = NumValues * value + (out - MinRng); // Warning: we overflow may when diff > max_safe (eg: =max_safe-min_safe+1)
-  }
-  return value;
+  return deltaV % rangeSize; // Warning: we expect NumValues <= 2**32, so diff too
 }
diff --git a/test/unit/distribution/internals/UnsafeUniformIntDistributionInternal.spec.ts b/test/unit/distribution/internals/UnsafeUniformIntDistributionInternal.spec.ts
index bbe7aa3a..80f5e72c 100644
--- a/test/unit/distribution/internals/UnsafeUniformIntDistributionInternal.spec.ts
+++ b/test/unit/distribution/internals/UnsafeUniformIntDistributionInternal.spec.ts
@@ -1,7 +1,6 @@
 import * as fc from 'fast-check';
 
 import { unsafeUniformIntDistributionInternal } from '../../../../src/distribution/internals/UnsafeUniformIntDistributionInternal';
-import mersenne from '../../../../src/generator/MersenneTwister';
 import { RandomGenerator } from '../../../../src/generator/RandomGenerator';
 
 class NatGenerator implements RandomGenerator {
@@ -22,22 +21,6 @@ class NatGenerator implements RandomGenerator {
   }
 }
 
-class ModNatGenerator implements RandomGenerator {
-  constructor(private current: RandomGenerator, private mod: number) {}
-  clone(): RandomGenerator {
-    return new ModNatGenerator(this.current, this.mod);
-  }
-  next(): [number, RandomGenerator] {
-    const nextRng = this.clone();
-    return [nextRng.unsafeNext(), nextRng];
-  }
-  unsafeNext(): number {
-    const [v, nrng] = this.current.next();
-    this.current = nrng;
-    return Math.abs(v % this.mod);
-  }
-}
-
 const MAX_RANGE: number = 1000;
 
 describe('unsafeUniformIntDistributionInternal', () => {
@@ -84,18 +67,4 @@ describe('unsafeUniformIntDistributionInternal', () => {
         }
       )
     ));
-  it('Should be able to generate values larger than the RandomGenerator and outside bitwise operations', () =>
-    fc.assert(
-      fc.property(fc.integer(), fc.integer({ min: 2, max: 0x7fffffff }), (seed, mod) => {
-        // kept it for legacy reasons, but could probably go without it
-        let rng: RandomGenerator = new ModNatGenerator(mersenne(seed), mod);
-        for (let numTries = 0; numTries < 100; ++numTries) {
-          const v = unsafeUniformIntDistributionInternal(Number.MAX_SAFE_INTEGER, rng);
-          if (v > 0xffffffff) {
-            return true;
-          }
-        }
-        return false;
-      })
-    ));
 });