Clarify which (arg)min/max index/value is returned (#32)

* Clarify which (arg)min/max index/value is returned

For performance, it's beneficial to avoid guaranteeing a particular
iteration order. For example, the current implementation of `min` may
return any of the minima because the iteration order of
`ArrayBase.fold()` is unspecified. The current implementation of
`argmin` does always return the first minimum (in logical order) since
`ArrayBase.indexed_iter()` always iterates in logical order, but we
may want to optimize the iteration order in the future.

* Replace some tests with argmin/max_matches_min/max

See discussion at #32 (comment)

Author: jturner314

src/quantile.rs

@@ -182,14 +182,18 @@ where
     S: Data<Elem = A>,
     D: Dimension,
 {
-    /// Finds the first index of the minimum value of the array.
+    /// Finds the index of the minimum value of the array.
     ///
     /// Returns `None` if any of the pairwise orderings tested by the function
     /// are undefined. (For example, this occurs if there are any
     /// floating-point NaN values in the array.)
     ///
     /// Returns `None` if the array is empty.
     ///
+    /// Even if there are multiple (equal) elements that are minima, only one
+    /// index is returned. (Which one is returned is unspecified and may depend
+    /// on the memory layout of the array.)
+    ///
     /// # Example
     ///
     /// ```
@@ -214,12 +218,20 @@ where
     /// floating-point NaN values in the array.)
     ///
     /// Additionally, returns `None` if the array is empty.
+    ///
+    /// Even if there are multiple (equal) elements that are minima, only one
+    /// is returned. (Which one is returned is unspecified and may depend on
+    /// the memory layout of the array.)
     fn min(&self) -> Option<&A>
     where
         A: PartialOrd;
 
     /// Finds the elementwise minimum of the array, skipping NaN values.
     ///
+    /// Even if there are multiple (equal) elements that are minima, only one
+    /// is returned. (Which one is returned is unspecified and may depend on
+    /// the memory layout of the array.)
+    ///
     /// **Warning** This method will return a NaN value if none of the values
     /// in the array are non-NaN values. Note that the NaN value might not be
     /// in the array.
@@ -228,14 +240,18 @@ where
         A: MaybeNan,
         A::NotNan: Ord;
 
-    /// Finds the first index of the maximum value of the array.
+    /// Finds the index of the maximum value of the array.
     ///
     /// Returns `None` if any of the pairwise orderings tested by the function
     /// are undefined. (For example, this occurs if there are any
     /// floating-point NaN values in the array.)
     ///
     /// Returns `None` if the array is empty.
     ///
+    /// Even if there are multiple (equal) elements that are maxima, only one
+    /// index is returned. (Which one is returned is unspecified and may depend
+    /// on the memory layout of the array.)
+    ///
     /// # Example
     ///
     /// ```
@@ -260,12 +276,20 @@ where
     /// floating-point NaN values in the array.)
     ///
     /// Additionally, returns `None` if the array is empty.
+    ///
+    /// Even if there are multiple (equal) elements that are maxima, only one
+    /// is returned. (Which one is returned is unspecified and may depend on
+    /// the memory layout of the array.)
     fn max(&self) -> Option<&A>
     where
         A: PartialOrd;
 
     /// Finds the elementwise maximum of the array, skipping NaN values.
     ///
+    /// Even if there are multiple (equal) elements that are maxima, only one
+    /// is returned. (Which one is returned is unspecified and may depend on
+    /// the memory layout of the array.)
+    ///
     /// **Warning** This method will return a NaN value if none of the values
     /// in the array are non-NaN values. Note that the NaN value might not be
     /// in the array.

tests/quantile.rs

@@ -1,12 +1,14 @@
 #[macro_use(array)]
 extern crate ndarray;
 extern crate ndarray_stats;
+extern crate quickcheck;
 
 use ndarray::prelude::*;
 use ndarray_stats::{
     interpolate::{Higher, Linear, Lower, Midpoint, Nearest},
     Quantile1dExt, QuantileExt,
 };
+use quickcheck::quickcheck;
 
 #[test]
 fn test_argmin() {
@@ -19,13 +21,17 @@ fn test_argmin() {
     let a = array![[1., 5., 3.], [2., ::std::f64::NAN, 6.]];
     assert_eq!(a.argmin(), None);
 
-    let a = array![[1, 0, 3], [2, 0, 6]];
-    assert_eq!(a.argmin(), Some((0, 1)));
-
     let a: Array2<i32> = array![[], []];
     assert_eq!(a.argmin(), None);
 }
 
+quickcheck! {
+    fn argmin_matches_min(data: Vec<f32>) -> bool {
+        let a = Array1::from(data);
+        a.argmin().map(|i| a[i]) == a.min().cloned()
+    }
+}
+
 #[test]
 fn test_min() {
     let a = array![[1, 5, 3], [2, 0, 6]];
@@ -64,13 +70,17 @@ fn test_argmax() {
     let a = array![[1., 5., 3.], [2., ::std::f64::NAN, 6.]];
     assert_eq!(a.argmax(), None);
 
-    let a = array![[1, 5, 6], [2, 0, 6]];
-    assert_eq!(a.argmax(), Some((0, 2)));
-
     let a: Array2<i32> = array![[], []];
     assert_eq!(a.argmax(), None);
 }
 
+quickcheck! {
+    fn argmax_matches_max(data: Vec<f32>) -> bool {
+        let a = Array1::from(data);
+        a.argmax().map(|i| a[i]) == a.max().cloned()
+    }
+}
+
 #[test]
 fn test_max() {
     let a = array![[1, 5, 7], [2, 0, 6]];