From 7b4e0dea75a27989c56dfbfa8aeebbdaf11304c9 Mon Sep 17 00:00:00 2001 From: Jim Turner Date: Sun, 31 Mar 2019 22:02:24 -0400 Subject: [PATCH 01/19] Clarify docs of get_many_from_sorted_mut_unchecked --- src/sort.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/sort.rs b/src/sort.rs index 568c0f54..f578e24c 100644 --- a/src/sort.rs +++ b/src/sort.rs @@ -183,10 +183,11 @@ where } /// To retrieve multiple indexes from the sorted array in an optimized fashion, -/// [get_many_from_sorted_mut] first of all sorts the `indexes` vector. +/// [get_many_from_sorted_mut] first of all sorts and deduplicates the +/// `indexes` vector. /// -/// `get_many_from_sorted_mut_unchecked` does not perform this sorting, -/// assuming that the user has already taken care of it. +/// `get_many_from_sorted_mut_unchecked` does not perform this sorting and +/// deduplication, assuming that the user has already taken care of it. /// /// Useful when you have to call [get_many_from_sorted_mut] multiple times /// using the same indexes. From 64ed72b1368765aac3ed1b3d061fbaac94109651 Mon Sep 17 00:00:00 2001 From: Jim Turner Date: Sun, 31 Mar 2019 23:58:23 -0400 Subject: [PATCH 02/19] Add get_many_from_sorted_mut benchmark --- Cargo.toml | 5 +++++ benches/sort.rs | 42 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+) create mode 100644 benches/sort.rs diff --git a/Cargo.toml b/Cargo.toml index cf7f8615..6f9b2755 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -23,7 +23,12 @@ itertools = { version = "0.7.0", default-features = false } indexmap = "1.0" [dev-dependencies] +criterion = "0.2" quickcheck = { version = "0.8.1", default-features = false } ndarray-rand = "0.9" approx = "0.3" quickcheck_macros = "0.8" + +[[bench]] +name = "sort" +harness = false diff --git a/benches/sort.rs b/benches/sort.rs new file mode 100644 index 00000000..febcf7b7 --- /dev/null +++ b/benches/sort.rs @@ -0,0 +1,42 @@ +extern crate criterion; +extern crate ndarray; +extern crate ndarray_stats; +extern crate rand; + +use criterion::{ + black_box, criterion_group, criterion_main, AxisScale, BatchSize, Criterion, + ParameterizedBenchmark, PlotConfiguration, +}; +use ndarray::prelude::*; +use ndarray_stats::Sort1dExt; +use rand::prelude::*; + +fn get_many_from_sorted_mut(c: &mut Criterion) { + let lens = vec![10, 100, 1000, 10000]; + let benchmark = ParameterizedBenchmark::new( + "get_many_from_sorted_mut", + |bencher, &len| { + let mut rng = StdRng::seed_from_u64(42); + let mut data: Vec<_> = (0..len).collect(); + data.shuffle(&mut rng); + let indices: Vec<_> = (0..len).step_by(len / 10).collect(); + bencher.iter_batched( + || Array1::from(data.clone()), + |mut arr| { + black_box(arr.get_many_from_sorted_mut(&indices)); + }, + BatchSize::SmallInput, + ) + }, + lens, + ) + .plot_config(PlotConfiguration::default().summary_scale(AxisScale::Logarithmic)); + c.bench("get_many_from_sorted_mut", benchmark); +} + +criterion_group! { + name = benches; + config = Criterion::default(); + targets = get_many_from_sorted_mut +} +criterion_main!(benches); From 2c9030985ffc2f287281e35005e9cef4fe465f6c Mon Sep 17 00:00:00 2001 From: Jim Turner Date: Mon, 1 Apr 2019 00:39:01 -0400 Subject: [PATCH 03/19] Add get_from_sorted_mut benchmark --- benches/sort.rs | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/benches/sort.rs b/benches/sort.rs index febcf7b7..cdcf8dd3 100644 --- a/benches/sort.rs +++ b/benches/sort.rs @@ -11,6 +11,31 @@ use ndarray::prelude::*; use ndarray_stats::Sort1dExt; use rand::prelude::*; +fn get_from_sorted_mut(c: &mut Criterion) { + let lens = vec![10, 100, 1000, 10000]; + let benchmark = ParameterizedBenchmark::new( + "get_from_sorted_mut", + |bencher, &len| { + let mut rng = StdRng::seed_from_u64(42); + let mut data: Vec<_> = (0..len).collect(); + data.shuffle(&mut rng); + let indices: Vec<_> = (0..len).step_by(len / 10).collect(); + bencher.iter_batched( + || Array1::from(data.clone()), + |mut arr| { + for &i in &indices { + black_box(arr.get_from_sorted_mut(i)); + } + }, + BatchSize::SmallInput, + ) + }, + lens, + ) + .plot_config(PlotConfiguration::default().summary_scale(AxisScale::Logarithmic)); + c.bench("get_from_sorted_mut", benchmark); +} + fn get_many_from_sorted_mut(c: &mut Criterion) { let lens = vec![10, 100, 1000, 10000]; let benchmark = ParameterizedBenchmark::new( @@ -37,6 +62,6 @@ fn get_many_from_sorted_mut(c: &mut Criterion) { criterion_group! { name = benches; config = Criterion::default(); - targets = get_many_from_sorted_mut + targets = get_from_sorted_mut, get_many_from_sorted_mut } criterion_main!(benches); From 3a4ea2e0c9fdd5a2c15d2fde18f55a283d6f8749 Mon Sep 17 00:00:00 2001 From: Jim Turner Date: Sun, 31 Mar 2019 22:02:32 -0400 Subject: [PATCH 04/19] Simplify get_many_from_sorted_mut_unchecked --- src/sort.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/sort.rs b/src/sort.rs index f578e24c..969ca151 100644 --- a/src/sort.rs +++ b/src/sort.rs @@ -205,11 +205,7 @@ where let values = _get_many_from_sorted_mut_unchecked(array, indexes); // We convert the vector to a more search-friendly IndexMap - let mut result = IndexMap::new(); - for (index, value) in indexes.into_iter().zip(values.into_iter()) { - result.insert(*index, value); - } - result + indexes.iter().cloned().zip(values.into_iter()).collect() } fn _get_many_from_sorted_mut_unchecked( From e5c94740ffdb5161ca7b967986d6f871e9bc0654 Mon Sep 17 00:00:00 2001 From: Jim Turner Date: Mon, 1 Apr 2019 00:30:24 -0400 Subject: [PATCH 05/19] Eliminate allocations from _get_many_from_sorted_mut_unchecked This improves performance, especially for small arrays. --- src/sort.rs | 120 +++++++++++++++++++++++++++++----------------------- 1 file changed, 68 insertions(+), 52 deletions(-) diff --git a/src/sort.rs b/src/sort.rs index 969ca151..88e37808 100644 --- a/src/sort.rs +++ b/src/sort.rs @@ -3,6 +3,7 @@ use ndarray::prelude::*; use ndarray::{s, Data, DataMut}; use rand::prelude::*; use rand::thread_rng; +use std::iter; /// Methods for sorting and partitioning 1-D arrays. pub trait Sort1dExt @@ -201,81 +202,96 @@ where A: Ord + Clone, S: DataMut, { - // The actual routine - let values = _get_many_from_sorted_mut_unchecked(array, indexes); + if indexes.is_empty() { + return IndexMap::new(); + } + + // Since `!indexes.is_empty` and indexes must be in-bounds, `array` must be + // non-empty. + let mut values: Vec<_> = iter::repeat(array[0].clone()).take(indexes.len()).collect(); + _get_many_from_sorted_mut_unchecked(array.view_mut(), &mut indexes.to_owned(), &mut values); - // We convert the vector to a more search-friendly IndexMap + // We convert the vector to a more search-friendly `IndexMap`. indexes.iter().cloned().zip(values.into_iter()).collect() } -fn _get_many_from_sorted_mut_unchecked( - array: &mut ArrayBase, - indexes: &[usize], -) -> Vec -where +/// This is the recursive portion of `get_many_from_sorted_mut_unchecked`. +/// +/// `indexes` is the list of indexes to get. `indexes` is mutable so that it +/// can be used as scratch space for this routine; the value of `indexes` after +/// calling this routine should be ignored. +/// +/// `values` is a pre-allocated slice to use for writing the output. Its +/// initial element values are ignored. +fn _get_many_from_sorted_mut_unchecked( + mut array: ArrayViewMut1, + indexes: &mut [usize], + values: &mut [A], +) where A: Ord + Clone, - S: DataMut, { let n = array.len(); + debug_assert!(n >= indexes.len()); // because indexes must be unique and in-bounds + debug_assert_eq!(indexes.len(), values.len()); - // Nothing to do in this case - if indexes.len() == 0 || n == 0 { - return vec![]; + if indexes.is_empty() { + // Nothing to do in this case. + return; } - // We can only reach this point with indexes.len() == 1 - // So it's safe to return a vector with a single value + // At this point, `n >= 1` since `indexes.len() >= 1`. if n == 1 { - let value = array[0].clone(); - return vec![value]; + // We can only reach this point if `indexes.len() == 1`, so we only + // need to assign the single value, and then we're done. + debug_assert_eq!(indexes.len(), 1); + values[0] = array[0].clone(); + return; } // We pick a random pivot index: the corresponding element is the pivot value let mut rng = thread_rng(); let pivot_index = rng.gen_range(0, n); - // We partition the array with respect to the pivot value - // The pivot value moves to `array_partition_index` - // Elements strictly smaller than the pivot value have indexes < `array_partition_index` - // Elements greater or equal to the pivot value have indexes > `array_partition_index` + // We partition the array with respect to the pivot value. + // The pivot value moves to `array_partition_index`. + // Elements strictly smaller than the pivot value have indexes < `array_partition_index`. + // Elements greater or equal to the pivot value have indexes > `array_partition_index`. let array_partition_index = array.partition_mut(pivot_index); - // We can use a divide et impera strategy, splitting the indexes we are searching - // in two chunks with respect to array_partition_index - let index_split = indexes.binary_search(&array_partition_index); - let (smaller_indexes, bigger_indexes) = match index_split { - Ok(index_split) => (&indexes[..index_split], &indexes[(index_split + 1)..]), - Err(index_split) => (&indexes[..index_split], &indexes[index_split..]), + // We use a divide-and-conquer strategy, splitting the indexes we are + // searching for (`indexes`) and the corresponding portions of the output + // slice (`values`) into pieces with respect to `array_partition_index`. + let (found_exact, index_split) = match indexes.binary_search(&array_partition_index) { + Ok(index) => (true, index), + Err(index) => (false, index), + }; + let (smaller_indexes, other_indexes) = indexes.split_at_mut(index_split); + let (smaller_values, other_values) = values.split_at_mut(index_split); + let (bigger_indexes, bigger_values) = if found_exact { + other_values[0] = array[array_partition_index].clone(); // Write exactly found value. + (&mut other_indexes[1..], &mut other_values[1..]) + } else { + (other_indexes, other_values) }; - // We are using a recursive search - to look for bigger_indexes in the right - // slice of the array we need to shift the indexes - let bigger_indexes: Vec = bigger_indexes - .into_iter() - .map(|x| x - array_partition_index - 1) - .collect(); - // We search recursively for the values corresponding to strictly smaller indexes - // to the left of partition_index - let smaller_values = _get_many_from_sorted_mut_unchecked( - &mut array.slice_mut(s![..array_partition_index]), + // We search recursively for the values corresponding to strictly smaller + // indexes to the left of `partition_index`. + _get_many_from_sorted_mut_unchecked( + array.slice_mut(s![..array_partition_index]), smaller_indexes, + smaller_values, ); - // We search recursively for the values corresponding to strictly bigger indexes - // to the right of partition_index+1 - let mut bigger_values = _get_many_from_sorted_mut_unchecked( - &mut array.slice_mut(s![(array_partition_index + 1)..]), - &bigger_indexes, + // We search recursively for the values corresponding to strictly bigger + // indexes to the right of `partition_index`. Since only the right portion + // of the array is passed in, the indexes need to be shifted by length of + // the removed portion. + bigger_indexes + .iter_mut() + .for_each(|x| *x -= array_partition_index + 1); + _get_many_from_sorted_mut_unchecked( + array.slice_mut(s![(array_partition_index + 1)..]), + bigger_indexes, + bigger_values, ); - - // We merge the results together, in the correct order - let mut results: Vec; - - results = smaller_values; - if index_split.is_ok() { - // Get the value associated to partition index - results.push(array[array_partition_index].clone()); - } - results.append(&mut bigger_values); - results } From 24ee7105fc541d11c750b8e0b2c3bb3c9785c759 Mon Sep 17 00:00:00 2001 From: Jim Turner Date: Mon, 1 Apr 2019 00:31:11 -0400 Subject: [PATCH 06/19] Call slice_axis_mut instead of slice_mut This has slightly lower overhead. --- src/sort.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/sort.rs b/src/sort.rs index 88e37808..52b27534 100644 --- a/src/sort.rs +++ b/src/sort.rs @@ -1,6 +1,6 @@ use indexmap::IndexMap; use ndarray::prelude::*; -use ndarray::{s, Data, DataMut}; +use ndarray::{Data, DataMut, Slice}; use rand::prelude::*; use rand::thread_rng; use std::iter; @@ -122,11 +122,12 @@ where let pivot_index = rng.gen_range(0, n); let partition_index = self.partition_mut(pivot_index); if i < partition_index { - self.slice_mut(s![..partition_index]).get_from_sorted_mut(i) + self.slice_axis_mut(Axis(0), Slice::from(..partition_index)) + .get_from_sorted_mut(i) } else if i == partition_index { self[i].clone() } else { - self.slice_mut(s![partition_index + 1..]) + self.slice_axis_mut(Axis(0), Slice::from(partition_index + 1..)) .get_from_sorted_mut(i - (partition_index + 1)) } } @@ -277,7 +278,7 @@ fn _get_many_from_sorted_mut_unchecked( // We search recursively for the values corresponding to strictly smaller // indexes to the left of `partition_index`. _get_many_from_sorted_mut_unchecked( - array.slice_mut(s![..array_partition_index]), + array.slice_axis_mut(Axis(0), Slice::from(..array_partition_index)), smaller_indexes, smaller_values, ); @@ -290,7 +291,7 @@ fn _get_many_from_sorted_mut_unchecked( .iter_mut() .for_each(|x| *x -= array_partition_index + 1); _get_many_from_sorted_mut_unchecked( - array.slice_mut(s![(array_partition_index + 1)..]), + array.slice_axis_mut(Axis(0), Slice::from(array_partition_index + 1..)), bigger_indexes, bigger_values, ); From 8739c3bc62fdfcdde742269ae222d619e38238bc Mon Sep 17 00:00:00 2001 From: Jim Turner Date: Mon, 1 Apr 2019 12:02:22 -0400 Subject: [PATCH 07/19] Replace iter::repeat with vec! This is a bit cleaner. --- src/sort.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/sort.rs b/src/sort.rs index 52b27534..3838420e 100644 --- a/src/sort.rs +++ b/src/sort.rs @@ -3,7 +3,6 @@ use ndarray::prelude::*; use ndarray::{Data, DataMut, Slice}; use rand::prelude::*; use rand::thread_rng; -use std::iter; /// Methods for sorting and partitioning 1-D arrays. pub trait Sort1dExt @@ -209,7 +208,7 @@ where // Since `!indexes.is_empty` and indexes must be in-bounds, `array` must be // non-empty. - let mut values: Vec<_> = iter::repeat(array[0].clone()).take(indexes.len()).collect(); + let mut values: Vec<_> = vec![array[0].clone(); indexes.len()]; _get_many_from_sorted_mut_unchecked(array.view_mut(), &mut indexes.to_owned(), &mut values); // We convert the vector to a more search-friendly `IndexMap`. From 88d896f5738689ceb797444988b79385abc9b860 Mon Sep 17 00:00:00 2001 From: Jim Turner Date: Mon, 1 Apr 2019 12:04:48 -0400 Subject: [PATCH 08/19] Fix typo in comment --- src/sort.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sort.rs b/src/sort.rs index 3838420e..c0391cc4 100644 --- a/src/sort.rs +++ b/src/sort.rs @@ -206,8 +206,8 @@ where return IndexMap::new(); } - // Since `!indexes.is_empty` and indexes must be in-bounds, `array` must be - // non-empty. + // Since `!indexes.is_empty()` and indexes must be in-bounds, `array` must + // be non-empty. let mut values: Vec<_> = vec![array[0].clone(); indexes.len()]; _get_many_from_sorted_mut_unchecked(array.view_mut(), &mut indexes.to_owned(), &mut values); From 29d507bd29d0ea03fa479d3ed7e1be9205f373aa Mon Sep 17 00:00:00 2001 From: Jim Turner Date: Mon, 1 Apr 2019 12:16:13 -0400 Subject: [PATCH 09/19] Remove unnecessary type annotation --- src/sort.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sort.rs b/src/sort.rs index c0391cc4..605fb14e 100644 --- a/src/sort.rs +++ b/src/sort.rs @@ -208,7 +208,7 @@ where // Since `!indexes.is_empty()` and indexes must be in-bounds, `array` must // be non-empty. - let mut values: Vec<_> = vec![array[0].clone(); indexes.len()]; + let mut values = vec![array[0].clone(); indexes.len()]; _get_many_from_sorted_mut_unchecked(array.view_mut(), &mut indexes.to_owned(), &mut values); // We convert the vector to a more search-friendly `IndexMap`. From d0879c8248ab5e69b8979cd66c50d357a2d35417 Mon Sep 17 00:00:00 2001 From: Jim Turner Date: Mon, 1 Apr 2019 18:05:42 -0400 Subject: [PATCH 10/19] Simplify quantiles tests For me, the tests are easier to understand when they don't collect into a `Vec`. --- tests/quantile.rs | 108 +++++++++++++++++++++------------------------- 1 file changed, 49 insertions(+), 59 deletions(-) diff --git a/tests/quantile.rs b/tests/quantile.rs index aa0ddbae..dd5eb96c 100644 --- a/tests/quantile.rs +++ b/tests/quantile.rs @@ -299,27 +299,18 @@ fn test_quantiles_mut(xs: Vec) -> bool { n64(0.5), n64(0.5), ]; - let mut checks = vec![]; - checks.push(check_one_interpolation_method_for_quantiles_mut::( - v.clone(), - &quantile_indexes, - )); - checks.push(check_one_interpolation_method_for_quantiles_mut::( - v.clone(), - &quantile_indexes, - )); - checks.push(check_one_interpolation_method_for_quantiles_mut::( - v.clone(), - &quantile_indexes, - )); - checks.push( - check_one_interpolation_method_for_quantiles_mut::(v.clone(), &quantile_indexes), - ); - checks.push(check_one_interpolation_method_for_quantiles_mut::( - v.clone(), - &quantile_indexes, - )); - checks.into_iter().all(|x| x) + let mut correct = true; + correct &= + check_one_interpolation_method_for_quantiles_mut::(v.clone(), &quantile_indexes); + correct &= + check_one_interpolation_method_for_quantiles_mut::(v.clone(), &quantile_indexes); + correct &= + check_one_interpolation_method_for_quantiles_mut::(v.clone(), &quantile_indexes); + correct &= + check_one_interpolation_method_for_quantiles_mut::(v.clone(), &quantile_indexes); + correct &= + check_one_interpolation_method_for_quantiles_mut::(v.clone(), &quantile_indexes); + correct } fn check_one_interpolation_method_for_quantiles_mut>( @@ -332,24 +323,19 @@ fn check_one_interpolation_method_for_quantiles_mut>( bulk_quantiles.is_none() } else { let bulk_quantiles = bulk_quantiles.unwrap(); - - let mut checks = vec![]; - for quantile_index in quantile_indexes.iter() { - let quantile = v.quantile_mut::(*quantile_index).unwrap(); - checks.push(quantile == *bulk_quantiles.get(quantile_index).unwrap()); - } - checks.into_iter().all(|x| x) + quantile_indexes.iter().all(|&quantile_index| { + let quantile = v.quantile_mut::(quantile_index).unwrap(); + quantile == bulk_quantiles[&quantile_index] + }) } } #[quickcheck] -fn test_quantiles_axis_mut(xs: Vec) -> bool { +fn test_quantiles_axis_mut(mut xs: Vec) -> bool { // We want a square matrix let axis_length = (xs.len() as f64).sqrt().floor() as usize; - let xs = &xs[..axis_length.pow(2)]; - let m = Array::from_vec(xs.to_vec()) - .into_shape((axis_length, axis_length)) - .unwrap(); + xs.truncate(axis_length * axis_length); + let m = Array::from_shape_vec((axis_length, axis_length), xs).unwrap(); // Unordered list of quantile indexes to look up, with a duplicate let quantile_indexes = vec![ @@ -365,27 +351,33 @@ fn test_quantiles_axis_mut(xs: Vec) -> bool { ]; // Test out all interpolation methods - let mut checks = vec![]; - checks.push(check_one_interpolation_method_for_quantiles_axis_mut::< - Linear, - >(m.clone(), &quantile_indexes, Axis(0))); - checks.push(check_one_interpolation_method_for_quantiles_axis_mut::< - Higher, - >(m.clone(), &quantile_indexes, Axis(0))); - checks.push( - check_one_interpolation_method_for_quantiles_axis_mut::( - m.clone(), - &quantile_indexes, - Axis(0), - ), + let mut correct = true; + correct &= check_one_interpolation_method_for_quantiles_axis_mut::( + m.clone(), + &quantile_indexes, + Axis(0), ); - checks.push(check_one_interpolation_method_for_quantiles_axis_mut::< - Midpoint, - >(m.clone(), &quantile_indexes, Axis(0))); - checks.push(check_one_interpolation_method_for_quantiles_axis_mut::< - Nearest, - >(m.clone(), &quantile_indexes, Axis(0))); - checks.into_iter().all(|x| x) + correct &= check_one_interpolation_method_for_quantiles_axis_mut::( + m.clone(), + &quantile_indexes, + Axis(0), + ); + correct &= check_one_interpolation_method_for_quantiles_axis_mut::( + m.clone(), + &quantile_indexes, + Axis(0), + ); + correct &= check_one_interpolation_method_for_quantiles_axis_mut::( + m.clone(), + &quantile_indexes, + Axis(0), + ); + correct &= check_one_interpolation_method_for_quantiles_axis_mut::( + m.clone(), + &quantile_indexes, + Axis(0), + ); + correct } fn check_one_interpolation_method_for_quantiles_axis_mut>( @@ -399,11 +391,9 @@ fn check_one_interpolation_method_for_quantiles_axis_mut>( bulk_quantiles.is_none() } else { let bulk_quantiles = bulk_quantiles.unwrap(); - let mut checks = vec![]; - for quantile_index in quantile_indexes.iter() { - let quantile = v.quantile_axis_mut::(axis, *quantile_index).unwrap(); - checks.push(quantile == *bulk_quantiles.get(quantile_index).unwrap()); - } - checks.into_iter().all(|x| x) + quantile_indexes.iter().all(|&quantile_index| { + let quantile = v.quantile_axis_mut::(axis, quantile_index).unwrap(); + quantile == bulk_quantiles[&quantile_index] + }) } } From 54c11be454a4345215ee486b799813755ca170db Mon Sep 17 00:00:00 2001 From: Jim Turner Date: Mon, 1 Apr 2019 18:23:32 -0400 Subject: [PATCH 11/19] Check keys in test_sorted_get_many_mut --- tests/sort.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/tests/sort.rs b/tests/sort.rs index 6bbe17b4..2de79b88 100644 --- a/tests/sort.rs +++ b/tests/sort.rs @@ -60,11 +60,13 @@ fn test_sorted_get_many_mut(mut xs: Vec) -> bool { let mut indexes: Vec = (0..n).into_iter().collect(); indexes.append(&mut (0..n).into_iter().collect()); - let sorted_v: Vec = v - .get_many_from_sorted_mut(&indexes) - .into_iter() - .map(|x| x.1) - .collect(); + let mut sorted_v = Vec::with_capacity(n); + for (i, (key, value)) in v.get_many_from_sorted_mut(&indexes).into_iter().enumerate() { + if i != key { + return false; + } + sorted_v.push(value); + } xs.sort(); println!("Sorted: {:?}. Truth: {:?}", sorted_v, xs); xs == sorted_v From 847fcd5096b9223a18d92f17cd8c5eb68f06875e Mon Sep 17 00:00:00 2001 From: Jim Turner Date: Mon, 1 Apr 2019 18:26:46 -0400 Subject: [PATCH 12/19] Simplify sort tests --- tests/sort.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/tests/sort.rs b/tests/sort.rs index 2de79b88..037e0d8f 100644 --- a/tests/sort.rs +++ b/tests/sort.rs @@ -58,7 +58,7 @@ fn test_sorted_get_many_mut(mut xs: Vec) -> bool { // Insert each index twice, to get a set of indexes with duplicates, not sorted let mut indexes: Vec = (0..n).into_iter().collect(); - indexes.append(&mut (0..n).into_iter().collect()); + indexes.append(&mut (0..n).collect()); let mut sorted_v = Vec::with_capacity(n); for (i, (key, value)) in v.get_many_from_sorted_mut(&indexes).into_iter().enumerate() { @@ -80,10 +80,7 @@ fn test_sorted_get_mut_as_sorting_algorithm(mut xs: Vec) -> bool { true } else { let mut v = Array::from_vec(xs.clone()); - let mut sorted_v = vec![]; - for i in 0..n { - sorted_v.push(v.get_from_sorted_mut(i)) - } + let sorted_v: Vec<_> = (0..n).map(|i| v.get_from_sorted_mut(i)).collect(); xs.sort(); xs == sorted_v } From c6f762a77d5a84350b0951435855bd9969c4415f Mon Sep 17 00:00:00 2001 From: Jim Turner Date: Mon, 1 Apr 2019 18:31:32 -0400 Subject: [PATCH 13/19] Improve sort and quantiles docs --- src/quantile/mod.rs | 22 +++++++++++----------- src/sort.rs | 8 ++++---- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/quantile/mod.rs b/src/quantile/mod.rs index 1554aa77..dd3539f6 100644 --- a/src/quantile/mod.rs +++ b/src/quantile/mod.rs @@ -222,15 +222,15 @@ where S: DataMut, I: Interpolate; - /// A bulk version of [quantile_axis_mut], optimized to retrieve multiple + /// A bulk version of [`quantile_axis_mut`], optimized to retrieve multiple /// quantiles at once. - /// It returns an IndexMap, with (quantile index, quantile over axis) as + /// It returns an `IndexMap`, with (quantile index, quantile over axis) as /// key-value pairs. /// - /// The IndexMap is sorted with respect to quantile indexes in increasing order: + /// The `IndexMap` is sorted with respect to quantile indexes in increasing order: /// this ordering is preserved when you iterate over it (using `iter`/`into_iter`). /// - /// See [quantile_axis_mut] for additional details on quantiles and the algorithm + /// See [`quantile_axis_mut`] for additional details on quantiles and the algorithm /// used to retrieve them. /// /// Returns `None` when the specified axis has length 0. @@ -238,7 +238,7 @@ where /// **Panics** if `axis` is out of bounds or if /// any `q` in `qs` is not between `0.` and `1.` (inclusive). /// - /// [quantile_axis_mut]: ##tymethod.quantile_axis_mut + /// [`quantile_axis_mut`]: #tymethod.quantile_axis_mut fn quantiles_axis_mut( &mut self, axis: Axis, @@ -252,7 +252,7 @@ where /// Return the `q`th quantile of the data along the specified axis, skipping NaN values. /// - /// See [`quantile_axis_mut`](##tymethod.quantile_axis_mut) for details. + /// See [`quantile_axis_mut`](#tymethod.quantile_axis_mut) for details. fn quantile_axis_skipnan_mut(&mut self, axis: Axis, q: N64) -> Option> where D: RemoveAxis, @@ -537,22 +537,22 @@ where S: DataMut, I: Interpolate; - /// A bulk version of [quantile_mut], optimized to retrieve multiple + /// A bulk version of [`quantile_mut`], optimized to retrieve multiple /// quantiles at once. - /// It returns an IndexMap, with (quantile index, quantile value) as + /// It returns an `IndexMap`, with (quantile index, quantile value) as /// key-value pairs. /// - /// The IndexMap is sorted with respect to quantile indexes in increasing order: + /// The `IndexMap` is sorted with respect to quantile indexes in increasing order: /// this ordering is preserved when you iterate over it (using `iter`/`into_iter`). /// /// It returns `None` if the array is empty. /// - /// See [quantile_mut] for additional details on quantiles and the algorithm + /// See [`quantile_mut`] for additional details on quantiles and the algorithm /// used to retrieve them. /// /// **Panics** if any `q` in `qs` is not between `0.` and `1.` (inclusive). /// - /// [quantile_mut]: ##tymethod.quantile_mut + /// [`quantile_mut`]: #tymethod.quantile_mut fn quantiles_mut(&mut self, qs: &[N64]) -> Option> where A: Ord + Clone, diff --git a/src/sort.rs b/src/sort.rs index 605fb14e..590ad156 100644 --- a/src/sort.rs +++ b/src/sort.rs @@ -33,17 +33,17 @@ where A: Ord + Clone, S: DataMut; - /// A bulk version of [get_from_sorted_mut], optimized to retrieve multiple + /// A bulk version of [`get_from_sorted_mut`], optimized to retrieve multiple /// indexes at once. - /// It returns an IndexMap, with indexes as keys and retrieved elements as + /// It returns an `IndexMap`, with indexes as keys and retrieved elements as /// values. - /// The IndexMap is sorted with respect to indexes in increasing order: + /// The `IndexMap` is sorted with respect to indexes in increasing order: /// this ordering is preserved when you iterate over it (using `iter`/`into_iter`). /// /// **Panics** if any element in `indexes` is greater than or equal to `n`, /// where `n` is the length of the array.. /// - /// [get_from_sorted_mut]: ##tymethod.get_from_sorted_mut + /// [`get_from_sorted_mut`]: #tymethod.get_from_sorted_mut fn get_many_from_sorted_mut(&mut self, indexes: &[usize]) -> IndexMap where A: Ord + Clone, From 1685095243e75ce76090138f5c80fa8ca8e00c4e Mon Sep 17 00:00:00 2001 From: Jim Turner Date: Mon, 1 Apr 2019 20:03:26 -0400 Subject: [PATCH 14/19] Make Interpolate::interpolate operate elementwise --- src/quantile/interpolate.rs | 74 +++++++------------------------------ 1 file changed, 13 insertions(+), 61 deletions(-) diff --git a/src/quantile/interpolate.rs b/src/quantile/interpolate.rs index ee8fffdd..a0fe64d4 100644 --- a/src/quantile/interpolate.rs +++ b/src/quantile/interpolate.rs @@ -1,6 +1,4 @@ //! Interpolation strategies. -use ndarray::azip; -use ndarray::prelude::*; use noisy_float::types::N64; use num_traits::{Float, FromPrimitive, NumOps, ToPrimitive}; @@ -45,14 +43,7 @@ pub trait Interpolate { /// **Panics** if `None` is provided for the lower value when it's needed /// or if `None` is provided for the higher value when it's needed. #[doc(hidden)] - fn interpolate( - lower: Option>, - higher: Option>, - q: N64, - len: usize, - ) -> Array - where - D: Dimension; + fn interpolate(lower: Option, higher: Option, q: N64, len: usize) -> T; } /// Select the higher value. @@ -75,12 +66,7 @@ impl Interpolate for Higher { fn needs_higher(_q: N64, _len: usize) -> bool { true } - fn interpolate( - _lower: Option>, - higher: Option>, - _q: N64, - _len: usize, - ) -> Array { + fn interpolate(_lower: Option, higher: Option, _q: N64, _len: usize) -> T { higher.unwrap() } } @@ -92,12 +78,7 @@ impl Interpolate for Lower { fn needs_higher(_q: N64, _len: usize) -> bool { false } - fn interpolate( - lower: Option>, - _higher: Option>, - _q: N64, - _len: usize, - ) -> Array { + fn interpolate(lower: Option, _higher: Option, _q: N64, _len: usize) -> T { lower.unwrap() } } @@ -109,12 +90,7 @@ impl Interpolate for Nearest { fn needs_higher(q: N64, len: usize) -> bool { !>::needs_lower(q, len) } - fn interpolate( - lower: Option>, - higher: Option>, - q: N64, - len: usize, - ) -> Array { + fn interpolate(lower: Option, higher: Option, q: N64, len: usize) -> T { if >::needs_lower(q, len) { lower.unwrap() } else { @@ -133,24 +109,11 @@ where fn needs_higher(_q: N64, _len: usize) -> bool { true } - fn interpolate( - lower: Option>, - higher: Option>, - _q: N64, - _len: usize, - ) -> Array - where - D: Dimension, - { + fn interpolate(lower: Option, higher: Option, _q: N64, _len: usize) -> T { let denom = T::from_u8(2).unwrap(); - let mut lower = lower.unwrap(); + let lower = lower.unwrap(); let higher = higher.unwrap(); - azip!( - mut lower, ref higher in { - *lower = lower.clone() + (higher.clone() - lower.clone()) / denom.clone() - } - ); - lower + lower.clone() + (higher.clone() - lower.clone()) / denom.clone() } } @@ -164,23 +127,12 @@ where fn needs_higher(_q: N64, _len: usize) -> bool { true } - fn interpolate( - lower: Option>, - higher: Option>, - q: N64, - len: usize, - ) -> Array - where - D: Dimension, - { + fn interpolate(lower: Option, higher: Option, q: N64, len: usize) -> T { let fraction = float_quantile_index_fraction(q, len).to_f64().unwrap(); - let mut a = lower.unwrap(); - let b = higher.unwrap(); - azip!(mut a, ref b in { - let a_f64 = a.to_f64().unwrap(); - let b_f64 = b.to_f64().unwrap(); - *a = a.clone() + T::from_f64(fraction * (b_f64 - a_f64)).unwrap(); - }); - a + let lower = lower.unwrap(); + let higher = higher.unwrap(); + let lower_f64 = lower.to_f64().unwrap(); + let higher_f64 = higher.to_f64().unwrap(); + lower.clone() + T::from_f64(fraction * (higher_f64 - lower_f64)).unwrap() } } From e965e85298d21552e32db17b097eaea69d653b75 Mon Sep 17 00:00:00 2001 From: Jim Turner Date: Mon, 1 Apr 2019 20:06:06 -0400 Subject: [PATCH 15/19] Make quantiles_* return Array instead of IndexMap --- src/quantile/mod.rs | 108 ++++++++++++++++++++++++-------------------- tests/quantile.rs | 20 ++++---- 2 files changed, 70 insertions(+), 58 deletions(-) diff --git a/src/quantile/mod.rs b/src/quantile/mod.rs index dd3539f6..82c97afd 100644 --- a/src/quantile/mod.rs +++ b/src/quantile/mod.rs @@ -1,8 +1,8 @@ use self::interpolate::{higher_index, lower_index, Interpolate}; use super::sort::get_many_from_sorted_mut_unchecked; -use indexmap::{IndexMap, IndexSet}; +use indexmap::IndexSet; use ndarray::prelude::*; -use ndarray::{Data, DataMut, RemoveAxis}; +use ndarray::{Data, DataMut, RemoveAxis, Zip}; use noisy_float::types::N64; use std::cmp; use {MaybeNan, MaybeNanExt}; @@ -224,11 +224,9 @@ where /// A bulk version of [`quantile_axis_mut`], optimized to retrieve multiple /// quantiles at once. - /// It returns an `IndexMap`, with (quantile index, quantile over axis) as - /// key-value pairs. /// - /// The `IndexMap` is sorted with respect to quantile indexes in increasing order: - /// this ordering is preserved when you iterate over it (using `iter`/`into_iter`). + /// Returns an `Array`, where subviews along `axis` of the array correspond + /// to the elements of `qs`. /// /// See [`quantile_axis_mut`] for additional details on quantiles and the algorithm /// used to retrieve them. @@ -239,11 +237,29 @@ where /// any `q` in `qs` is not between `0.` and `1.` (inclusive). /// /// [`quantile_axis_mut`]: #tymethod.quantile_axis_mut - fn quantiles_axis_mut( - &mut self, - axis: Axis, - qs: &[N64], - ) -> Option>> + /// + /// # Example + /// + /// ```rust + /// # extern crate ndarray; + /// # extern crate ndarray_stats; + /// # extern crate noisy_float; + /// # + /// use ndarray::{array, Axis}; + /// use ndarray_stats::{QuantileExt, interpolate::Nearest}; + /// use noisy_float::types::n64; + /// + /// # fn main() { + /// let mut data = array![[3, 4, 5], [6, 7, 8]]; + /// let axis = Axis(1); + /// let qs = &[n64(0.3), n64(0.7)]; + /// let quantiles = data.quantiles_axis_mut::(axis, qs).unwrap(); + /// for (&q, quantile) in qs.iter().zip(quantiles.axis_iter(axis)) { + /// assert_eq!(quantile, data.quantile_axis_mut::(axis, q).unwrap()); + /// } + /// # } + /// ``` + fn quantiles_axis_mut(&mut self, axis: Axis, qs: &[N64]) -> Option> where D: RemoveAxis, A: Ord + Clone, @@ -395,11 +411,7 @@ where })) } - fn quantiles_axis_mut( - &mut self, - axis: Axis, - qs: &[N64], - ) -> Option>> + fn quantiles_axis_mut(&mut self, axis: Axis, qs: &[N64]) -> Option> where D: RemoveAxis, A: Ord + Clone, @@ -413,6 +425,12 @@ where return None; } + let mut results_shape = self.raw_dim(); + results_shape[axis.index()] = qs.len(); + if results_shape.size() == 0 { + return Some(Array::from_shape_vec(results_shape, Vec::new()).unwrap()); + } + let mut deduped_qs: Vec = qs.to_vec(); deduped_qs.sort_by(|a, b| a.partial_cmp(b).unwrap()); deduped_qs.dedup(); @@ -431,30 +449,25 @@ where } let searched_indexes: Vec = searched_indexes.into_iter().collect(); - // Retrieve the values corresponding to each index for each slice along the specified axis - // For each 1-dimensional slice along the specified axis we get back an IndexMap - // which can be used to retrieve the desired values using searched_indexes - let values = self.map_axis_mut(axis, |mut x| { - get_many_from_sorted_mut_unchecked(&mut x, &searched_indexes) - }); - - // Combine the retrieved values according to specified interpolation strategy to - // get the desired quantiles - let mut results = IndexMap::new(); - for q in qs { - let lower = if I::needs_lower(*q, axis_len) { - Some(values.map(|x| x[&lower_index(*q, axis_len)].clone())) - } else { - None - }; - let higher = if I::needs_higher(*q, axis_len) { - Some(values.map(|x| x[&higher_index(*q, axis_len)].clone())) - } else { - None - }; - let interpolated = I::interpolate(lower, higher, *q, axis_len); - results.insert(*q, interpolated); - } + let mut results = Array::from_elem(results_shape, self.first().unwrap().clone()); + Zip::from(results.lanes_mut(axis)) + .and(self.lanes_mut(axis)) + .apply(|mut results, mut data| { + let index_map = get_many_from_sorted_mut_unchecked(&mut data, &searched_indexes); + for (result, &q) in results.iter_mut().zip(qs) { + let lower = if I::needs_lower(q, axis_len) { + Some(index_map[&lower_index(q, axis_len)].clone()) + } else { + None + }; + let higher = if I::needs_higher(q, axis_len) { + Some(index_map[&higher_index(q, axis_len)].clone()) + } else { + None + }; + *result = I::interpolate(lower, higher, q, axis_len); + } + }); Some(results) } @@ -466,7 +479,7 @@ where I: Interpolate, { self.quantiles_axis_mut::(axis, &[q]) - .map(|x| x.into_iter().next().unwrap().1) + .map(|a| a.index_axis_move(axis, 0)) } fn quantile_axis_skipnan_mut(&mut self, axis: Axis, q: N64) -> Option> @@ -539,13 +552,11 @@ where /// A bulk version of [`quantile_mut`], optimized to retrieve multiple /// quantiles at once. - /// It returns an `IndexMap`, with (quantile index, quantile value) as - /// key-value pairs. /// - /// The `IndexMap` is sorted with respect to quantile indexes in increasing order: - /// this ordering is preserved when you iterate over it (using `iter`/`into_iter`). + /// Returns an `Array`, where the elements of the array correspond to the + /// elements of `qs`. /// - /// It returns `None` if the array is empty. + /// Returns `None` if the array is empty. /// /// See [`quantile_mut`] for additional details on quantiles and the algorithm /// used to retrieve them. @@ -553,7 +564,7 @@ where /// **Panics** if any `q` in `qs` is not between `0.` and `1.` (inclusive). /// /// [`quantile_mut`]: #tymethod.quantile_mut - fn quantiles_mut(&mut self, qs: &[N64]) -> Option> + fn quantiles_mut(&mut self, qs: &[N64]) -> Option> where A: Ord + Clone, S: DataMut, @@ -574,14 +585,13 @@ where .map(|v| v.into_scalar()) } - fn quantiles_mut(&mut self, qs: &[N64]) -> Option> + fn quantiles_mut(&mut self, qs: &[N64]) -> Option> where A: Ord + Clone, S: DataMut, I: Interpolate, { self.quantiles_axis_mut::(Axis(0), qs) - .map(|v| v.into_iter().map(|x| (x.0, x.1.into_scalar())).collect()) } } diff --git a/tests/quantile.rs b/tests/quantile.rs index dd5eb96c..1b98e1a6 100644 --- a/tests/quantile.rs +++ b/tests/quantile.rs @@ -1,3 +1,4 @@ +extern crate itertools; extern crate ndarray; extern crate ndarray_stats; extern crate noisy_float; @@ -5,6 +6,7 @@ extern crate noisy_float; extern crate quickcheck; extern crate quickcheck_macros; +use itertools::izip; use ndarray::array; use ndarray::prelude::*; use ndarray_stats::{ @@ -317,15 +319,14 @@ fn check_one_interpolation_method_for_quantiles_mut>( mut v: Array1, quantile_indexes: &[N64], ) -> bool { - let bulk_quantiles = v.quantiles_mut::(&quantile_indexes); + let bulk_quantiles = v.clone().quantiles_mut::(&quantile_indexes); if v.len() == 0 { bulk_quantiles.is_none() } else { let bulk_quantiles = bulk_quantiles.unwrap(); - quantile_indexes.iter().all(|&quantile_index| { - let quantile = v.quantile_mut::(quantile_index).unwrap(); - quantile == bulk_quantiles[&quantile_index] + izip!(quantile_indexes, &bulk_quantiles).all(|(&quantile_index, &quantile)| { + quantile == v.quantile_mut::(quantile_index).unwrap() }) } } @@ -385,15 +386,16 @@ fn check_one_interpolation_method_for_quantiles_axis_mut>( quantile_indexes: &[N64], axis: Axis, ) -> bool { - let bulk_quantiles = v.quantiles_axis_mut::(axis, &quantile_indexes); + let bulk_quantiles = v.clone().quantiles_axis_mut::(axis, &quantile_indexes); if v.len() == 0 { bulk_quantiles.is_none() } else { let bulk_quantiles = bulk_quantiles.unwrap(); - quantile_indexes.iter().all(|&quantile_index| { - let quantile = v.quantile_axis_mut::(axis, quantile_index).unwrap(); - quantile == bulk_quantiles[&quantile_index] - }) + izip!(quantile_indexes, bulk_quantiles.axis_iter(axis)).all( + |(&quantile_index, quantile)| { + quantile == v.quantile_axis_mut::(axis, quantile_index).unwrap() + }, + ) } } From cfc408f7bf3668e1de126000fc28645e1ae7cd33 Mon Sep 17 00:00:00 2001 From: Jim Turner Date: Mon, 1 Apr 2019 21:27:13 -0400 Subject: [PATCH 16/19] Add interpolate parameter to quantile* This has a few advantages: * It's now possible to save the interpolation strategy in a variable and easily re-use it. * We can now freely add more type parameters to the `quantile*` methods as needed without making them more difficult to call. * We now have the flexibility to add more advanced interpolation strategies in the future (e.g. one that wraps a closure). * Calling the `quantile*` methods is now slightly more compact because a turbofish isn't necessary. --- src/histogram/strategies.rs | 4 +-- src/quantile/mod.rs | 36 ++++++++++---------- tests/quantile.rs | 67 +++++++++++++++++++++---------------- 3 files changed, 59 insertions(+), 48 deletions(-) diff --git a/src/histogram/strategies.rs b/src/histogram/strategies.rs index 9f933df0..fa90c385 100644 --- a/src/histogram/strategies.rs +++ b/src/histogram/strategies.rs @@ -308,8 +308,8 @@ where let n_points = a.len(); let mut a_copy = a.to_owned(); - let first_quartile = a_copy.quantile_mut::(n64(0.25)).unwrap(); - let third_quartile = a_copy.quantile_mut::(n64(0.75)).unwrap(); + let first_quartile = a_copy.quantile_mut(n64(0.25), &Nearest).unwrap(); + let third_quartile = a_copy.quantile_mut(n64(0.75), &Nearest).unwrap(); let iqr = third_quartile - first_quartile; let bin_width = FreedmanDiaconis::compute_bin_width(n_points, iqr); diff --git a/src/quantile/mod.rs b/src/quantile/mod.rs index 82c97afd..c880c73b 100644 --- a/src/quantile/mod.rs +++ b/src/quantile/mod.rs @@ -191,7 +191,7 @@ where /// in increasing order. /// If `(N-1)q` is not an integer the desired quantile lies between /// two data points: we return the lower, nearest, higher or interpolated - /// value depending on the type `Interpolate` bound `I`. + /// value depending on the `interpolate` strategy. /// /// Some examples: /// - `q=0.` returns the minimum along each 1-dimensional lane; @@ -215,7 +215,7 @@ where /// /// **Panics** if `axis` is out of bounds or if /// `q` is not between `0.` and `1.` (inclusive). - fn quantile_axis_mut(&mut self, axis: Axis, q: N64) -> Option> + fn quantile_axis_mut(&mut self, axis: Axis, q: N64, interpolate: &I) -> Option> where D: RemoveAxis, A: Ord + Clone, @@ -253,13 +253,13 @@ where /// let mut data = array![[3, 4, 5], [6, 7, 8]]; /// let axis = Axis(1); /// let qs = &[n64(0.3), n64(0.7)]; - /// let quantiles = data.quantiles_axis_mut::(axis, qs).unwrap(); + /// let quantiles = data.quantiles_axis_mut(axis, qs, &Nearest).unwrap(); /// for (&q, quantile) in qs.iter().zip(quantiles.axis_iter(axis)) { - /// assert_eq!(quantile, data.quantile_axis_mut::(axis, q).unwrap()); + /// assert_eq!(quantile, data.quantile_axis_mut(axis, q, &Nearest).unwrap()); /// } /// # } /// ``` - fn quantiles_axis_mut(&mut self, axis: Axis, qs: &[N64]) -> Option> + fn quantiles_axis_mut(&mut self, axis: Axis, qs: &[N64], interpolate: &I) -> Option> where D: RemoveAxis, A: Ord + Clone, @@ -269,7 +269,7 @@ where /// Return the `q`th quantile of the data along the specified axis, skipping NaN values. /// /// See [`quantile_axis_mut`](#tymethod.quantile_axis_mut) for details. - fn quantile_axis_skipnan_mut(&mut self, axis: Axis, q: N64) -> Option> + fn quantile_axis_skipnan_mut(&mut self, axis: Axis, q: N64, interpolate: &I) -> Option> where D: RemoveAxis, A: MaybeNan, @@ -411,7 +411,7 @@ where })) } - fn quantiles_axis_mut(&mut self, axis: Axis, qs: &[N64]) -> Option> + fn quantiles_axis_mut(&mut self, axis: Axis, qs: &[N64], _interpolate: &I) -> Option> where D: RemoveAxis, A: Ord + Clone, @@ -471,18 +471,18 @@ where Some(results) } - fn quantile_axis_mut(&mut self, axis: Axis, q: N64) -> Option> + fn quantile_axis_mut(&mut self, axis: Axis, q: N64, interpolate: &I) -> Option> where D: RemoveAxis, A: Ord + Clone, S: DataMut, I: Interpolate, { - self.quantiles_axis_mut::(axis, &[q]) + self.quantiles_axis_mut(axis, &[q], interpolate) .map(|a| a.index_axis_move(axis, 0)) } - fn quantile_axis_skipnan_mut(&mut self, axis: Axis, q: N64) -> Option> + fn quantile_axis_skipnan_mut(&mut self, axis: Axis, q: N64, interpolate: &I) -> Option> where D: RemoveAxis, A: MaybeNan, @@ -500,7 +500,7 @@ where } else { Some( not_nan - .quantile_axis_mut::(Axis(0), q) + .quantile_axis_mut::(Axis(0), q, interpolate) .unwrap() .into_scalar(), ) @@ -523,7 +523,7 @@ where /// in increasing order. /// If `(N-1)q` is not an integer the desired quantile lies between /// two data points: we return the lower, nearest, higher or interpolated - /// value depending on the type `Interpolate` bound `I`. + /// value depending on the `interpolate` strategy. /// /// Some examples: /// - `q=0.` returns the minimum; @@ -544,7 +544,7 @@ where /// Returns `None` if the array is empty. /// /// **Panics** if `q` is not between `0.` and `1.` (inclusive). - fn quantile_mut(&mut self, q: N64) -> Option + fn quantile_mut(&mut self, q: N64, interpolate: &I) -> Option where A: Ord + Clone, S: DataMut, @@ -564,7 +564,7 @@ where /// **Panics** if any `q` in `qs` is not between `0.` and `1.` (inclusive). /// /// [`quantile_mut`]: #tymethod.quantile_mut - fn quantiles_mut(&mut self, qs: &[N64]) -> Option> + fn quantiles_mut(&mut self, qs: &[N64], interpolate: &I) -> Option> where A: Ord + Clone, S: DataMut, @@ -575,23 +575,23 @@ impl Quantile1dExt for ArrayBase where S: Data, { - fn quantile_mut(&mut self, q: N64) -> Option + fn quantile_mut(&mut self, q: N64, interpolate: &I) -> Option where A: Ord + Clone, S: DataMut, I: Interpolate, { - self.quantile_axis_mut::(Axis(0), q) + self.quantile_axis_mut::(Axis(0), q, interpolate) .map(|v| v.into_scalar()) } - fn quantiles_mut(&mut self, qs: &[N64]) -> Option> + fn quantiles_mut(&mut self, qs: &[N64], interpolate: &I) -> Option> where A: Ord + Clone, S: DataMut, I: Interpolate, { - self.quantiles_axis_mut::(Axis(0), qs) + self.quantiles_axis_mut(Axis(0), qs, interpolate) } } diff --git a/tests/quantile.rs b/tests/quantile.rs index 1b98e1a6..2957afe7 100644 --- a/tests/quantile.rs +++ b/tests/quantile.rs @@ -182,41 +182,41 @@ fn test_max_skipnan_all_nan() { #[test] fn test_quantile_axis_mut_with_odd_axis_length() { let mut a = arr2(&[[1, 3, 2, 10], [2, 4, 3, 11], [3, 5, 6, 12]]); - let p = a.quantile_axis_mut::(Axis(0), n64(0.5)).unwrap(); + let p = a.quantile_axis_mut(Axis(0), n64(0.5), &Lower).unwrap(); assert!(p == a.index_axis(Axis(0), 1)); } #[test] fn test_quantile_axis_mut_with_zero_axis_length() { let mut a = Array2::::zeros((5, 0)); - assert!(a.quantile_axis_mut::(Axis(1), n64(0.5)).is_none()); + assert!(a.quantile_axis_mut(Axis(1), n64(0.5), &Lower).is_none()); } #[test] fn test_quantile_axis_mut_with_empty_array() { let mut a = Array2::::zeros((5, 0)); - let p = a.quantile_axis_mut::(Axis(0), n64(0.5)).unwrap(); + let p = a.quantile_axis_mut(Axis(0), n64(0.5), &Lower).unwrap(); assert_eq!(p.shape(), &[0]); } #[test] fn test_quantile_axis_mut_with_even_axis_length() { let mut b = arr2(&[[1, 3, 2, 10], [2, 4, 3, 11], [3, 5, 6, 12], [4, 6, 7, 13]]); - let q = b.quantile_axis_mut::(Axis(0), n64(0.5)).unwrap(); + let q = b.quantile_axis_mut(Axis(0), n64(0.5), &Lower).unwrap(); assert!(q == b.index_axis(Axis(0), 1)); } #[test] fn test_quantile_axis_mut_to_get_minimum() { let mut b = arr2(&[[1, 3, 22, 10]]); - let q = b.quantile_axis_mut::(Axis(1), n64(0.)).unwrap(); + let q = b.quantile_axis_mut(Axis(1), n64(0.), &Lower).unwrap(); assert!(q == arr1(&[1])); } #[test] fn test_quantile_axis_mut_to_get_maximum() { let mut b = arr1(&[1, 3, 22, 10]); - let q = b.quantile_axis_mut::(Axis(0), n64(1.)).unwrap(); + let q = b.quantile_axis_mut(Axis(0), n64(1.), &Lower).unwrap(); assert!(q == arr0(22)); } @@ -224,7 +224,7 @@ fn test_quantile_axis_mut_to_get_maximum() { fn test_quantile_axis_skipnan_mut_higher_opt_i32() { let mut a = arr2(&[[Some(4), Some(2), None, Some(1), Some(5)], [None; 5]]); let q = a - .quantile_axis_skipnan_mut::(Axis(1), n64(0.6)) + .quantile_axis_skipnan_mut(Axis(1), n64(0.6), &Higher) .unwrap(); assert_eq!(q.shape(), &[2]); assert_eq!(q[0], Some(4)); @@ -235,7 +235,7 @@ fn test_quantile_axis_skipnan_mut_higher_opt_i32() { fn test_quantile_axis_skipnan_mut_nearest_opt_i32() { let mut a = arr2(&[[Some(4), Some(2), None, Some(1), Some(5)], [None; 5]]); let q = a - .quantile_axis_skipnan_mut::(Axis(1), n64(0.6)) + .quantile_axis_skipnan_mut(Axis(1), n64(0.6), &Nearest) .unwrap(); assert_eq!(q.shape(), &[2]); assert_eq!(q[0], Some(4)); @@ -246,7 +246,7 @@ fn test_quantile_axis_skipnan_mut_nearest_opt_i32() { fn test_quantile_axis_skipnan_mut_midpoint_opt_i32() { let mut a = arr2(&[[Some(4), Some(2), None, Some(1), Some(5)], [None; 5]]); let q = a - .quantile_axis_skipnan_mut::(Axis(1), n64(0.6)) + .quantile_axis_skipnan_mut(Axis(1), n64(0.6), &Midpoint) .unwrap(); assert_eq!(q.shape(), &[2]); assert_eq!(q[0], Some(3)); @@ -257,7 +257,7 @@ fn test_quantile_axis_skipnan_mut_midpoint_opt_i32() { fn test_quantile_axis_skipnan_mut_linear_f64() { let mut a = arr2(&[[1., 2., ::std::f64::NAN, 3.], [::std::f64::NAN; 4]]); let q = a - .quantile_axis_skipnan_mut::(Axis(1), n64(0.75)) + .quantile_axis_skipnan_mut(Axis(1), n64(0.75), &Linear) .unwrap(); assert_eq!(q.shape(), &[2]); assert!((q[0] - 2.5).abs() < 1e-12); @@ -268,7 +268,7 @@ fn test_quantile_axis_skipnan_mut_linear_f64() { fn test_quantile_axis_skipnan_mut_linear_opt_i32() { let mut a = arr2(&[[Some(2), Some(4), None, Some(1)], [None; 4]]); let q = a - .quantile_axis_skipnan_mut::(Axis(1), n64(0.75)) + .quantile_axis_skipnan_mut(Axis(1), n64(0.75), &Linear) .unwrap(); assert_eq!(q.shape(), &[2]); assert_eq!(q[0], Some(3)); @@ -280,7 +280,7 @@ fn test_midpoint_overflow() { // Regression test // This triggered an overflow panic with a naive Midpoint implementation: (a+b)/2 let mut a: Array1 = array![129, 130, 130, 131]; - let median = a.quantile_mut::(n64(0.5)).unwrap(); + let median = a.quantile_mut(n64(0.5), &Midpoint).unwrap(); let expected_median = 130; assert_eq!(median, expected_median); } @@ -303,30 +303,31 @@ fn test_quantiles_mut(xs: Vec) -> bool { ]; let mut correct = true; correct &= - check_one_interpolation_method_for_quantiles_mut::(v.clone(), &quantile_indexes); + check_one_interpolation_method_for_quantiles_mut(v.clone(), &quantile_indexes, &Linear); correct &= - check_one_interpolation_method_for_quantiles_mut::(v.clone(), &quantile_indexes); + check_one_interpolation_method_for_quantiles_mut(v.clone(), &quantile_indexes, &Higher); correct &= - check_one_interpolation_method_for_quantiles_mut::(v.clone(), &quantile_indexes); + check_one_interpolation_method_for_quantiles_mut(v.clone(), &quantile_indexes, &Lower); correct &= - check_one_interpolation_method_for_quantiles_mut::(v.clone(), &quantile_indexes); + check_one_interpolation_method_for_quantiles_mut(v.clone(), &quantile_indexes, &Midpoint); correct &= - check_one_interpolation_method_for_quantiles_mut::(v.clone(), &quantile_indexes); + check_one_interpolation_method_for_quantiles_mut(v.clone(), &quantile_indexes, &Nearest); correct } -fn check_one_interpolation_method_for_quantiles_mut>( +fn check_one_interpolation_method_for_quantiles_mut( mut v: Array1, quantile_indexes: &[N64], + interpolate: &impl Interpolate, ) -> bool { - let bulk_quantiles = v.clone().quantiles_mut::(&quantile_indexes); + let bulk_quantiles = v.clone().quantiles_mut(&quantile_indexes, interpolate); if v.len() == 0 { bulk_quantiles.is_none() } else { let bulk_quantiles = bulk_quantiles.unwrap(); izip!(quantile_indexes, &bulk_quantiles).all(|(&quantile_index, &quantile)| { - quantile == v.quantile_mut::(quantile_index).unwrap() + quantile == v.quantile_mut(quantile_index, interpolate).unwrap() }) } } @@ -353,40 +354,48 @@ fn test_quantiles_axis_mut(mut xs: Vec) -> bool { // Test out all interpolation methods let mut correct = true; - correct &= check_one_interpolation_method_for_quantiles_axis_mut::( + correct &= check_one_interpolation_method_for_quantiles_axis_mut( m.clone(), &quantile_indexes, Axis(0), + &Linear, ); - correct &= check_one_interpolation_method_for_quantiles_axis_mut::( + correct &= check_one_interpolation_method_for_quantiles_axis_mut( m.clone(), &quantile_indexes, Axis(0), + &Higher, ); - correct &= check_one_interpolation_method_for_quantiles_axis_mut::( + correct &= check_one_interpolation_method_for_quantiles_axis_mut( m.clone(), &quantile_indexes, Axis(0), + &Lower, ); - correct &= check_one_interpolation_method_for_quantiles_axis_mut::( + correct &= check_one_interpolation_method_for_quantiles_axis_mut( m.clone(), &quantile_indexes, Axis(0), + &Midpoint, ); - correct &= check_one_interpolation_method_for_quantiles_axis_mut::( + correct &= check_one_interpolation_method_for_quantiles_axis_mut( m.clone(), &quantile_indexes, Axis(0), + &Nearest, ); correct } -fn check_one_interpolation_method_for_quantiles_axis_mut>( +fn check_one_interpolation_method_for_quantiles_axis_mut( mut v: Array2, quantile_indexes: &[N64], axis: Axis, + interpolate: &impl Interpolate, ) -> bool { - let bulk_quantiles = v.clone().quantiles_axis_mut::(axis, &quantile_indexes); + let bulk_quantiles = v + .clone() + .quantiles_axis_mut(axis, &quantile_indexes, interpolate); if v.len() == 0 { bulk_quantiles.is_none() @@ -394,7 +403,9 @@ fn check_one_interpolation_method_for_quantiles_axis_mut>( let bulk_quantiles = bulk_quantiles.unwrap(); izip!(quantile_indexes, bulk_quantiles.axis_iter(axis)).all( |(&quantile_index, quantile)| { - quantile == v.quantile_axis_mut::(axis, quantile_index).unwrap() + quantile + == v.quantile_axis_mut(axis, quantile_index, interpolate) + .unwrap() }, ) } From b5d8a085a1d5bd348b44cd65213b66db45458c49 Mon Sep 17 00:00:00 2001 From: Jim Turner Date: Mon, 1 Apr 2019 21:39:45 -0400 Subject: [PATCH 17/19] Make get_many_from_sorted_mut take array of indexes This is slightly more versatile because `ArrayBase` allows arbitrary strides. --- src/sort.rs | 8 +++++--- tests/sort.rs | 6 +++++- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/sort.rs b/src/sort.rs index 590ad156..5a0b5d3b 100644 --- a/src/sort.rs +++ b/src/sort.rs @@ -44,10 +44,11 @@ where /// where `n` is the length of the array.. /// /// [`get_from_sorted_mut`]: #tymethod.get_from_sorted_mut - fn get_many_from_sorted_mut(&mut self, indexes: &[usize]) -> IndexMap + fn get_many_from_sorted_mut(&mut self, indexes: &ArrayBase) -> IndexMap where A: Ord + Clone, - S: DataMut; + S: DataMut, + S2: Data; /// Partitions the array in increasing order based on the value initially /// located at `pivot_index` and returns the new index of the value. @@ -132,10 +133,11 @@ where } } - fn get_many_from_sorted_mut(&mut self, indexes: &[usize]) -> IndexMap + fn get_many_from_sorted_mut(&mut self, indexes: &ArrayBase) -> IndexMap where A: Ord + Clone, S: DataMut, + S2: Data, { let mut deduped_indexes: Vec = indexes.to_vec(); deduped_indexes.sort_unstable(); diff --git a/tests/sort.rs b/tests/sort.rs index 037e0d8f..2d1df06c 100644 --- a/tests/sort.rs +++ b/tests/sort.rs @@ -61,7 +61,11 @@ fn test_sorted_get_many_mut(mut xs: Vec) -> bool { indexes.append(&mut (0..n).collect()); let mut sorted_v = Vec::with_capacity(n); - for (i, (key, value)) in v.get_many_from_sorted_mut(&indexes).into_iter().enumerate() { + for (i, (key, value)) in v + .get_many_from_sorted_mut(&Array::from(indexes)) + .into_iter() + .enumerate() + { if i != key { return false; } From 00a21c06fa2e02c2c9eb6e8fd69aab4e8973c678 Mon Sep 17 00:00:00 2001 From: Jim Turner Date: Mon, 1 Apr 2019 22:08:25 -0400 Subject: [PATCH 18/19] Make quantiles* take array instead of slice --- src/quantile/mod.rs | 174 +++++++++++++++++++++++++++++--------------- tests/quantile.rs | 57 +++++++++------ 2 files changed, 152 insertions(+), 79 deletions(-) diff --git a/src/quantile/mod.rs b/src/quantile/mod.rs index c880c73b..7d59037a 100644 --- a/src/quantile/mod.rs +++ b/src/quantile/mod.rs @@ -215,7 +215,12 @@ where /// /// **Panics** if `axis` is out of bounds or if /// `q` is not between `0.` and `1.` (inclusive). - fn quantile_axis_mut(&mut self, axis: Axis, q: N64, interpolate: &I) -> Option> + fn quantile_axis_mut( + &mut self, + axis: Axis, + q: N64, + interpolate: &I, + ) -> Option> where D: RemoveAxis, A: Ord + Clone, @@ -245,7 +250,7 @@ where /// # extern crate ndarray_stats; /// # extern crate noisy_float; /// # - /// use ndarray::{array, Axis}; + /// use ndarray::{array, aview1, Axis}; /// use ndarray_stats::{QuantileExt, interpolate::Nearest}; /// use noisy_float::types::n64; /// @@ -253,23 +258,34 @@ where /// let mut data = array![[3, 4, 5], [6, 7, 8]]; /// let axis = Axis(1); /// let qs = &[n64(0.3), n64(0.7)]; - /// let quantiles = data.quantiles_axis_mut(axis, qs, &Nearest).unwrap(); + /// let quantiles = data.quantiles_axis_mut(axis, &aview1(qs), &Nearest).unwrap(); /// for (&q, quantile) in qs.iter().zip(quantiles.axis_iter(axis)) { /// assert_eq!(quantile, data.quantile_axis_mut(axis, q, &Nearest).unwrap()); /// } /// # } /// ``` - fn quantiles_axis_mut(&mut self, axis: Axis, qs: &[N64], interpolate: &I) -> Option> + fn quantiles_axis_mut( + &mut self, + axis: Axis, + qs: &ArrayBase, + interpolate: &I, + ) -> Option> where D: RemoveAxis, A: Ord + Clone, S: DataMut, + S2: Data, I: Interpolate; /// Return the `q`th quantile of the data along the specified axis, skipping NaN values. /// /// See [`quantile_axis_mut`](#tymethod.quantile_axis_mut) for details. - fn quantile_axis_skipnan_mut(&mut self, axis: Axis, q: N64, interpolate: &I) -> Option> + fn quantile_axis_skipnan_mut( + &mut self, + axis: Axis, + q: N64, + interpolate: &I, + ) -> Option> where D: RemoveAxis, A: MaybeNan, @@ -411,78 +427,110 @@ where })) } - fn quantiles_axis_mut(&mut self, axis: Axis, qs: &[N64], _interpolate: &I) -> Option> + fn quantiles_axis_mut( + &mut self, + axis: Axis, + qs: &ArrayBase, + interpolate: &I, + ) -> Option> where D: RemoveAxis, A: Ord + Clone, S: DataMut, + S2: Data, I: Interpolate, { - assert!(qs.iter().all(|x| (*x >= 0.) && (*x <= 1.))); - - let axis_len = self.len_of(axis); - if axis_len == 0 { - return None; - } - - let mut results_shape = self.raw_dim(); - results_shape[axis.index()] = qs.len(); - if results_shape.size() == 0 { - return Some(Array::from_shape_vec(results_shape, Vec::new()).unwrap()); - } + // Minimize number of type parameters to avoid monomorphization bloat. + fn quantiles_axis_mut( + mut data: ArrayViewMut, + axis: Axis, + qs: ArrayView1, + _interpolate: &I, + ) -> Option> + where + D: RemoveAxis, + A: Ord + Clone, + I: Interpolate, + { + assert!(qs.iter().all(|x| (*x >= 0.) && (*x <= 1.))); + + let axis_len = data.len_of(axis); + if axis_len == 0 { + return None; + } - let mut deduped_qs: Vec = qs.to_vec(); - deduped_qs.sort_by(|a, b| a.partial_cmp(b).unwrap()); - deduped_qs.dedup(); - - // IndexSet preserves insertion order: - // - indexes will stay sorted; - // - we avoid index duplication. - let mut searched_indexes = IndexSet::new(); - for q in deduped_qs.iter() { - if I::needs_lower(*q, axis_len) { - searched_indexes.insert(lower_index(*q, axis_len)); + let mut results_shape = data.raw_dim(); + results_shape[axis.index()] = qs.len(); + if results_shape.size() == 0 { + return Some(Array::from_shape_vec(results_shape, Vec::new()).unwrap()); } - if I::needs_higher(*q, axis_len) { - searched_indexes.insert(higher_index(*q, axis_len)); + + let mut deduped_qs: Vec = qs.to_vec(); + deduped_qs.sort_by(|a, b| a.partial_cmp(b).unwrap()); + deduped_qs.dedup(); + + // IndexSet preserves insertion order: + // - indexes will stay sorted; + // - we avoid index duplication. + let mut searched_indexes = IndexSet::new(); + for q in deduped_qs.iter() { + if I::needs_lower(*q, axis_len) { + searched_indexes.insert(lower_index(*q, axis_len)); + } + if I::needs_higher(*q, axis_len) { + searched_indexes.insert(higher_index(*q, axis_len)); + } } + let searched_indexes: Vec = searched_indexes.into_iter().collect(); + + let mut results = Array::from_elem(results_shape, data.first().unwrap().clone()); + Zip::from(results.lanes_mut(axis)) + .and(data.lanes_mut(axis)) + .apply(|mut results, mut data| { + let index_map = + get_many_from_sorted_mut_unchecked(&mut data, &searched_indexes); + for (result, &q) in results.iter_mut().zip(qs) { + let lower = if I::needs_lower(q, axis_len) { + Some(index_map[&lower_index(q, axis_len)].clone()) + } else { + None + }; + let higher = if I::needs_higher(q, axis_len) { + Some(index_map[&higher_index(q, axis_len)].clone()) + } else { + None + }; + *result = I::interpolate(lower, higher, q, axis_len); + } + }); + Some(results) } - let searched_indexes: Vec = searched_indexes.into_iter().collect(); - - let mut results = Array::from_elem(results_shape, self.first().unwrap().clone()); - Zip::from(results.lanes_mut(axis)) - .and(self.lanes_mut(axis)) - .apply(|mut results, mut data| { - let index_map = get_many_from_sorted_mut_unchecked(&mut data, &searched_indexes); - for (result, &q) in results.iter_mut().zip(qs) { - let lower = if I::needs_lower(q, axis_len) { - Some(index_map[&lower_index(q, axis_len)].clone()) - } else { - None - }; - let higher = if I::needs_higher(q, axis_len) { - Some(index_map[&higher_index(q, axis_len)].clone()) - } else { - None - }; - *result = I::interpolate(lower, higher, q, axis_len); - } - }); - Some(results) + + quantiles_axis_mut(self.view_mut(), axis, qs.view(), interpolate) } - fn quantile_axis_mut(&mut self, axis: Axis, q: N64, interpolate: &I) -> Option> + fn quantile_axis_mut( + &mut self, + axis: Axis, + q: N64, + interpolate: &I, + ) -> Option> where D: RemoveAxis, A: Ord + Clone, S: DataMut, I: Interpolate, { - self.quantiles_axis_mut(axis, &[q], interpolate) + self.quantiles_axis_mut(axis, &aview1(&[q]), interpolate) .map(|a| a.index_axis_move(axis, 0)) } - fn quantile_axis_skipnan_mut(&mut self, axis: Axis, q: N64, interpolate: &I) -> Option> + fn quantile_axis_skipnan_mut( + &mut self, + axis: Axis, + q: N64, + interpolate: &I, + ) -> Option> where D: RemoveAxis, A: MaybeNan, @@ -564,10 +612,15 @@ where /// **Panics** if any `q` in `qs` is not between `0.` and `1.` (inclusive). /// /// [`quantile_mut`]: #tymethod.quantile_mut - fn quantiles_mut(&mut self, qs: &[N64], interpolate: &I) -> Option> + fn quantiles_mut( + &mut self, + qs: &ArrayBase, + interpolate: &I, + ) -> Option> where A: Ord + Clone, S: DataMut, + S2: Data, I: Interpolate; } @@ -585,10 +638,15 @@ where .map(|v| v.into_scalar()) } - fn quantiles_mut(&mut self, qs: &[N64], interpolate: &I) -> Option> + fn quantiles_mut( + &mut self, + qs: &ArrayBase, + interpolate: &I, + ) -> Option> where A: Ord + Clone, S: DataMut, + S2: Data, I: Interpolate, { self.quantiles_axis_mut(Axis(0), qs, interpolate) diff --git a/tests/quantile.rs b/tests/quantile.rs index 2957afe7..f880bab9 100644 --- a/tests/quantile.rs +++ b/tests/quantile.rs @@ -290,7 +290,7 @@ fn test_quantiles_mut(xs: Vec) -> bool { let v = Array::from_vec(xs.clone()); // Unordered list of quantile indexes to look up, with a duplicate - let quantile_indexes = vec![ + let quantile_indexes = Array::from(vec![ n64(0.75), n64(0.90), n64(0.95), @@ -300,24 +300,39 @@ fn test_quantiles_mut(xs: Vec) -> bool { n64(0.25), n64(0.5), n64(0.5), - ]; + ]); let mut correct = true; - correct &= - check_one_interpolation_method_for_quantiles_mut(v.clone(), &quantile_indexes, &Linear); - correct &= - check_one_interpolation_method_for_quantiles_mut(v.clone(), &quantile_indexes, &Higher); - correct &= - check_one_interpolation_method_for_quantiles_mut(v.clone(), &quantile_indexes, &Lower); - correct &= - check_one_interpolation_method_for_quantiles_mut(v.clone(), &quantile_indexes, &Midpoint); - correct &= - check_one_interpolation_method_for_quantiles_mut(v.clone(), &quantile_indexes, &Nearest); + correct &= check_one_interpolation_method_for_quantiles_mut( + v.clone(), + quantile_indexes.view(), + &Linear, + ); + correct &= check_one_interpolation_method_for_quantiles_mut( + v.clone(), + quantile_indexes.view(), + &Higher, + ); + correct &= check_one_interpolation_method_for_quantiles_mut( + v.clone(), + quantile_indexes.view(), + &Lower, + ); + correct &= check_one_interpolation_method_for_quantiles_mut( + v.clone(), + quantile_indexes.view(), + &Midpoint, + ); + correct &= check_one_interpolation_method_for_quantiles_mut( + v.clone(), + quantile_indexes.view(), + &Nearest, + ); correct } fn check_one_interpolation_method_for_quantiles_mut( mut v: Array1, - quantile_indexes: &[N64], + quantile_indexes: ArrayView1, interpolate: &impl Interpolate, ) -> bool { let bulk_quantiles = v.clone().quantiles_mut(&quantile_indexes, interpolate); @@ -340,7 +355,7 @@ fn test_quantiles_axis_mut(mut xs: Vec) -> bool { let m = Array::from_shape_vec((axis_length, axis_length), xs).unwrap(); // Unordered list of quantile indexes to look up, with a duplicate - let quantile_indexes = vec![ + let quantile_indexes = Array::from(vec![ n64(0.75), n64(0.90), n64(0.95), @@ -350,37 +365,37 @@ fn test_quantiles_axis_mut(mut xs: Vec) -> bool { n64(0.25), n64(0.5), n64(0.5), - ]; + ]); // Test out all interpolation methods let mut correct = true; correct &= check_one_interpolation_method_for_quantiles_axis_mut( m.clone(), - &quantile_indexes, + quantile_indexes.view(), Axis(0), &Linear, ); correct &= check_one_interpolation_method_for_quantiles_axis_mut( m.clone(), - &quantile_indexes, + quantile_indexes.view(), Axis(0), &Higher, ); correct &= check_one_interpolation_method_for_quantiles_axis_mut( m.clone(), - &quantile_indexes, + quantile_indexes.view(), Axis(0), &Lower, ); correct &= check_one_interpolation_method_for_quantiles_axis_mut( m.clone(), - &quantile_indexes, + quantile_indexes.view(), Axis(0), &Midpoint, ); correct &= check_one_interpolation_method_for_quantiles_axis_mut( m.clone(), - &quantile_indexes, + quantile_indexes.view(), Axis(0), &Nearest, ); @@ -389,7 +404,7 @@ fn test_quantiles_axis_mut(mut xs: Vec) -> bool { fn check_one_interpolation_method_for_quantiles_axis_mut( mut v: Array2, - quantile_indexes: &[N64], + quantile_indexes: ArrayView1, axis: Axis, interpolate: &impl Interpolate, ) -> bool { From 8f9f0b615a09de0bdcbb9364e6bd88849b3bf353 Mon Sep 17 00:00:00 2001 From: Jim Turner Date: Mon, 1 Apr 2019 22:14:59 -0400 Subject: [PATCH 19/19] Remove unnecessary IndexSet --- src/quantile/mod.rs | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/src/quantile/mod.rs b/src/quantile/mod.rs index 7d59037a..c53646e5 100644 --- a/src/quantile/mod.rs +++ b/src/quantile/mod.rs @@ -1,6 +1,5 @@ use self::interpolate::{higher_index, lower_index, Interpolate}; use super::sort::get_many_from_sorted_mut_unchecked; -use indexmap::IndexSet; use ndarray::prelude::*; use ndarray::{Data, DataMut, RemoveAxis, Zip}; use noisy_float::types::N64; @@ -465,23 +464,17 @@ where return Some(Array::from_shape_vec(results_shape, Vec::new()).unwrap()); } - let mut deduped_qs: Vec = qs.to_vec(); - deduped_qs.sort_by(|a, b| a.partial_cmp(b).unwrap()); - deduped_qs.dedup(); - - // IndexSet preserves insertion order: - // - indexes will stay sorted; - // - we avoid index duplication. - let mut searched_indexes = IndexSet::new(); - for q in deduped_qs.iter() { - if I::needs_lower(*q, axis_len) { - searched_indexes.insert(lower_index(*q, axis_len)); + let mut searched_indexes = Vec::with_capacity(2 * qs.len()); + for &q in &qs { + if I::needs_lower(q, axis_len) { + searched_indexes.push(lower_index(q, axis_len)); } - if I::needs_higher(*q, axis_len) { - searched_indexes.insert(higher_index(*q, axis_len)); + if I::needs_higher(q, axis_len) { + searched_indexes.push(higher_index(q, axis_len)); } } - let searched_indexes: Vec = searched_indexes.into_iter().collect(); + searched_indexes.sort(); + searched_indexes.dedup(); let mut results = Array::from_elem(results_shape, data.first().unwrap().clone()); Zip::from(results.lanes_mut(axis))