Skip to content

Commit cf4f73e

Browse files
authored
sample_efraimidis_spirakis: error on more than amount non-finite weights (#1814)
Also contains some doc changes.
1 parent 2c1aba5 commit cf4f73e

3 files changed

Lines changed: 41 additions & 3 deletions

File tree

src/distr/weighted/mod.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,9 @@ pub trait Weight: Clone {
2828

2929
/// Checked addition
3030
///
31+
/// Note that for floating-point formats with a representation of infinity,
32+
/// overflow-to-infinity is not considered an error.
33+
///
3134
/// - `Result::Ok`: On success, `v` is added to `self`
3235
/// - `Result::Err`: Returns an error when `Self` cannot represent the
3336
/// result of `self + v` (i.e. overflow). The value of `self` should be

src/seq/index.rs

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -295,7 +295,10 @@ where
295295
/// an alternative.
296296
///
297297
/// Error cases:
298-
/// - [`WeightError::InvalidWeight`] when a weight is not-a-number or negative.
298+
/// - [`WeightError::InvalidWeight`] when a weight is not-a-number or negative,
299+
/// or when infinite weights fill the reservoir before all elements have
300+
/// been processed (this always happens with more than `amount` infinite
301+
/// weights, and may also happen with exactly `amount`, depending on order).
299302
///
300303
/// This implementation uses `O(length + amount)` space and `O(length)` time.
301304
#[cfg(feature = "std")]
@@ -341,7 +344,10 @@ where
341344
/// It uses `O(length + amount)` space and `O(length)` time.
342345
///
343346
/// Error cases:
344-
/// - [`WeightError::InvalidWeight`] when a weight is not-a-number or negative.
347+
/// - [`WeightError::InvalidWeight`] when a weight is not-a-number or negative,
348+
/// or when infinite weights fill the reservoir before all elements have
349+
/// been processed (this always happens with more than `amount` infinite
350+
/// weights, and may also happen with exactly `amount`, depending on order).
345351
#[cfg(feature = "std")]
346352
fn sample_efraimidis_spirakis<R, F, X, N>(
347353
rng: &mut R,
@@ -408,6 +414,10 @@ where
408414
if index < length {
409415
let mut x = rng.random::<f64>().ln() / candidates.peek().unwrap().key;
410416
while index < length {
417+
if !x.is_finite() {
418+
return Err(WeightError::InvalidWeight);
419+
}
420+
411421
let weight = weight(index.as_usize()).into();
412422
if weight > 0.0 {
413423
x -= weight;
@@ -656,6 +666,30 @@ mod test {
656666
assert_eq!(r.unwrap().len(), 9);
657667
}
658668

669+
#[cfg(feature = "std")]
670+
#[test]
671+
fn test_sample_weighted_infinities() {
672+
let mut rng = crate::test::rng(1351);
673+
674+
for _ in 0..10 {
675+
let result = sample_weighted(&mut rng, 5, |i| 2.0 / (2.0 - (i as f64)).abs(), 2);
676+
assert!(result.is_ok());
677+
// Since one input has infinite weight, it must be selected:
678+
assert!(result.unwrap().iter().any(|i| i == 2));
679+
}
680+
681+
// Sampling from amount infinities should succeed
682+
let weights = [1.0, 0.0, f32::INFINITY, 2.5, f32::INFINITY];
683+
let result = sample_weighted(&mut rng, weights.len(), |i| weights[i], 2);
684+
assert!(result.is_ok());
685+
let mut results = result.unwrap().into_vec();
686+
results.sort();
687+
assert_eq!(results, [2, 4]);
688+
689+
// Sampling from too many infinities is an error
690+
assert!(sample_weighted(&mut rng, weights.len(), |i| weights[i], 1).is_err());
691+
}
692+
659693
#[test]
660694
fn value_stability_sample() {
661695
let do_test = |length, amount, values: &[u32]| {

src/seq/slice.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ pub trait IndexedRandom: Index<usize> {
8888
/// Chooses `amount` elements from the slice at random, without repetition,
8989
/// and in random order. The returned iterator is appropriate both for
9090
/// collection into a `Vec` and filling an existing buffer (see example).
91+
/// If `amount > self.len()`, all available elements are sampled.
9192
///
9293
/// In case this API is not sufficiently flexible, use [`index::sample`].
9394
///
@@ -126,7 +127,7 @@ pub trait IndexedRandom: Index<usize> {
126127
/// Uniformly sample a fixed-size array of distinct elements from self
127128
///
128129
/// Chooses `N` elements from the slice at random, without repetition,
129-
/// and in random order.
130+
/// and in random order. Returns `None` if (and only if) `N > self.len()`.
130131
///
131132
/// For slices, complexity is the same as [`index::sample_array`].
132133
///

0 commit comments

Comments
 (0)