From 25df00a4cc898020b3ed25ccb47627a1f442547c Mon Sep 17 00:00:00 2001 From: LukeMathWalker Date: Sun, 24 Feb 2019 18:01:32 +0000 Subject: [PATCH 1/6] Reproduce overflow error for Midpoint --- src/lib.rs | 3 ++- tests/quantile.rs | 11 +++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 5d764a67..4039c093 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -6,7 +6,8 @@ //! - [`partitioning`]; //! - [`correlation analysis`] (covariance, pearson correlation); //! - [`histogram computation`]. -//! +// +//! ! //! Please feel free to contribute new functionality! A roadmap can be found [`here`]. //! //! Our work is inspired by other existing statistical packages such as diff --git a/tests/quantile.rs b/tests/quantile.rs index ea5ddaa7..ae85d277 100644 --- a/tests/quantile.rs +++ b/tests/quantile.rs @@ -6,6 +6,7 @@ use ndarray::prelude::*; use ndarray_stats::{ interpolate::{Higher, Linear, Lower, Midpoint, Nearest}, QuantileExt, + Quantile1dExt, }; #[test] @@ -148,3 +149,13 @@ fn test_quantile_axis_skipnan_mut_linear_opt_i32() { assert_eq!(q[0], Some(3)); assert!(q[1].is_none()); } + +#[test] +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::(0.5).unwrap(); + let expected_median = 130; + assert_eq!(median, expected_median); +} \ No newline at end of file From 7381adb6fb650f329d5bf3c36c6b13aab5a4eda5 Mon Sep 17 00:00:00 2001 From: LukeMathWalker Date: Sun, 24 Feb 2019 18:30:24 +0000 Subject: [PATCH 2/6] Fix Midpoint --- src/quantile.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/quantile.rs b/src/quantile.rs index 9188aa2f..393a3746 100644 --- a/src/quantile.rs +++ b/src/quantile.rs @@ -9,7 +9,7 @@ pub mod interpolate { use ndarray::azip; use ndarray::prelude::*; use num_traits::{FromPrimitive, ToPrimitive}; - use std::ops::{Add, Div}; + use std::ops::{Add, Div, Sub}; /// Used to provide an interpolation strategy to [`quantile_axis_mut`]. /// @@ -116,7 +116,7 @@ pub mod interpolate { impl Interpolate for Midpoint where - T: Add + Div + Clone + FromPrimitive, + T: Add + Sub + Div + Clone + FromPrimitive, { fn needs_lower(_q: f64, _len: usize) -> bool { true @@ -134,7 +134,14 @@ pub mod interpolate { D: Dimension, { let denom = T::from_u8(2).unwrap(); - (lower.unwrap() + higher.unwrap()).mapv_into(|x| x / denom.clone()) + let mut lower = lower.unwrap(); + let higher = higher.unwrap(); + azip!( + mut lower, ref higher in { + *lower = lower.clone() + (higher.clone() - lower.clone()) / denom.clone() + } + ); + lower } } From dcc83441e0a219e0f34ca6b59bfcc8ad487dd06f Mon Sep 17 00:00:00 2001 From: LukeMathWalker Date: Sun, 24 Feb 2019 18:32:22 +0000 Subject: [PATCH 3/6] Typo --- src/lib.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 4039c093..5d764a67 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -6,8 +6,7 @@ //! - [`partitioning`]; //! - [`correlation analysis`] (covariance, pearson correlation); //! - [`histogram computation`]. -// -//! ! +//! //! Please feel free to contribute new functionality! A roadmap can be found [`here`]. //! //! Our work is inspired by other existing statistical packages such as From e4b2d7c6b13ae2c6c598d6574c1a280fe7b84d13 Mon Sep 17 00:00:00 2001 From: LukeMathWalker Date: Tue, 26 Feb 2019 08:18:31 +0000 Subject: [PATCH 4/6] Use NumOps when arithmetic is required --- src/quantile.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/quantile.rs b/src/quantile.rs index 393a3746..be2aee58 100644 --- a/src/quantile.rs +++ b/src/quantile.rs @@ -8,7 +8,7 @@ use {MaybeNan, MaybeNanExt, Sort1dExt}; pub mod interpolate { use ndarray::azip; use ndarray::prelude::*; - use num_traits::{FromPrimitive, ToPrimitive}; + use num_traits::{FromPrimitive, ToPrimitive, NumOps}; use std::ops::{Add, Div, Sub}; /// Used to provide an interpolation strategy to [`quantile_axis_mut`]. @@ -116,7 +116,7 @@ pub mod interpolate { impl Interpolate for Midpoint where - T: Add + Sub + Div + Clone + FromPrimitive, + T: NumOps + Clone + FromPrimitive, { fn needs_lower(_q: f64, _len: usize) -> bool { true @@ -147,7 +147,7 @@ pub mod interpolate { impl Interpolate for Linear where - T: Add + Clone + FromPrimitive + ToPrimitive, + T: NumOps + Clone + FromPrimitive + ToPrimitive, { fn needs_lower(_q: f64, _len: usize) -> bool { true From 0a76e8247f468292807a1fc5a2b01a990612f397 Mon Sep 17 00:00:00 2001 From: LukeMathWalker Date: Tue, 26 Feb 2019 08:24:31 +0000 Subject: [PATCH 5/6] Add Rem implementation --- src/maybe_nan/impl_not_none.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/maybe_nan/impl_not_none.rs b/src/maybe_nan/impl_not_none.rs index 22703084..0fdd8b4e 100644 --- a/src/maybe_nan/impl_not_none.rs +++ b/src/maybe_nan/impl_not_none.rs @@ -2,7 +2,7 @@ use super::NotNone; use num_traits::{FromPrimitive, ToPrimitive}; use std::cmp; use std::fmt; -use std::ops::{Add, Deref, DerefMut, Div, Mul, Sub}; +use std::ops::{Add, Deref, DerefMut, Div, Mul, Sub, Rem}; impl Deref for NotNone { type Target = T; @@ -96,6 +96,14 @@ impl Div for NotNone { } } +impl Rem for NotNone { + type Output = NotNone; + #[inline] + fn rem(self, rhs: Self) -> Self::Output { + self.map(|v| v.rem(rhs.unwrap())) + } +} + impl ToPrimitive for NotNone { #[inline] fn to_isize(&self) -> Option { From 52e85833bfb635a04171f4f48ca67647e3a07809 Mon Sep 17 00:00:00 2001 From: LukeMathWalker Date: Tue, 26 Feb 2019 08:25:17 +0000 Subject: [PATCH 6/6] Remove unused imports --- src/quantile.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/quantile.rs b/src/quantile.rs index be2aee58..f92cdff7 100644 --- a/src/quantile.rs +++ b/src/quantile.rs @@ -9,7 +9,6 @@ pub mod interpolate { use ndarray::azip; use ndarray::prelude::*; use num_traits::{FromPrimitive, ToPrimitive, NumOps}; - use std::ops::{Add, Div, Sub}; /// Used to provide an interpolation strategy to [`quantile_axis_mut`]. ///