Repository: bluss/rust-ndarray
Pull request #300 (2017-04-03 19:48:31)

Various cleanups
- There's one performance improvement here, checked indexing for arrays of ndim >= 3.


Patch:
diff --git a/benches/bench1.rs b/benches/bench1.rs
index 8e8d1000..399a8b12 100644
--- a/benches/bench1.rs
+++ b/benches/bench1.rs
@@ -583,6 +583,33 @@ fn add_2d_f32_regular(bench: &mut test::Bencher)
     });
 }
 
+const ADD3DSZ: usize = 16;
+
+#[bench]
+fn add_3d_strided(bench: &mut test::Bencher)
+{
+    let mut a = Array::<i32, _>::zeros((ADD3DSZ, ADD3DSZ, ADD3DSZ * 2));
+    let mut a = a.slice_mut(s![.., .., ..;2]);
+    let b = Array::<i32, _>::zeros(a.dim());
+    let bv = b.view();
+    bench.iter(|| {
+        a += &bv;
+    });
+}
+
+#[bench]
+fn add_3d_strided_dyn(bench: &mut test::Bencher)
+{
+    let mut a = Array::<i32, _>::zeros(&[ADD3DSZ, ADD3DSZ, ADD3DSZ * 2][..]);
+    let mut a = a.slice_mut(s![.., .., ..;2]);
+    let b = Array::<i32, _>::zeros(a.dim());
+    let bv = b.view();
+    bench.iter(|| {
+        a += &bv;
+    });
+}
+
+
 const ADD1D_SIZE: usize = 64 * 64;
 
 #[bench]
diff --git a/benches/iter.rs b/benches/iter.rs
index 478ac918..106bb245 100644
--- a/benches/iter.rs
+++ b/benches/iter.rs
@@ -202,3 +202,35 @@ fn vector_sum_3_zip_unchecked(bench: &mut Bencher)
         }
     });
 }
+
+// index iterator size
+const ISZ: usize = 16;
+
+#[bench]
+fn indexed_iter_3d_ix3(bench: &mut Bencher) {
+    let mut a = Array::<f64, _>::zeros((ISZ, ISZ, ISZ));
+    for ((i, j, k), elt) in a.indexed_iter_mut() {
+        *elt = (i + 100 * j + 10000 * k) as _;
+    }
+
+    bench.iter(|| {
+        for (i, &elt) in a.indexed_iter() {
+            assert!(a[i] == elt);
+        }
+    })
+}
+
+#[bench]
+fn indexed_iter_3d_dyn(bench: &mut Bencher) {
+    let mut a = Array::<f64, _>::zeros((ISZ, ISZ, ISZ));
+    for ((i, j, k), elt) in a.indexed_iter_mut() {
+        *elt = (i + 100 * j + 10000 * k) as _;
+    }
+    let a = a.into_shape(&[ISZ; 3][..]).unwrap();
+
+    bench.iter(|| {
+        for (i, &elt) in a.indexed_iter() {
+            assert!(a[i] == elt);
+        }
+    })
+}
diff --git a/src/aliases.rs b/src/aliases.rs
index 16841488..759b2039 100644
--- a/src/aliases.rs
+++ b/src/aliases.rs
@@ -71,8 +71,7 @@ pub type Ix6 = Dim<[Ix; 6]>;
 /// // We can index into a, b using fixed size arrays:
 /// a[[0, 0, 0, 0]] = 0.;
 /// b[[0, 2, 3]] = a[[0, 0, 2, 3]];
-///
-/// // Note: It will panic at runtime if the number of indices given does
+/// // Note: indexing will panic at runtime if the number of indices given does
 /// // not match the array.
 ///
 /// // We can keep them in the same vector because both the arrays have
diff --git a/src/arraytraits.rs b/src/arraytraits.rs
index d74af9e8..ec069dbb 100644
--- a/src/arraytraits.rs
+++ b/src/arraytraits.rs
@@ -230,7 +230,7 @@ impl<'a, A, Slice: ?Sized> From<&'a Slice> for ArrayBase<ViewRepr<&'a A>, Ix1>
     fn from(slice: &'a Slice) -> Self {
         let xs = slice.as_ref();
         unsafe {
-            Self::new_(xs.as_ptr(), Ix1(xs.len()), Ix1(1))
+            Self::from_shape_ptr(xs.len(), xs.as_ptr())
         }
     }
 }
