From 523e4ace968d67a971fe59aa410acb26f19fcdd7 Mon Sep 17 00:00:00 2001 From: Hydrogs Date: Thu, 26 Sep 2024 12:35:24 +0200 Subject: [PATCH 1/7] Add ParallelIterator::combinations --- src/iter/combinations.rs | 742 +++++++++++++++++++++++++++++++++++++++ src/iter/mod.rs | 46 +++ 2 files changed, 788 insertions(+) create mode 100644 src/iter/combinations.rs diff --git a/src/iter/combinations.rs b/src/iter/combinations.rs new file mode 100644 index 000000000..ed1dd9468 --- /dev/null +++ b/src/iter/combinations.rs @@ -0,0 +1,742 @@ +#![warn(clippy::pedantic)] // TODO: remove + +use std::ops::Div; +use std::sync::Arc; + +use super::plumbing::*; +use super::*; + +const OVERFLOW_MSG: &str = "Total number of combinations is too big."; + +/// `Combinations` is an iterator that iterates over the k-length combinations +/// of the elements from the original iterator. +/// This struct is created by the [`combinations()`] method on [`ParallelIterator`] +/// +/// [`combinations()`]: trait.ParallelIterator.html#method.combinations() +/// [`ParallelIterator`]: trait.ParallelIterator.html +#[must_use = "iterator adaptors are lazy and do nothing unless consumed"] +#[derive(Debug, Clone)] +pub struct Combinations { + base: I, + k: usize, +} + +impl Combinations { + /// Creates a new `Combinations` iterator. + pub(super) fn new(base: I, k: usize) -> Self { + Self { k, base } + } +} + +impl Combinations +where + I: ParallelIterator, +{ + fn into_producer(self) -> CombinationsProducer { + let items: Arc<[I::Item]> = self.base.collect(); + let n = items.len(); + let k = self.k; + let until = checked_binomial(n, k).expect(OVERFLOW_MSG); + + CombinationsProducer { + items, + offset: 0, + until, + k: self.k, + } + } +} + +impl ParallelIterator for Combinations +where + I: ParallelIterator, + I::Item: Clone + Send + Sync, +{ + type Item = Vec; + + fn drive_unindexed(self, consumer: C) -> C::Result + where + C: UnindexedConsumer, + { + bridge_unindexed(self.into_producer(), consumer) + } + + fn opt_len(&self) -> Option { + // HACK: even though the length can be computed with the code below, + // our implementation of drive_unindexed does not support indexed Consumer + + // self.base + // .opt_len() + // .and_then(|l| checked_binomial(l, self.k)) + None + } +} + +impl IndexedParallelIterator for Combinations +where + I: IndexedParallelIterator, + I::Item: Clone + Send + Sync, +{ + fn len(&self) -> usize { + checked_binomial(self.base.len(), self.k).expect(OVERFLOW_MSG) + } + + fn drive>(self, consumer: C) -> C::Result { + bridge(self, consumer) + } + + fn with_producer>(self, callback: CB) -> CB::Output { + callback.callback(self.into_producer()) + } +} + +struct CombinationsProducer { + items: Arc<[T]>, + offset: usize, + until: usize, + k: usize, +} + +impl CombinationsProducer { + fn new(items: Arc<[T]>, offset: usize, until: usize, k: usize) -> Self { + Self { + items, + offset, + until, + k, + } + } + + fn n(&self) -> usize { + self.items.len() + } + + fn len(&self) -> usize { + self.until - self.offset + } +} + +impl Producer for CombinationsProducer +where + T: Clone + Send + Sync, +{ + type Item = Vec; + type IntoIter = CombinationsSeq; + + fn into_iter(self) -> Self::IntoIter { + let n = self.n(); + CombinationsSeq::from_offsets(self.items, self.offset, self.until, n, self.k) + } + + fn split_at(self, index: usize) -> (Self, Self) { + let index = index + self.offset; + + ( + Self::new(Arc::clone(&self.items), self.offset, index, self.k), + Self::new(self.items, index, self.until, self.k), + ) + } +} + +impl UnindexedProducer for CombinationsProducer +where + T: Clone + Send + Sync, +{ + type Item = Vec; + + fn split(self) -> (Self, Option) { + if self.len() == 1 { + return (self, None); + } + let mid_point = self.len() / 2; + let (a, b) = self.split_at(mid_point); + (a, Some(b)) + } + + fn fold_with(self, folder: F) -> F + where + F: Folder, + { + folder.consume_iter(self.into_iter()) + } +} + +struct CombinationsSeq { + items: Arc<[T]>, + indices: Indices, + until: Indices, + len: usize, +} + +impl CombinationsSeq { + fn from_offsets(items: Arc<[T]>, from: usize, until: usize, n: usize, k: usize) -> Self { + Self { + indices: Indices::new(from, n, k), + until: Indices::new(until, n, k), + items, + len: until - from, + } + } + + fn may_next(&self) -> Option<()> { + if self.indices.is_empty() || self.until.is_empty() { + // there are no indices to compute + return None; + } + if self.indices == self.until { + // reached the end + return None; + } + + Some(()) + } +} + +impl Iterator for CombinationsSeq +where + T: Clone, +{ + type Item = Vec; + + fn next(&mut self) -> Option { + self.may_next()?; + let indices = self.indices.clone(); + self.indices.increment(); + Some(indices.deindex(&self.items)) + } +} + +impl ExactSizeIterator for CombinationsSeq +where + T: Clone, +{ + fn len(&self) -> usize { + self.len + } +} + +impl DoubleEndedIterator for CombinationsSeq +where + T: Clone, +{ + fn next_back(&mut self) -> Option { + self.may_next()?; + self.until.decrement(); + Some(self.until.deindex(&self.items)) + } +} + +#[derive(PartialEq, Clone)] +struct Indices { + indices: Vec, + n: usize, + position: IndicesPosition, +} + +#[derive(PartialEq, Clone, Copy)] +enum IndicesPosition { + End, + Middle, +} + +impl Indices { + fn new(offset: usize, n: usize, k: usize) -> Self { + let total = checked_binomial(n, k).expect(OVERFLOW_MSG); + if offset == total { + Self { + indices: unrank(offset - 1, n, k), + n, + position: IndicesPosition::End, + } + } else if offset < total { + Self { + indices: unrank(offset, n, k), + n, + position: IndicesPosition::Middle, + } + } else { + panic!("Offset should be inside the bounds of the possible combinations.") + } + } + + fn increment(&mut self) { + match self.position { + IndicesPosition::Middle => { + if is_last(&self.indices, self.n) { + self.position = IndicesPosition::End + } else { + increment_indices(&mut self.indices, self.n); + } + } + IndicesPosition::End => panic!("No more indices"), + } + } + + fn decrement(&mut self) { + match self.position { + IndicesPosition::End => self.position = IndicesPosition::Middle, + IndicesPosition::Middle => decrement_indices(&mut self.indices, self.n), + } + } + + fn deindex(&self, items: &[T]) -> Vec { + match self.position { + IndicesPosition::Middle => self.indices.iter().map(|i| &items[*i]).cloned().collect(), + IndicesPosition::End => panic!("End position indices cannot be deindexed."), + } + } + + fn is_empty(&self) -> bool { + self.indices.is_empty() + } +} + +/// Calculates the binomial coefficient for given `n` and `k`. +/// +/// The binomial coefficient, often denoted as C(n, k) or "n choose k", represents +/// the number of ways to choose `k` elements from a set of `n` elements without regard +/// to the order of selection. +/// +/// # Overflow +/// Note that this function will overflow even if the result is smaller than [`isize::MAX`]. +/// The guarantee is that it does not overflow if `nCk * k <= isize::MAX`. +fn checked_binomial(n: usize, k: usize) -> Option { + if k > n { + return Some(0); + } + + // nCk is symmetric around (n-k), so choose the smallest to iterate faster + let k = std::cmp::min(k, n - k); + + // Fast paths x2 ~ x6 speedup + match k { + 1 => return Some(n), + 2 => return n.checked_mul(n - 1).map(|r| r / 2), + 3 => { + return n + .checked_mul(n - 1)? + .div(2) + .checked_mul(n - 2) + .map(|r| r / 3) + } + 4 if n <= 77_937 /* prevent unnecessary overflow */ => { + return n + .checked_mul(n - 1)? + .div(2) + .checked_mul(n - 2)? + .checked_mul(n - 3) + .map(|r| r / 12) + } + 5 if n <= 10_811 /* prevent unnecessary overflow */ => { + return n + .checked_mul(n - 1)? + .div(2) + .checked_mul(n - 2)? + .checked_mul(n - 3)? + .div(4) + .checked_mul(n - 4) + .map(|r| r / 15) + } + _ => {} + } + + // `factorial(n) / factorial(n - k) / factorial(k)` but trying to avoid overflows + let mut result: usize = 1; + for i in 0..k { + result = result.checked_mul(n - i)?; + result /= i + 1; + } + Some(result) +} + +/// The indices corresponding to a given `offset` +/// in the corresponding ordered list of possible combinations of `n` choose `k`. +/// +/// # Panics +/// - If n < k: not enough elements to form a list of indices +/// - If the total number of combinations of `n` choose `k` is too big, roughly if it does not fit in usize. +fn unrank(offset: usize, n: usize, k: usize) -> Vec { + // Source: + // Antoine Genitrini, Martin Pépin. Lexicographic unranking of combinations revisited. + // Algorithms, 2021, 14 (3), pp.97. 10.3390/a14030097. hal-03040740v2 + + assert!( + n >= k, + "Not enough elements to choose k from. Note that n must be at least k." + ); + + debug_assert!(offset < checked_binomial(n, k).unwrap()); + + if k == 1 { + return vec![offset]; + } + + let mut res = vec![0; k]; + let mut b = checked_binomial(n - 1, k - 1).expect(OVERFLOW_MSG); + let mut m = 0; + let mut i = 0; + let mut u = offset; + + let mut first = true; + + while i < k - 1 { + if first { + first = false; + } else { + b /= n - m - i; + } + + if b > u { + res[i] = m + i; + b *= k - i - 1; + i += 1; + } else { + u -= b; + b *= n - m - k; + m += 1; + } + } + + if k > 0 { + res[k - 1] = n + u - b; + } + + res +} + +/// Increments indices representing the combination to advance to the next +/// (in lexicographic order by increasing sequence) combination. For example +/// if we have n=4 & k=2 then `[0, 1] -> [0, 2] -> [0, 3] -> [1, 2] -> ...` +fn increment_indices(indices: &mut [usize], n: usize) { + if indices.is_empty() { + return; + } + + let k = indices.len(); + + // Scan from the end, looking for an index to increment + let mut i: usize = indices.len() - 1; + + while indices[i] == i + n - k { + debug_assert!(i > 0); + i -= 1; + } + + // Increment index, and reset the ones to its right + indices[i] += 1; + for j in i + 1..indices.len() { + indices[j] = indices[j - 1] + 1; + } +} + +fn decrement_indices(indices: &mut [usize], n: usize) { + if indices.is_empty() { + return; + } + + let k = indices.len(); + + if k == 1 { + indices[0] -= 1; + return; + } + + let mut i = k - 1; + + while (i > 0) && indices[i] == indices[i - 1] + 1 { + debug_assert!(i > 0); + i -= 1; + } + + // Decrement index, and reset the ones to its right + indices[i] -= 1; + for j in i + 1..indices.len() { + indices[j] = n - k + j; + } +} + +fn is_last(indices: &[usize], n: usize) -> bool { + let k = indices.len(); + + // BENCH: vs == ((n-k)..n).collect() + indices + .iter() + .enumerate() + .all(|(i, &index)| index == n - k + i) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn checked_binomial_works() { + assert_eq!(checked_binomial(7, 2), Some(21)); + assert_eq!(checked_binomial(8, 2), Some(28)); + assert_eq!(checked_binomial(11, 3), Some(165)); + assert_eq!(checked_binomial(11, 4), Some(330)); + assert_eq!(checked_binomial(64, 2), Some(2016)); + assert_eq!(checked_binomial(64, 7), Some(621_216_192)); + } + + #[test] + fn checked_binomial_k_greater_than_n() { + assert_eq!(checked_binomial(5, 6), Some(0)); + assert_eq!(checked_binomial(3, 4), Some(0)); + } + + #[test] + fn test_binom_symmetry() { + assert_eq!(checked_binomial(10, 2), checked_binomial(10, 8)); + assert_eq!(checked_binomial(103, 2), checked_binomial(103, 101)); + } + + #[test] + fn checked_binomial_repeated_values() { + assert_eq!(checked_binomial(1000, 1000), Some(1)); + assert_eq!(checked_binomial(2048, 2048), Some(1)); + assert_eq!(checked_binomial(1000, 0), Some(1)); + assert_eq!(checked_binomial(2048, 0), Some(1)); + } + + #[test] + #[cfg(target_pointer_width = "64")] + fn checked_binomial_not_overflows() { + assert_eq!( + checked_binomial(4_294_967_296, 2), + Some(9_223_372_034_707_292_160) + ); + assert_eq!( + checked_binomial(3_329_022, 3), + Some(6_148_913_079_097_324_540) + ); + assert_eq!( + checked_binomial(102_570, 4), + Some(4_611_527_207_790_103_770) + ); + assert_eq!(checked_binomial(13_467, 5), Some(3_688_506_309_678_005_178)); + assert_eq!(checked_binomial(109, 15), Some(1_015_428_940_004_440_080)); + } + + #[test] + #[cfg(target_pointer_width = "64")] + fn checked_overflows() { + assert_eq!(checked_binomial(109, 16), None); + } + + #[test] + fn checked_binomial_identity() { + assert_eq!(checked_binomial(1, 1), Some(1)); + assert_eq!(checked_binomial(100, 1), Some(100)); + assert_eq!(checked_binomial(25_000, 1), Some(25_000)); + assert_eq!(checked_binomial(25_000, 0), Some(1)); + } + + #[test] + fn unrank_5_2() { + assert_eq!(unrank(0, 5, 2), vec![0, 1]); + assert_eq!(unrank(1, 5, 2), vec![0, 2]); + assert_eq!(unrank(2, 5, 2), vec![0, 3]); + assert_eq!(unrank(3, 5, 2), vec![0, 4]); + assert_eq!(unrank(4, 5, 2), vec![1, 2]); + assert_eq!(unrank(5, 5, 2), vec![1, 3]); + assert_eq!(unrank(6, 5, 2), vec![1, 4]); + assert_eq!(unrank(7, 5, 2), vec![2, 3]); + assert_eq!(unrank(8, 5, 2), vec![2, 4]); + assert_eq!(unrank(9, 5, 2), vec![3, 4]); + } + + #[test] + fn unrank_zero_index() { + for k in 1..7 { + for n in k..11 { + assert_eq!(unrank(0, n, k), (0..k).collect::>()); + } + } + } + + #[test] + fn unrank_one_index() { + for n in 4..50 { + for k in 1..n { + let mut expected: Vec<_> = (0..k).collect(); + *expected.last_mut().unwrap() += 1; + assert_eq!(unrank(1, n, k), expected); + } + } + } + + #[test] + fn unrank_last() { + for n in 4..50 { + for k in 1..n { + let expected: Vec<_> = ((n - k)..n).collect(); + let offset = checked_binomial(n, k).unwrap() - 1; + assert_eq!(unrank(offset, n, k), expected); + } + } + } + + #[test] + fn unrank_k1() { + for n in 1..50 { + for u in 0..n { + assert_eq!(unrank(u, n, 1), vec![u]); + } + } + } + + #[test] + fn increment_indices_rank_consistent() { + for i in 0..16 { + for n in (i + 2)..(2 * (i + 1)) { + for k in 1..n { + let mut indices = unrank(i, n, k); + increment_indices(&mut indices, n); + assert_eq!(unrank(i + 1, n, k), indices); + } + } + } + } + + #[test] + fn increment_indices_rank_consistent_last_indices() { + for n in 2..15 { + for k in 1..n { + let max_iter = checked_binomial(n, k).unwrap(); + for i in (max_iter - n / 2)..(max_iter - 1) { + let mut indices = unrank(i, n, k); + increment_indices(&mut indices, n); + assert_eq!(unrank(i + 1, n, k), indices); + } + } + } + } + + #[test] + fn decrement_indices_rank_consistent() { + for i in 1..16 { + for n in (i + 2)..(2 * (i + 1)) { + for k in 1..n { + let mut indices = unrank(i, n, k); + decrement_indices(&mut indices, n); + assert_eq!(unrank(i - 1, n, k), indices); + } + } + } + } + + #[test] + fn decrement_indices_rank_consistent_last_indices() { + for n in 2..15 { + for k in 1..n { + let max_iter = checked_binomial(n, k).unwrap(); + for i in (max_iter - n / 2)..(max_iter) { + let mut indices = unrank(i, n, k); + decrement_indices(&mut indices, n); + assert_eq!(unrank(i - 1, n, k), indices); + } + } + } + } + + #[test] + #[should_panic] + fn unrank_offset_outofbounds() { + let n = 10; + let k = 2; + let u = checked_binomial(n, k).unwrap() + 1; + unrank(u, n, k); + } + + #[test] + fn par_iter_works() { + let a: Vec<_> = (0..5).into_par_iter().combinations(2).collect(); + + // FIXME: somewhere `until` is not correctly set + assert_eq!( + a, + vec![ + vec![0, 1], + vec![0, 2], + vec![0, 3], + vec![0, 4], + vec![1, 2], + vec![1, 3], + vec![1, 4], + vec![2, 3], + vec![2, 4], + vec![3, 4], + ] + ); + + let b: Vec<_> = (0..5).into_par_iter().combinations(3).collect(); + + assert_eq!( + b, + vec![ + vec![0, 1, 2], + vec![0, 1, 3], + vec![0, 1, 4], + vec![0, 2, 3], + vec![0, 2, 4], + vec![0, 3, 4], + vec![1, 2, 3], + vec![1, 2, 4], + vec![1, 3, 4], + vec![2, 3, 4], + ] + ); + } + + #[test] + fn par_iter_k1() { + assert_eq!( + (0..200).into_par_iter().combinations(1).collect::>(), + (0..200).map(|i| vec![i]).collect::>() + ) + } + + #[test] + fn par_iter_rev_works() { + let a: Vec<_> = (0..5).into_par_iter().combinations(2).rev().collect(); + assert_eq!( + a, + vec![ + vec![0, 1], + vec![0, 2], + vec![0, 3], + vec![0, 4], + vec![1, 2], + vec![1, 3], + vec![1, 4], + vec![2, 3], + vec![2, 4], + vec![3, 4], + ] + .into_iter() + .rev() + .collect::>() + ); + + let b: Vec<_> = (0..5).into_par_iter().combinations(3).rev().collect(); + assert_eq!( + b, + vec![ + vec![0, 1, 2], + vec![0, 1, 3], + vec![0, 1, 4], + vec![0, 2, 3], + vec![0, 2, 4], + vec![0, 3, 4], + vec![1, 2, 3], + vec![1, 2, 4], + vec![1, 3, 4], + vec![2, 3, 4], + ] + .into_iter() + .rev() + .collect::>() + ); + } +} diff --git a/src/iter/mod.rs b/src/iter/mod.rs index c7e3a0d99..5dd8918ac 100644 --- a/src/iter/mod.rs +++ b/src/iter/mod.rs @@ -108,6 +108,7 @@ mod chain; mod chunks; mod cloned; mod collect; +mod combinations; mod copied; mod empty; mod enumerate; @@ -166,6 +167,7 @@ pub use self::{ chain::Chain, chunks::Chunks, cloned::Cloned, + combinations::Combinations, copied::Copied, empty::{empty, Empty}, enumerate::Enumerate, @@ -1962,6 +1964,50 @@ pub trait ParallelIterator: Sized + Send { PanicFuse::new(self) } + /// Creates an iterator that iterates over the `k`-length combinations + /// of the elements from the original iterator. + /// + /// The iterator element type is `Vec`. + /// The iterator produces a new `Vec` per iteration + /// and clones the iterator elements. + /// + /// # Guarantees + /// If the original iterator is deterministic, + /// this iterator yields items in a reliable order. + /// + /// ``` + /// use rayon::prelude::*; + /// + /// let it: Vec<_> = (1..5).into_par_iter().combinations(3).collect(); + /// + /// assert_eq!(it[0], vec![1, 2, 3]); + /// assert_eq!(it[1], vec![1, 2, 4]); + /// assert_eq!(it[2], vec![1, 3, 4]); + /// assert_eq!(it[3], vec![2, 3, 4]); + /// ``` + /// + /// Note: Combinations does not take into account the equality of the iterated values. + /// ``` + /// use rayon::prelude::*; + /// + /// let it: Vec<_> = [1, 2, 2].into_par_iter().combinations(2).collect(); + /// + /// assert_eq!(it[0], vec![1, 2]); // Note: these are the same + /// assert_eq!(it[1], vec![1, 2]); // Note: these are the same + /// assert_eq!(it[2], vec![2, 2]); + /// ``` + /// + /// # Panics + /// Take into account that due to the factorial nature of the total possible combinations, + /// this method is likely to overflow. + /// The iterator will panic if the total number of combinations (`nCk`) is bigger than [usize::MAX]. + /// + /// The iterator will also panic if the total number of elements of the previous iterator + /// is smaller than k-length of the desired combinations. + fn combinations(self, k: usize) -> Combinations { + Combinations::new(self, k) + } + /// Creates a fresh collection containing all the elements produced /// by this parallel iterator. /// From ac78b34ee069265d2ee40cd74613e6c70ba50b6a Mon Sep 17 00:00:00 2001 From: Hydrogs Date: Thu, 26 Sep 2024 18:04:09 +0200 Subject: [PATCH 2/7] Add ParallelIterator::array_combinations Tries to share as much code as possible with ParallelIterator::combinations --- src/iter/combinations.rs | 376 ++++++++++++++++++++++++++++++++------- src/iter/mod.rs | 42 ++++- 2 files changed, 354 insertions(+), 64 deletions(-) diff --git a/src/iter/combinations.rs b/src/iter/combinations.rs index ed1dd9468..d67f3f20e 100644 --- a/src/iter/combinations.rs +++ b/src/iter/combinations.rs @@ -1,5 +1,4 @@ -#![warn(clippy::pedantic)] // TODO: remove - +use std::marker::PhantomData; use std::ops::Div; use std::sync::Arc; @@ -32,7 +31,7 @@ impl Combinations where I: ParallelIterator, { - fn into_producer(self) -> CombinationsProducer { + fn into_producer(self) -> CombinationsProducer> { let items: Arc<[I::Item]> = self.base.collect(); let n = items.len(); let k = self.k; @@ -43,6 +42,7 @@ where offset: 0, until, k: self.k, + indices: PhantomData, } } } @@ -90,20 +90,103 @@ where } } -struct CombinationsProducer { +/// `ArrayCombinations` is an iterator that iterates over the k-length combinations +/// of the elements from the original iterator. +/// This struct is created by the [`array_combinations()`] method on [`ParallelIterator`] +/// +/// [`combinations()`]: trait.ParallelIterator.html#method.combinations() +/// [`ParallelIterator`]: trait.ParallelIterator.html +#[must_use = "iterator adaptors are lazy and do nothing unless consumed"] +#[derive(Debug, Clone)] +pub struct ArrayCombinations { + base: I, +} + +impl ArrayCombinations { + /// Creates a new `Combinations` iterator. + pub(super) fn new(base: I) -> Self { + Self { base } + } +} + +impl ArrayCombinations +where + I: ParallelIterator, +{ + fn into_producer(self) -> CombinationsProducer { + let items: Arc<[I::Item]> = self.base.collect(); + let n = items.len(); + let until = checked_binomial(n, K).expect(OVERFLOW_MSG); + + CombinationsProducer { + items, + offset: 0, + until, + k: K, + indices: PhantomData, + } + } +} + +impl ParallelIterator for ArrayCombinations +where + I: ParallelIterator, + I::Item: Clone + Send + Sync, +{ + type Item = Vec; + + fn drive_unindexed(self, consumer: C) -> C::Result + where + C: UnindexedConsumer, + { + bridge_unindexed(self.into_producer(), consumer) + } + + fn opt_len(&self) -> Option { + // HACK: even though the length can be computed with the code below, + // our implementation of drive_unindexed does not support indexed Consumer + + // self.base + // .opt_len() + // .and_then(|l| checked_binomial(l, self.k)) + None + } +} + +impl IndexedParallelIterator for ArrayCombinations +where + I: IndexedParallelIterator, + I::Item: Clone + Send + Sync, +{ + fn len(&self) -> usize { + checked_binomial(self.base.len(), K).expect(OVERFLOW_MSG) + } + + fn drive>(self, consumer: C) -> C::Result { + bridge(self, consumer) + } + + fn with_producer>(self, callback: CB) -> CB::Output { + callback.callback(self.into_producer()) + } +} + +struct CombinationsProducer { items: Arc<[T]>, offset: usize, until: usize, k: usize, + indices: PhantomData, } -impl CombinationsProducer { +impl CombinationsProducer { fn new(items: Arc<[T]>, offset: usize, until: usize, k: usize) -> Self { Self { items, offset, until, k, + indices: PhantomData, } } @@ -116,12 +199,13 @@ impl CombinationsProducer { } } -impl Producer for CombinationsProducer +impl Producer for CombinationsProducer where T: Clone + Send + Sync, + I: AsRef<[usize]> + AsMut<[usize]> + PartialEq + Clone + DefaultWithSize + Send, { type Item = Vec; - type IntoIter = CombinationsSeq; + type IntoIter = CombinationsSeq; fn into_iter(self) -> Self::IntoIter { let n = self.n(); @@ -138,9 +222,10 @@ where } } -impl UnindexedProducer for CombinationsProducer +impl UnindexedProducer for CombinationsProducer where T: Clone + Send + Sync, + I: AsRef<[usize]> + AsMut<[usize]> + PartialEq + Clone + DefaultWithSize + Send, { type Item = Vec; @@ -161,23 +246,30 @@ where } } -struct CombinationsSeq { +struct CombinationsSeq { items: Arc<[T]>, - indices: Indices, - until: Indices, + indices: Indices, + until: Indices, len: usize, } -impl CombinationsSeq { +impl CombinationsSeq +where + I: AsMut<[usize]> + DefaultWithSize, +{ fn from_offsets(items: Arc<[T]>, from: usize, until: usize, n: usize, k: usize) -> Self { Self { - indices: Indices::new(from, n, k), - until: Indices::new(until, n, k), + indices: Indices::new(I::default_with_size(k), from, n), + until: Indices::new(I::default_with_size(k), until, n), items, len: until - from, } } - +} +impl CombinationsSeq +where + I: AsRef<[usize]> + PartialEq, +{ fn may_next(&self) -> Option<()> { if self.indices.is_empty() || self.until.is_empty() { // there are no indices to compute @@ -192,9 +284,10 @@ impl CombinationsSeq { } } -impl Iterator for CombinationsSeq +impl Iterator for CombinationsSeq where T: Clone, + I: AsRef<[usize]> + AsMut<[usize]> + PartialEq + Clone, { type Item = Vec; @@ -206,18 +299,20 @@ where } } -impl ExactSizeIterator for CombinationsSeq +impl ExactSizeIterator for CombinationsSeq where T: Clone, + I: AsRef<[usize]> + AsMut<[usize]> + PartialEq + Clone, { fn len(&self) -> usize { self.len } } -impl DoubleEndedIterator for CombinationsSeq +impl DoubleEndedIterator for CombinationsSeq where T: Clone, + I: AsRef<[usize]> + AsMut<[usize]> + PartialEq + Clone, { fn next_back(&mut self) -> Option { self.may_next()?; @@ -227,8 +322,8 @@ where } #[derive(PartialEq, Clone)] -struct Indices { - indices: Vec, +struct Indices { + indices: I, n: usize, position: IndicesPosition, } @@ -239,18 +334,24 @@ enum IndicesPosition { Middle, } -impl Indices { - fn new(offset: usize, n: usize, k: usize) -> Self { +impl Indices +where + I: AsMut<[usize]>, +{ + fn new(mut indices: I, offset: usize, n: usize) -> Self { + let k = indices.as_mut().len(); let total = checked_binomial(n, k).expect(OVERFLOW_MSG); if offset == total { + unrank(indices.as_mut(), offset - 1, n); Self { - indices: unrank(offset - 1, n, k), + indices, n, position: IndicesPosition::End, } } else if offset < total { + unrank(indices.as_mut(), offset, n); Self { - indices: unrank(offset, n, k), + indices, n, position: IndicesPosition::Middle, } @@ -262,10 +363,10 @@ impl Indices { fn increment(&mut self) { match self.position { IndicesPosition::Middle => { - if is_last(&self.indices, self.n) { + if is_last(self.indices.as_mut(), self.n) { self.position = IndicesPosition::End } else { - increment_indices(&mut self.indices, self.n); + increment_indices(self.indices.as_mut(), self.n); } } IndicesPosition::End => panic!("No more indices"), @@ -275,19 +376,30 @@ impl Indices { fn decrement(&mut self) { match self.position { IndicesPosition::End => self.position = IndicesPosition::Middle, - IndicesPosition::Middle => decrement_indices(&mut self.indices, self.n), + IndicesPosition::Middle => decrement_indices(self.indices.as_mut(), self.n), } } +} +impl Indices +where + I: AsRef<[usize]>, +{ fn deindex(&self, items: &[T]) -> Vec { match self.position { - IndicesPosition::Middle => self.indices.iter().map(|i| &items[*i]).cloned().collect(), + IndicesPosition::Middle => self + .indices + .as_ref() + .iter() + .map(|i| &items[*i]) + .cloned() + .collect(), IndicesPosition::End => panic!("End position indices cannot be deindexed."), } } fn is_empty(&self) -> bool { - self.indices.is_empty() + self.indices.as_ref().is_empty() } } @@ -355,23 +467,25 @@ fn checked_binomial(n: usize, k: usize) -> Option { /// # Panics /// - If n < k: not enough elements to form a list of indices /// - If the total number of combinations of `n` choose `k` is too big, roughly if it does not fit in usize. -fn unrank(offset: usize, n: usize, k: usize) -> Vec { +fn unrank(indices: &mut [usize], offset: usize, n: usize) { // Source: // Antoine Genitrini, Martin Pépin. Lexicographic unranking of combinations revisited. // Algorithms, 2021, 14 (3), pp.97. 10.3390/a14030097. hal-03040740v2 + let k = indices.len(); + assert!( n >= k, - "Not enough elements to choose k from. Note that n must be at least k." + "Not enough elements to choose k from. Note that n must be at least k. Called with n={n}, k={k}" ); debug_assert!(offset < checked_binomial(n, k).unwrap()); if k == 1 { - return vec![offset]; + indices[0] = offset; + return; } - let mut res = vec![0; k]; let mut b = checked_binomial(n - 1, k - 1).expect(OVERFLOW_MSG); let mut m = 0; let mut i = 0; @@ -387,7 +501,7 @@ fn unrank(offset: usize, n: usize, k: usize) -> Vec { } if b > u { - res[i] = m + i; + indices[i] = m + i; b *= k - i - 1; i += 1; } else { @@ -398,10 +512,8 @@ fn unrank(offset: usize, n: usize, k: usize) -> Vec { } if k > 0 { - res[k - 1] = n + u - b; + indices[k - 1] = n + u - b; } - - res } /// Increments indices representing the combination to advance to the next @@ -465,10 +577,45 @@ fn is_last(indices: &[usize], n: usize) -> bool { .all(|(i, &index)| index == n - k + i) } +trait DefaultWithSize { + fn default_with_size(size: usize) -> Self; +} + +impl DefaultWithSize for Vec +where + T: Default + Copy, +{ + fn default_with_size(size: usize) -> Self { + vec![T::default(); size] + } +} + +impl DefaultWithSize for [T; K] +where + T: Default + Copy, +{ + fn default_with_size(size: usize) -> Self { + assert_eq!(size, K); + [T::default(); K] + } +} + #[cfg(test)] mod tests { use super::*; + fn unrank_vec(offset: usize, n: usize, k: usize) -> Vec { + let mut indices = vec![0; k]; + unrank(&mut indices, offset, n); + indices + } + + fn unrank_const(offset: usize, n: usize) -> [usize; K] { + let mut indices = [0; K]; + unrank(&mut indices, offset, n); + indices + } + #[test] fn checked_binomial_works() { assert_eq!(checked_binomial(7, 2), Some(21)); @@ -534,23 +681,24 @@ mod tests { #[test] fn unrank_5_2() { - assert_eq!(unrank(0, 5, 2), vec![0, 1]); - assert_eq!(unrank(1, 5, 2), vec![0, 2]); - assert_eq!(unrank(2, 5, 2), vec![0, 3]); - assert_eq!(unrank(3, 5, 2), vec![0, 4]); - assert_eq!(unrank(4, 5, 2), vec![1, 2]); - assert_eq!(unrank(5, 5, 2), vec![1, 3]); - assert_eq!(unrank(6, 5, 2), vec![1, 4]); - assert_eq!(unrank(7, 5, 2), vec![2, 3]); - assert_eq!(unrank(8, 5, 2), vec![2, 4]); - assert_eq!(unrank(9, 5, 2), vec![3, 4]); + assert_eq!(unrank_vec(0, 5, 2), vec![0, 1]); + assert_eq!(unrank_vec(1, 5, 2), vec![0, 2]); + assert_eq!(unrank_vec(2, 5, 2), vec![0, 3]); + assert_eq!(unrank_vec(3, 5, 2), vec![0, 4]); + assert_eq!(unrank_vec(4, 5, 2), vec![1, 2]); + assert_eq!(unrank_vec(5, 5, 2), vec![1, 3]); + assert_eq!(unrank_vec(6, 5, 2), vec![1, 4]); + assert_eq!(unrank_vec(7, 5, 2), vec![2, 3]); + assert_eq!(unrank_vec(8, 5, 2), vec![2, 4]); + assert_eq!(unrank_vec(9, 5, 2), vec![3, 4]); } #[test] fn unrank_zero_index() { for k in 1..7 { for n in k..11 { - assert_eq!(unrank(0, n, k), (0..k).collect::>()); + dbg!(n, k); + assert_eq!(unrank_vec(0, n, k), (0..k).collect::>()); } } } @@ -561,7 +709,7 @@ mod tests { for k in 1..n { let mut expected: Vec<_> = (0..k).collect(); *expected.last_mut().unwrap() += 1; - assert_eq!(unrank(1, n, k), expected); + assert_eq!(unrank_vec(1, n, k), expected); } } } @@ -572,7 +720,7 @@ mod tests { for k in 1..n { let expected: Vec<_> = ((n - k)..n).collect(); let offset = checked_binomial(n, k).unwrap() - 1; - assert_eq!(unrank(offset, n, k), expected); + assert_eq!(unrank_vec(offset, n, k), expected); } } } @@ -580,8 +728,8 @@ mod tests { #[test] fn unrank_k1() { for n in 1..50 { - for u in 0..n { - assert_eq!(unrank(u, n, 1), vec![u]); + for offset in 0..n { + assert_eq!(unrank_const(offset, n), [offset]); } } } @@ -591,9 +739,9 @@ mod tests { for i in 0..16 { for n in (i + 2)..(2 * (i + 1)) { for k in 1..n { - let mut indices = unrank(i, n, k); + let mut indices = unrank_vec(i, n, k); increment_indices(&mut indices, n); - assert_eq!(unrank(i + 1, n, k), indices); + assert_eq!(unrank_vec(i + 1, n, k), indices); } } } @@ -605,9 +753,9 @@ mod tests { for k in 1..n { let max_iter = checked_binomial(n, k).unwrap(); for i in (max_iter - n / 2)..(max_iter - 1) { - let mut indices = unrank(i, n, k); + let mut indices = unrank_vec(i, n, k); increment_indices(&mut indices, n); - assert_eq!(unrank(i + 1, n, k), indices); + assert_eq!(unrank_vec(i + 1, n, k), indices); } } } @@ -618,9 +766,9 @@ mod tests { for i in 1..16 { for n in (i + 2)..(2 * (i + 1)) { for k in 1..n { - let mut indices = unrank(i, n, k); + let mut indices = unrank_vec(i, n, k); decrement_indices(&mut indices, n); - assert_eq!(unrank(i - 1, n, k), indices); + assert_eq!(unrank_vec(i - 1, n, k), indices); } } } @@ -632,9 +780,9 @@ mod tests { for k in 1..n { let max_iter = checked_binomial(n, k).unwrap(); for i in (max_iter - n / 2)..(max_iter) { - let mut indices = unrank(i, n, k); + let mut indices = unrank_vec(i, n, k); decrement_indices(&mut indices, n); - assert_eq!(unrank(i - 1, n, k), indices); + assert_eq!(unrank_vec(i - 1, n, k), indices); } } } @@ -644,9 +792,9 @@ mod tests { #[should_panic] fn unrank_offset_outofbounds() { let n = 10; - let k = 2; - let u = checked_binomial(n, k).unwrap() + 1; - unrank(u, n, k); + const K: usize = 2; + let u = checked_binomial(n, K).unwrap() + 1; + unrank_const::(u, n); } #[test] @@ -739,4 +887,106 @@ mod tests { .collect::>() ); } + + #[test] + fn array_par_iter_works() { + let a: Vec<_> = (0..5).into_par_iter().array_combinations::<2>().collect(); + + // FIXME: somewhere `until` is not correctly set + assert_eq!( + a, + vec![ + [0, 1], + [0, 2], + [0, 3], + [0, 4], + [1, 2], + [1, 3], + [1, 4], + [2, 3], + [2, 4], + [3, 4], + ] + ); + + let b: Vec<_> = (0..5).into_par_iter().array_combinations::<3>().collect(); + + assert_eq!( + b, + vec![ + [0, 1, 2], + [0, 1, 3], + [0, 1, 4], + [0, 2, 3], + [0, 2, 4], + [0, 3, 4], + [1, 2, 3], + [1, 2, 4], + [1, 3, 4], + [2, 3, 4], + ] + ); + } + + #[test] + fn array_par_iter_k1() { + assert_eq!( + (0..200) + .into_par_iter() + .array_combinations::<1>() + .collect::>(), + (0..200).map(|i| [i]).collect::>() + ) + } + + #[test] + fn array_par_iter_rev_works() { + let a: Vec<_> = (0..5) + .into_par_iter() + .array_combinations::<2>() + .rev() + .collect(); + assert_eq!( + a, + vec![ + [0, 1], + [0, 2], + [0, 3], + [0, 4], + [1, 2], + [1, 3], + [1, 4], + [2, 3], + [2, 4], + [3, 4], + ] + .into_iter() + .rev() + .collect::>() + ); + + let b: Vec<_> = (0..5) + .into_par_iter() + .array_combinations::<3>() + .rev() + .collect(); + assert_eq!( + b, + vec![ + [0, 1, 2], + [0, 1, 3], + [0, 1, 4], + [0, 2, 3], + [0, 2, 4], + [0, 3, 4], + [1, 2, 3], + [1, 2, 4], + [1, 3, 4], + [2, 3, 4], + ] + .into_iter() + .rev() + .collect::>() + ); + } } diff --git a/src/iter/mod.rs b/src/iter/mod.rs index 5dd8918ac..992f7c7e8 100644 --- a/src/iter/mod.rs +++ b/src/iter/mod.rs @@ -167,7 +167,7 @@ pub use self::{ chain::Chain, chunks::Chunks, cloned::Cloned, - combinations::Combinations, + combinations::{ArrayCombinations, Combinations}, copied::Copied, empty::{empty, Empty}, enumerate::Enumerate, @@ -1964,6 +1964,46 @@ pub trait ParallelIterator: Sized + Send { PanicFuse::new(self) } + /// Creates an iterator adaptor that iterates over the `k`-length combinations of the + /// elements from an iterator. + /// + /// Iterator element type is `[Self::Item; K]`. + /// The iterator produces a new array per iteration, + /// and clones the iterator elements. + /// + /// # Guarantees + /// If the adapted iterator is deterministic, + /// this iterator adapter yields items in a reliable order. + /// + /// ``` + /// use rayon::prelude::*; + /// + /// let v: Vec<_> = (1..5).into_par_iter().array_combinations::<2>().collect(); + /// assert_eq!(v, vec![[1, 2], [1, 3], [1, 4], [2, 3], [2, 4], [3, 4]]); + /// ``` + /// + /// Note: Combinations does not take into account the equality of the iterated values. + /// ``` + /// use rayon::prelude::*; + /// + /// let it: Vec<_> = [1, 2, 2].into_par_iter().array_combinations::<2>().collect(); + /// + /// assert_eq!(it[0], [1, 2]); // Note: these are the same + /// assert_eq!(it[1], [1, 2]); // Note: these are the same + /// assert_eq!(it[2], [2, 2]); + /// ``` + /// + /// # Panics + /// Take into account that due to the factorial nature of the total possible combinations, + /// this method is likely to overflow. + /// The iterator will panic if the total number of combinations (`nCk`) is bigger than [usize::MAX]. + /// + /// The iterator will also panic if the total number of elements of the previous iterator + /// is smaller than k-length of the desired combinations. + fn array_combinations(self) -> ArrayCombinations { + ArrayCombinations::new(self) + } + /// Creates an iterator that iterates over the `k`-length combinations /// of the elements from the original iterator. /// From 0b0faf138c3193fb5418c6417457f65fe783cf14 Mon Sep 17 00:00:00 2001 From: Hydrogs Date: Thu, 26 Sep 2024 18:57:00 +0200 Subject: [PATCH 3/7] Use collect_into_vec for Indexed iterators --- src/iter/combinations.rs | 91 +++++++++++++++++++++------------------- 1 file changed, 48 insertions(+), 43 deletions(-) diff --git a/src/iter/combinations.rs b/src/iter/combinations.rs index d67f3f20e..fcc10da3a 100644 --- a/src/iter/combinations.rs +++ b/src/iter/combinations.rs @@ -27,26 +27,6 @@ impl Combinations { } } -impl Combinations -where - I: ParallelIterator, -{ - fn into_producer(self) -> CombinationsProducer> { - let items: Arc<[I::Item]> = self.base.collect(); - let n = items.len(); - let k = self.k; - let until = checked_binomial(n, k).expect(OVERFLOW_MSG); - - CombinationsProducer { - items, - offset: 0, - until, - k: self.k, - indices: PhantomData, - } - } -} - impl ParallelIterator for Combinations where I: ParallelIterator, @@ -58,7 +38,8 @@ where where C: UnindexedConsumer, { - bridge_unindexed(self.into_producer(), consumer) + let producer = CombinationsProducer::<_, Vec<_>>::from_unindexed(self.base, self.k); + bridge_unindexed(producer, consumer) } fn opt_len(&self) -> Option { @@ -86,7 +67,8 @@ where } fn with_producer>(self, callback: CB) -> CB::Output { - callback.callback(self.into_producer()) + let producer = CombinationsProducer::<_, Vec<_>>::from_indexed(self.base, self.k); + callback.callback(producer) } } @@ -109,25 +91,6 @@ impl ArrayCombinations { } } -impl ArrayCombinations -where - I: ParallelIterator, -{ - fn into_producer(self) -> CombinationsProducer { - let items: Arc<[I::Item]> = self.base.collect(); - let n = items.len(); - let until = checked_binomial(n, K).expect(OVERFLOW_MSG); - - CombinationsProducer { - items, - offset: 0, - until, - k: K, - indices: PhantomData, - } - } -} - impl ParallelIterator for ArrayCombinations where I: ParallelIterator, @@ -139,7 +102,17 @@ where where C: UnindexedConsumer, { - bridge_unindexed(self.into_producer(), consumer) + let items: Arc<[I::Item]> = self.base.collect(); + let n = items.len(); + let until = checked_binomial(n, K).expect(OVERFLOW_MSG); + let producer = CombinationsProducer { + items, + offset: 0, + until, + k: K, + indices: PhantomData::<[usize; K]>, + }; + bridge_unindexed(producer, consumer) } fn opt_len(&self) -> Option { @@ -167,7 +140,8 @@ where } fn with_producer>(self, callback: CB) -> CB::Output { - callback.callback(self.into_producer()) + let producer = CombinationsProducer::<_, [_; K]>::from_indexed(self.base, K); + callback.callback(producer) } } @@ -199,6 +173,37 @@ impl CombinationsProducer { } } +impl CombinationsProducer +where + T: Send, +{ + fn from_unindexed(base: B, k: usize) -> Self + where + B: ParallelIterator, + { + let items: Arc<[T]> = base.collect(); + let n = items.len(); + let until = checked_binomial(n, k).expect(OVERFLOW_MSG); + + Self::new(items, 0, until, k) + } + + fn from_indexed(base: B, k: usize) -> Self + where + B: IndexedParallelIterator, + { + let n = base.len(); + let items = { + let mut buffer = Vec::with_capacity(n); + base.collect_into_vec(&mut buffer); + buffer.into() + }; + let until = checked_binomial(n, k).expect(OVERFLOW_MSG); + + Self::new(items, 0, until, k) + } +} + impl Producer for CombinationsProducer where T: Clone + Send + Sync, From 0b9fa057d543045bd700c95b1701ff2473ecc5c6 Mon Sep 17 00:00:00 2001 From: Hydrogs Date: Thu, 26 Sep 2024 20:29:47 +0200 Subject: [PATCH 4/7] Change binomial to reduce overflows It uses the implementations from Itertools --- src/iter/combinations.rs | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/src/iter/combinations.rs b/src/iter/combinations.rs index fcc10da3a..209c9f61e 100644 --- a/src/iter/combinations.rs +++ b/src/iter/combinations.rs @@ -413,10 +413,6 @@ where /// The binomial coefficient, often denoted as C(n, k) or "n choose k", represents /// the number of ways to choose `k` elements from a set of `n` elements without regard /// to the order of selection. -/// -/// # Overflow -/// Note that this function will overflow even if the result is smaller than [`isize::MAX`]. -/// The guarantee is that it does not overflow if `nCk * k <= isize::MAX`. fn checked_binomial(n: usize, k: usize) -> Option { if k > n { return Some(0); @@ -459,9 +455,12 @@ fn checked_binomial(n: usize, k: usize) -> Option { // `factorial(n) / factorial(n - k) / factorial(k)` but trying to avoid overflows let mut result: usize = 1; - for i in 0..k { - result = result.checked_mul(n - i)?; - result /= i + 1; + let mut n = n; + for i in 1..=k { + result = (result / i) + .checked_mul(n)? + .checked_add((result % i).checked_mul(n)? / i)?; + n -= 1; } Some(result) } @@ -670,12 +669,6 @@ mod tests { assert_eq!(checked_binomial(109, 15), Some(1_015_428_940_004_440_080)); } - #[test] - #[cfg(target_pointer_width = "64")] - fn checked_overflows() { - assert_eq!(checked_binomial(109, 16), None); - } - #[test] fn checked_binomial_identity() { assert_eq!(checked_binomial(1, 1), Some(1)); From 73598ce5d51e7d28ec87112cdcd1e89888ed7d19 Mon Sep 17 00:00:00 2001 From: Adrian Date: Tue, 1 Oct 2024 02:02:51 +0200 Subject: [PATCH 5/7] Fix incorrect overflow handling in binomial In cases where `k` was 2 or 3, no guard was used, so this function could return `None`, when the general algorithm can compute the actual Some(_). Also, checked arithmetic was used when it is not needed, since the initial guards are enough to assure no overflows will occur. Tests where changed to include these cases. --- src/iter/combinations.rs | 52 +++++++++------------------------------- 1 file changed, 11 insertions(+), 41 deletions(-) diff --git a/src/iter/combinations.rs b/src/iter/combinations.rs index 209c9f61e..59fd150d2 100644 --- a/src/iter/combinations.rs +++ b/src/iter/combinations.rs @@ -1,5 +1,4 @@ use std::marker::PhantomData; -use std::ops::Div; use std::sync::Arc; use super::plumbing::*; @@ -424,32 +423,12 @@ fn checked_binomial(n: usize, k: usize) -> Option { // Fast paths x2 ~ x6 speedup match k { 1 => return Some(n), - 2 => return n.checked_mul(n - 1).map(|r| r / 2), - 3 => { - return n - .checked_mul(n - 1)? - .div(2) - .checked_mul(n - 2) - .map(|r| r / 3) - } - 4 if n <= 77_937 /* prevent unnecessary overflow */ => { - return n - .checked_mul(n - 1)? - .div(2) - .checked_mul(n - 2)? - .checked_mul(n - 3) - .map(|r| r / 12) - } - 5 if n <= 10_811 /* prevent unnecessary overflow */ => { - return n - .checked_mul(n - 1)? - .div(2) - .checked_mul(n - 2)? - .checked_mul(n - 3)? - .div(4) - .checked_mul(n - 4) - .map(|r| r / 15) - } + // The guards prevent unnecessary overflows, + // those cases can be handled by the general checked algorithm + 2 if n <= 4_294_967_296 => return Some(n * (n - 1) / 2), + 3 if n <= 3_329_022 => return Some(n * (n - 1) / 2 * (n - 2) / 3), + 4 if n <= 77_937 => return Some(n * (n - 1) / 2 * (n - 2) * (n - 3) / 12), + 5 if n <= 10_811 => return Some(n * (n - 1) / 2 * (n - 2) * (n - 3) / 4 * (n - 4) / 15), _ => {} } @@ -653,20 +632,11 @@ mod tests { #[test] #[cfg(target_pointer_width = "64")] fn checked_binomial_not_overflows() { - assert_eq!( - checked_binomial(4_294_967_296, 2), - Some(9_223_372_034_707_292_160) - ); - assert_eq!( - checked_binomial(3_329_022, 3), - Some(6_148_913_079_097_324_540) - ); - assert_eq!( - checked_binomial(102_570, 4), - Some(4_611_527_207_790_103_770) - ); - assert_eq!(checked_binomial(13_467, 5), Some(3_688_506_309_678_005_178)); - assert_eq!(checked_binomial(109, 15), Some(1_015_428_940_004_440_080)); + assert!(checked_binomial(4_294_967_297, 2).is_some()); + assert!(checked_binomial(3_329_023, 3).is_some()); + assert!(checked_binomial(102_570, 4).is_some()); + assert!(checked_binomial(13_467, 5).is_some()); + assert!(checked_binomial(109, 15).is_some()); } #[test] From 5bbbe24c64c811beb639c7aa038c3b7045746945 Mon Sep 17 00:00:00 2001 From: Adrian Date: Tue, 1 Oct 2024 02:23:43 +0200 Subject: [PATCH 6/7] Make Clippy happy --- src/iter/combinations.rs | 38 ++++++++++++++++++++++---------------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/src/iter/combinations.rs b/src/iter/combinations.rs index 59fd150d2..437fff5de 100644 --- a/src/iter/combinations.rs +++ b/src/iter/combinations.rs @@ -345,22 +345,26 @@ where fn new(mut indices: I, offset: usize, n: usize) -> Self { let k = indices.as_mut().len(); let total = checked_binomial(n, k).expect(OVERFLOW_MSG); - if offset == total { - unrank(indices.as_mut(), offset - 1, n); - Self { - indices, - n, - position: IndicesPosition::End, + match offset.cmp(&total) { + Ordering::Equal => { + unrank(indices.as_mut(), offset - 1, n); + Self { + indices, + n, + position: IndicesPosition::End, + } } - } else if offset < total { - unrank(indices.as_mut(), offset, n); - Self { - indices, - n, - position: IndicesPosition::Middle, + Ordering::Less => { + unrank(indices.as_mut(), offset, n); + Self { + indices, + n, + position: IndicesPosition::Middle, + } + } + Ordering::Greater => { + panic!("Offset should be inside the bounds of the possible combinations.") } - } else { - panic!("Offset should be inside the bounds of the possible combinations.") } } @@ -545,8 +549,10 @@ fn decrement_indices(indices: &mut [usize], n: usize) { // Decrement index, and reset the ones to its right indices[i] -= 1; - for j in i + 1..indices.len() { - indices[j] = n - k + j; + for (j, index) in indices.iter_mut().enumerate().skip(i + 1) { + *index = n - k + j; + for (j, index) in indices.iter_mut().enumerate().skip(i + 1) { + *index = n - k + j; } } From dcdb94c475bb9fc36462d3e67c04e85f885b1f87 Mon Sep 17 00:00:00 2001 From: Adrian Date: Tue, 1 Oct 2024 05:26:34 +0200 Subject: [PATCH 7/7] Fix broken code --- src/iter/combinations.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/iter/combinations.rs b/src/iter/combinations.rs index 437fff5de..f9646c971 100644 --- a/src/iter/combinations.rs +++ b/src/iter/combinations.rs @@ -549,8 +549,6 @@ fn decrement_indices(indices: &mut [usize], n: usize) { // Decrement index, and reset the ones to its right indices[i] -= 1; - for (j, index) in indices.iter_mut().enumerate().skip(i + 1) { - *index = n - k + j; for (j, index) in indices.iter_mut().enumerate().skip(i + 1) { *index = n - k + j; }