fix random item draw

related to

• !122 (merged)

description

!122 (merged) 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

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

measuring the performance

i did run the same timing experiment from !122 (merged) 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

-use rand::{seq::SliceRandom, Rng, RngCore};
+use rand::{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();
 
@@ -18,8 +17,9 @@ pub(super) fn draw_unique_elements<T: Clone>(
     n: usize,
     rng: &mut impl RngCore,
 ) -> Vec<T> {
-    let mut things = things.to_vec();
-    things.shuffle(rng);
-
-    things.iter().take(n).cloned().collect()
+    let mut res = vec![];
+    for i in draw_unique_indices(n, things.len(), rng) {
+        res.push(things[i].clone());
+    }
+    res
 }