@@ -256,7 +256,7 @@ impl<'a, A, Slice: ?Sized> From<&'a mut Slice> for ArrayBase<ViewRepr<&'a mut A>
     fn from(slice: &'a mut Slice) -> Self {
         let xs = slice.as_mut();
         unsafe {
-            Self::new_(xs.as_mut_ptr(), Ix1(xs.len()), Ix1(1))
+            Self::from_shape_ptr(xs.len(), xs.as_mut_ptr())
         }
     }
 }
diff --git a/src/data_traits.rs b/src/data_traits.rs
index dcbe8c75..6c647f14 100644
--- a/src/data_traits.rs
+++ b/src/data_traits.rs
@@ -15,6 +15,8 @@ use {
     ArrayBase,
     Dimension,
     ViewRepr,
+    OwnedRepr,
+    OwnedRcRepr,
 };
 
 /// Array representation trait.
@@ -68,7 +70,7 @@ pub unsafe trait DataClone : Data {
     }
 }
 
-unsafe impl<A> Data for Rc<Vec<A>> {
+unsafe impl<A> Data for OwnedRcRepr<A> {
     type Elem = A;
     fn _data_slice(&self) -> &[A] {
         self
@@ -76,7 +78,7 @@ unsafe impl<A> Data for Rc<Vec<A>> {
 }
 
 // NOTE: Copy on write
-unsafe impl<A> DataMut for Rc<Vec<A>>
+unsafe impl<A> DataMut for OwnedRcRepr<A>
     where A: Clone
 {
     fn ensure_unique<D>(self_: &mut ArrayBase<Self, D>)
@@ -112,23 +114,23 @@ unsafe impl<A> DataMut for Rc<Vec<A>>
     }
 }
 
-unsafe impl<A> DataClone for Rc<Vec<A>> {
+unsafe impl<A> DataClone for OwnedRcRepr<A> {
     unsafe fn clone_with_ptr(&self, ptr: *mut Self::Elem) -> (Self, *mut Self::Elem) {
         // pointer is preserved
         (self.clone(), ptr)
     }
 }
 
-unsafe impl<A> Data for Vec<A> {
+unsafe impl<A> Data for OwnedRepr<A> {
     type Elem = A;
     fn _data_slice(&self) -> &[A] {
         self
     }
 }
 
-unsafe impl<A> DataMut for Vec<A> { }
+unsafe impl<A> DataMut for OwnedRepr<A> { }
 
-unsafe impl<A> DataClone for Vec<A>
+unsafe impl<A> DataClone for OwnedRepr<A>
     where A: Clone
 {
     unsafe fn clone_with_ptr(&self, ptr: *mut Self::Elem) -> (Self, *mut Self::Elem) {
@@ -185,7 +187,7 @@ pub unsafe trait DataOwned : Data {
     #[doc(hidden)]
     fn new(elements: Vec<Self::Elem>) -> Self;
     #[doc(hidden)]
-    fn into_shared(self) -> Rc<Vec<Self::Elem>>;
+    fn into_shared(self) -> OwnedRcRepr<Self::Elem>;
 }
 
 /// Array representation trait.
@@ -195,23 +197,23 @@ pub unsafe trait DataOwned : Data {
 /// ***Internal trait, see `Data`.***
 pub unsafe trait DataShared : Clone + DataClone { }
 
-unsafe impl<A> DataShared for Rc<Vec<A>> {}
+unsafe impl<A> DataShared for OwnedRcRepr<A> {}
 unsafe impl<'a, A> DataShared for ViewRepr<&'a A> {}
 
-unsafe impl<A> DataOwned for Vec<A> {
+unsafe impl<A> DataOwned for OwnedRepr<A> {
     fn new(elements: Vec<A>) -> Self {
         elements
     }
-    fn into_shared(self) -> Rc<Vec<A>> {
+    fn into_shared(self) -> OwnedRcRepr<A> {
         Rc::new(self)
     }
 }
 
-unsafe impl<A> DataOwned for Rc<Vec<A>> {
+unsafe impl<A> DataOwned for OwnedRcRepr<A> {
     fn new(elements: Vec<A>) -> Self {
         Rc::new(elements)
     }
-    fn into_shared(self) -> Rc<Vec<A>> {
+    fn into_shared(self) -> OwnedRcRepr<A> {
         self
     }
 }
diff --git a/src/dimension/dimension_trait.rs b/src/dimension/dimension_trait.rs
index 930fb751..e3cdefe1 100644
--- a/src/dimension/dimension_trait.rs
+++ b/src/dimension/dimension_trait.rs
@@ -113,7 +113,7 @@ pub unsafe trait Dimension : Clone + Eq + Debug + Send + Sync + Default +
         {
             let mut it = strides.slice_mut().iter_mut().rev();
             // Set first element to 1
-            for rs in it.by_ref() {
+            while let Some(rs) = it.next() {
                 *rs = 1;
                 break;
             }
@@ -134,12 +134,12 @@ pub unsafe trait Dimension : Clone + Eq + Debug + Send + Sync + Default +
         {
             let mut it = strides.slice_mut().iter_mut();
             // Set first element to 1
-            for rs in it.by_ref() {
+            while let Some(rs) = it.next() {
                 *rs = 1;
                 break;
             }
             let mut cum_prod = 1;
-            for (rs, dim) in it.zip(self.slice().iter()) {
+            for (rs, dim) in it.zip(self.slice()) {
                 cum_prod *= *dim;
                 *rs = cum_prod;
             }
@@ -558,30 +558,6 @@ unsafe impl Dimension for Dim<[Ix; 2]> {
         }
     }
     
-    #[inline]
-    fn is_contiguous(dim: &Self, strides: &Self) -> bool {
-        let defaults = dim.default_strides();
-        if strides.equal(&defaults) {
-            return true;
-        }
-        
-        if dim.ndim() == 1 { return false; }
-        let order = strides._fastest_varying_stride_order();
-        let strides = strides.slice();
-
-        // FIXME: Negative strides
-        let dim_slice = dim.slice();
-        let mut cstride = 1;
-        for &i in order.slice() {
-            // a dimension of length 1 can have unequal strides
-            if dim_slice[i] != 1 && strides[i] != cstride {
-                return false;
-            }
-            cstride *= dim_slice[i];
-        }
-        true
-    }
-
     #[inline]
     fn first_index(&self) -> Option<Self> {
         let m = get!(self, 0);
@@ -683,6 +659,26 @@ unsafe impl Dimension for Dim<[Ix; 3]> {
         stride_offset(i, s) + stride_offset(j, t) + stride_offset(k, u)
     }
 
+    /// Return stride offset for this dimension and index.
+    #[inline]
+    fn stride_offset_checked(&self, strides: &Self, index: &Self) -> Option<isize>
+    {
+        let m = get!(self, 0);
+        let n = get!(self, 1);
+        let l = get!(self, 2);
+        let i = get!(index, 0);
+        let j = get!(index, 1);
+        let k = get!(index, 2);
+        let s = get!(strides, 0);
+        let t = get!(strides, 1);
+        let u = get!(strides, 2);
+        if i < m && j < n && k < l {
+            Some(stride_offset(i, s) + stride_offset(j, t) + stride_offset(k, u))
+        } else {
+            None
+        }
+    }
+
     #[inline]
     fn _fastest_varying_stride_order(&self) -> Self {
         let mut stride = *self;
diff --git a/src/dimension/mod.rs b/src/dimension/mod.rs
index 83eb03d3..baec4d43 100644
--- a/src/dimension/mod.rs
+++ b/src/dimension/mod.rs
@@ -139,6 +139,7 @@ fn stride_offset_checked_arithmetic<D>(dim: &D, strides: &D, index: &D)
 }
 
 /// Stride offset checked general version (slices)
+#[inline]
 pub fn stride_offset_checked(dim: &[Ix], strides: &[Ix], index: &[Ix]) -> Option<isize> {
     if index.len() != dim.len() {
         return None;
diff --git a/src/free_functions.rs b/src/free_functions.rs
index 9d42d32f..8494dc5d 100644
--- a/src/free_functions.rs
+++ b/src/free_functions.rs
@@ -68,7 +68,7 @@ pub fn rcarr1<A: Clone>(xs: &[A]) -> RcArray<A, Ix1> {
 
 /// Create a zero-dimensional array view borrowing `x`.
 pub fn aview0<A>(x: &A) -> ArrayView0<A> {
-    unsafe { ArrayView::new_(x, Ix0(), Ix0()) }
+    unsafe { ArrayView::from_shape_ptr(Ix0(), x) }
 }
 
 /// Create a one-dimensional array view with elements borrowing `xs`.
@@ -98,8 +98,7 @@ pub fn aview2<A, V: FixedInitializer<Elem=A>>(xs: &[V]) -> ArrayView2<A> {
     };
     let dim = Ix2(rows, cols);
     unsafe {
-        let strides = dim.default_strides();
-        ArrayView::new_(data.as_ptr(), dim, strides)
+        ArrayView::from_shape_ptr(dim, data.as_ptr())
     }
 }
 
diff --git a/src/impl_owned_array.rs b/src/impl_owned_array.rs
index 16b70be3..5d90c023 100644
--- a/src/impl_owned_array.rs
+++ b/src/impl_owned_array.rs
@@ -2,8 +2,12 @@
 use std::rc::Rc;
 
 use imp_prelude::*;
+use {
+    OwnedRepr,
+    OwnedRcRepr,
+};
 
-impl<A, D> ArrayBase<Vec<A>, D>
+impl<A, D> ArrayBase<OwnedRepr<A>, D>
     where D: Dimension
 {
     /// Return a vector of the elements in the array, in the way they are
@@ -17,7 +21,7 @@ impl<A, D> ArrayBase<Vec<A>, D>
 }
 
 // RcArray
-impl<A, D> ArrayBase<Rc<Vec<A>>, D>
+impl<A, D> ArrayBase<OwnedRcRepr<A>, D>
     where A: Clone,
           D: Dimension
 {
diff --git a/src/lib.rs b/src/lib.rs
index 33b74bbe..17523b7c 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -504,7 +504,7 @@ pub struct ArrayBase<S, D>
 /// An array where the data has shared ownership and is copy on write.
 /// It can act as both an owner as the data as well as a shared reference (view
 /// like).
-pub type RcArray<A, D> = ArrayBase<Rc<Vec<A>>, D>;
+pub type RcArray<A, D> = ArrayBase<OwnedRcRepr<A>, D>;
 
 /// An array that owns its data uniquely.
 ///
@@ -522,7 +522,7 @@ pub type RcArray<A, D> = ArrayBase<Rc<Vec<A>>, D>;
 /// [`Array1`](Array1.t.html),
 /// [`Array2`](Array2.t.html),
 /// [`Array3`](Array3.t.html) and so on.
-pub type Array<A, D> = ArrayBase<Vec<A>, D>;
+pub type Array<A, D> = ArrayBase<OwnedRepr<A>, D>;
 
 /// A lightweight array view.
 ///
@@ -547,6 +547,12 @@ pub type ArrayView<'a, A, D> = ArrayBase<ViewRepr<&'a A>, D>;
 /// [ab]: struct.ArrayBase.html
 pub type ArrayViewMut<'a, A, D> = ArrayBase<ViewRepr<&'a mut A>, D>;
 
+/// Array's representation.
+type OwnedRepr<A> = Vec<A>;
+
+/// RcArray's representation.
+type OwnedRcRepr<A> = Rc<Vec<A>>;
+
 /// Array view’s representation.
 #[derive(Copy, Clone)]
 // This is just a marker type, to carry the lifetime parameter.
diff --git a/src/numeric/impl_numeric.rs b/src/numeric/impl_numeric.rs
index 8beeeba0..e02e7eaf 100644
--- a/src/numeric/impl_numeric.rs
+++ b/src/numeric/impl_numeric.rs
@@ -15,6 +15,8 @@ use numeric_util;
 
 use {
     LinalgScalar,
+    FoldWhile,
+    Zip,
 };
 
 /// Numerical methods for arrays.
@@ -124,8 +126,20 @@ impl<A, S, D> ArrayBase<S, D>
               S2: Data<Elem=A>,
               E: Dimension,
     {
-        let rhs_broadcast = rhs.broadcast_unwrap(self.raw_dim());
-        self.iter().zip(rhs_broadcast.iter()).all(|(x, y)| (*x - *y).abs() <= tol)
+        let result =
+            Zip::from(self)
+            .and(rhs.broadcast_unwrap(self.raw_dim()))
+            .fold_while((), |_, x, y| {
+                if (*x - *y).abs() <= tol {
+                    FoldWhile::Continue(())
+                } else {
+                    FoldWhile::Done(())
+                }
+            });
+        match result {
+            FoldWhile::Continue(_) => true,
+            FoldWhile::Done(_) => false,
+        }
     }
 }
 


Test patch:
diff --git a/tests/dimension.rs b/tests/dimension.rs
index 37d36670..6f75ebbf 100644
--- a/tests/dimension.rs
+++ b/tests/dimension.rs
@@ -8,6 +8,7 @@ use ndarray::{
     Axis,
     Dimension,
     Dim,
+    IntoDimension,
 };
 
 #[test]
@@ -172,3 +173,37 @@ fn test_array_view() {
     test_dim(&Dim(vec![1, 1, 2, 3]));
     test_dim(&Dim(7));
 }
+
+#[test]
+fn test_all_ndindex() {
+macro_rules! ndindex {
+    ($($i:expr),*) => {
+        for &rev in &[false, true] {
+            // rev is for C / F order
+            let size = $($i *)* 1;
+            let mut a = Array::linspace(0., (size - 1) as f64, size);
+            if rev {
+                a = a.reversed_axes();
+            }
+            for (i, &elt) in a.indexed_iter() {
+                let dim = i.into_dimension();
+                assert_eq!(elt, a[i]);
+                assert_eq!(elt, a[dim]);
+            }
+            let dim = a.shape().to_vec();
+            let b = a.broadcast(dim).unwrap();
+            for (i, &elt) in a.indexed_iter() {
+                let dim = i.into_dimension();
+                assert_eq!(elt, b[dim]);
+                assert_eq!(elt, b[dim.slice()]);
+            }
+        }
+    }
+}
+    ndindex!(10);
+    ndindex!(10, 4);
+    ndindex!(10, 4, 3);
+    ndindex!(10, 4, 3, 2);
+    ndindex!(10, 4, 3, 2, 2);
+    ndindex!(10, 4, 3, 2, 2, 2);
+}
diff --git a/tests/ixdyn.rs b/tests/ixdyn.rs
index 5e4db018..e293c7ab 100644
--- a/tests/ixdyn.rs
+++ b/tests/ixdyn.rs
@@ -51,6 +51,25 @@ fn test_ixdyn_iterate() {
     }
 }
 
+#[test]
+fn test_ixdyn_index_iterate() {
+    for &rev in &[false, true] {
+        let mut a = Array::zeros((2, 3, 4).set_f(rev));
+        let dim = a.shape().to_vec();
+        for ((i, j, k), elt) in a.indexed_iter_mut() {
+            *elt = i + 10 * j + 100 * k;
+        }
+        let a = a.into_shape(dim).unwrap();
+        println!("{:?}", a.dim());
+        let mut c = 0;
+        for (i, elt) in a.indexed_iter() {
+            assert_eq!(a[i], *elt);
+            c += 1;
+        }
+        assert_eq!(c, a.len());
+    }
+}
+
 #[test]
 fn test_ixdyn_uget() {
     // check that we are out of bounds