Skip to content

Commit

Permalink
Deprecate tree_fold1 for tree_reduce
Browse files Browse the repository at this point in the history
And rename various identifiers.
  • Loading branch information
Philippe-Cholet authored and jswrenn committed Mar 14, 2024
1 parent 78a9e7e commit 45c5dec
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 33 deletions.
38 changes: 19 additions & 19 deletions benches/tree_reduce.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use itertools::{cloned, Itertools};
trait IterEx: Iterator {
// Another efficient implementation against which to compare,
// but needs `std` so is less desirable.
fn tree_fold1_vec<F>(self, mut f: F) -> Option<Self::Item>
fn tree_reduce_vec<F>(self, mut f: F) -> Option<Self::Item>
where
F: FnMut(Self::Item, Self::Item) -> Self::Item,
Self: Sized,
Expand Down Expand Up @@ -91,14 +91,14 @@ def_benchs! {

def_benchs! {
10_000,
tree_fold1,
tree_fold1_stack_10k,
tree_reduce,
tree_reduce_stack_10k,
}

def_benchs! {
10_000,
tree_fold1_vec,
tree_fold1_vec_10k,
tree_reduce_vec,
tree_reduce_vec_10k,
}

def_benchs! {
Expand All @@ -109,14 +109,14 @@ def_benchs! {

def_benchs! {
100,
tree_fold1,
tree_fold1_stack_100,
tree_reduce,
tree_reduce_stack_100,
}

def_benchs! {
100,
tree_fold1_vec,
tree_fold1_vec_100,
tree_reduce_vec,
tree_reduce_vec_100,
}

def_benchs! {
Expand All @@ -127,24 +127,24 @@ def_benchs! {

def_benchs! {
8,
tree_fold1,
tree_fold1_stack_08,
tree_reduce,
tree_reduce_stack_08,
}

def_benchs! {
8,
tree_fold1_vec,
tree_fold1_vec_08,
tree_reduce_vec,
tree_reduce_vec_08,
}

criterion_main!(
fold1_10k,
tree_fold1_stack_10k,
tree_fold1_vec_10k,
tree_reduce_stack_10k,
tree_reduce_vec_10k,
fold1_100,
tree_fold1_stack_100,
tree_fold1_vec_100,
tree_reduce_stack_100,
tree_reduce_vec_100,
fold1_08,
tree_fold1_stack_08,
tree_fold1_vec_08,
tree_reduce_stack_08,
tree_reduce_vec_08,
);
26 changes: 18 additions & 8 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2466,7 +2466,7 @@ pub trait Itertools: Iterator {
/// - if `f` is a trivial operation like `u32::wrapping_add`, prefer the normal
/// [`Iterator::reduce`] instead since it will most likely result in the generation of simpler
/// code because the compiler is able to optimize it
/// - otherwise if `f` is non-trivial like `format!`, you should use `tree_fold1` since it
/// - otherwise if `f` is non-trivial like `format!`, you should use `tree_reduce` since it
/// reduces the number of operations from `O(n)` to `O(ln(n))`
///
/// Here "non-trivial" means:
Expand All @@ -2479,20 +2479,20 @@ pub trait Itertools: Iterator {
///
/// // The same tree as above
/// let num_strings = (1..8).map(|x| x.to_string());
/// assert_eq!(num_strings.tree_fold1(|x, y| format!("f({}, {})", x, y)),
/// assert_eq!(num_strings.tree_reduce(|x, y| format!("f({}, {})", x, y)),
/// Some(String::from("f(f(f(1, 2), f(3, 4)), f(f(5, 6), 7))")));
///
/// // Like fold1, an empty iterator produces None
/// assert_eq!((0..0).tree_fold1(|x, y| x * y), None);
/// assert_eq!((0..0).tree_reduce(|x, y| x * y), None);
///
/// // tree_fold1 matches fold1 for associative operations...
/// assert_eq!((0..10).tree_fold1(|x, y| x + y),
/// // tree_reduce matches fold1 for associative operations...
/// assert_eq!((0..10).tree_reduce(|x, y| x + y),
/// (0..10).fold1(|x, y| x + y));
/// // ...but not for non-associative ones
/// assert_ne!((0..10).tree_fold1(|x, y| x - y),
/// assert_ne!((0..10).tree_reduce(|x, y| x - y),
/// (0..10).fold1(|x, y| x - y));
/// ```
fn tree_fold1<F>(mut self, mut f: F) -> Option<Self::Item>
fn tree_reduce<F>(mut self, mut f: F) -> Option<Self::Item>
where
F: FnMut(Self::Item, Self::Item) -> Self::Item,
Self: Sized,
Expand All @@ -2505,7 +2505,7 @@ pub trait Itertools: Iterator {
FF: FnMut(T, T) -> T,
{
// This function could be replaced with `it.next().ok_or(None)`,
// but half the useful tree_fold1 work is combining adjacent items,
// but half the useful tree_reduce work is combining adjacent items,
// so put that in a form that LLVM is more likely to optimize well.

let a = if let Some(v) = it.next() {
Expand Down Expand Up @@ -2555,6 +2555,16 @@ pub trait Itertools: Iterator {
}
}

/// See [`.tree_reduce()`](Itertools::tree_reduce).
#[deprecated(note = "Use .tree_reduce() instead", since = "0.13.0")]
fn tree_fold1<F>(self, f: F) -> Option<Self::Item>
where
F: FnMut(Self::Item, Self::Item) -> Self::Item,
Self: Sized,
{
self.tree_reduce(f)
}

/// An iterator method that applies a function, producing a single, final value.
///
/// `fold_while()` is basically equivalent to [`Iterator::fold`] but with additional support for
Expand Down
4 changes: 2 additions & 2 deletions tests/quick.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1334,7 +1334,7 @@ quickcheck! {
}

quickcheck! {
fn tree_fold1_f64(mut a: Vec<f64>) -> TestResult {
fn tree_reduce_f64(mut a: Vec<f64>) -> TestResult {
fn collapse_adjacent<F>(x: Vec<f64>, mut f: F) -> Vec<f64>
where F: FnMut(f64, f64) -> f64
{
Expand All @@ -1353,7 +1353,7 @@ quickcheck! {
return TestResult::discard();
}

let actual = a.iter().cloned().tree_fold1(f64::atan2);
let actual = a.iter().cloned().tree_reduce(f64::atan2);

while a.len() > 1 {
a = collapse_adjacent(a, f64::atan2);
Expand Down
4 changes: 2 additions & 2 deletions tests/test_core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -278,9 +278,9 @@ fn part() {
}

#[test]
fn tree_fold1() {
fn tree_reduce() {
for i in 0..100 {
assert_eq!((0..i).tree_fold1(|x, y| x + y), (0..i).fold1(|x, y| x + y));
assert_eq!((0..i).tree_reduce(|x, y| x + y), (0..i).fold1(|x, y| x + y));
}
}

Expand Down
4 changes: 2 additions & 2 deletions tests/test_std.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1434,7 +1434,7 @@ fn fold_while() {
}

#[test]
fn tree_fold1() {
fn tree_reduce() {
let x = [
"",
"0",
Expand All @@ -1461,7 +1461,7 @@ fn tree_fold1() {
Some(s.to_string())
};
let num_strings = (0..i).map(|x| x.to_string());
let actual = num_strings.tree_fold1(|a, b| format!("{} {} x", a, b));
let actual = num_strings.tree_reduce(|a, b| format!("{} {} x", a, b));
assert_eq!(actual, expected);
}
}
Expand Down

0 comments on commit 45c5dec

Please sign in to comment.