From e5311d6c0251d764923d99daa73d8a7121099da6 Mon Sep 17 00:00:00 2001
From: STEVAN Antoine <antoine.stevan@isae-supaero.fr>
Date: Thu, 30 May 2024 09:20:55 +0000
Subject: [PATCH] fix random item draw (dragoon/komodo!127)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

related to
- !122

## description
!122 introduced a `draw_unique_indices` function which uses a `HashSet` to accumulate unique indices in the range `0..<len`.
however, a `HashSet` does not preserve the order of insertion when iterating over the elements of the set... which results in apparent randomness, even though the RNG seed is the same :open_mouth:

this MR switches back to using `shuffle` which used to work, even though a bit less performant :ok_hand:
it's basically a revert of !122, while keeping the refactoring into `random.rs`.

## measuring the performance
i did run the same timing experiment from !122 but with `main` on `bb55005` and the MR on `fix-shuffle`

| env     | main                   | mr                      | improvement         |
| ------- | ---------------------- | ----------------------- | ------------------- |
| fixed:0 | 6sec 244ms 238µs 45ns | 8sec 734ms 929µs 328ns | -39.88783363238997  |
| fixed:1 | 639ms 720µs 39ns      | 731ms 360µs 261ns      | -14.325051024390373 |

we loose a bit
---
 bins/inbreeding/src/random.rs | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/bins/inbreeding/src/random.rs b/bins/inbreeding/src/random.rs
index 8593049f..d5cb9276 100644
--- a/bins/inbreeding/src/random.rs
+++ b/bins/inbreeding/src/random.rs
@@ -1,6 +1,7 @@
-use rand::{Rng, RngCore};
+use rand::{seq::SliceRandom, Rng, RngCore};
 use std::collections::HashSet;
 
+#[allow(dead_code)]
 fn draw_unique_indices(n: usize, vec_len: usize, rng: &mut impl RngCore) -> HashSet<usize> {
     let mut indices = HashSet::new();
 
@@ -17,9 +18,8 @@ pub(super) fn draw_unique_elements<T: Clone>(
     n: usize,
     rng: &mut impl RngCore,
 ) -> Vec<T> {
-    let mut res = vec![];
-    for i in draw_unique_indices(n, things.len(), rng) {
-        res.push(things[i].clone());
-    }
-    res
+    let mut things = things.to_vec();
+    things.shuffle(rng);
+
+    things.iter().take(n).cloned().collect()
 }
-- 
GitLab