Restructured usage of convolves, added unit testing.

examples/convolution.rs

@@ -1,17 +0,0 @@
-extern crate nalgebra;
-use nalgebra::{Vector2,Vector3,Vector4,Vector5,convolve_full,convolve_same,convolve_valid};
-
-fn main(){
-    let vec = Vector4::new(1.0,2.0,3.0,4.0);
-    let ker = Vector3::new(1.0,2.0,2.1);
-
-    let actual =  Vector5::from_vec(vec![1.0,4.0,7.0,10.0,8.0]);
-
-    let expected = convolve_full(vec,ker);
-    let expected2 = convolve_same(vec,ker);
-    // let expected3 = convolve_valid(vec,ker);
-    println!("{}", actual);
-    println!("{}", expected);
-    println!("{}", expected2);
-    // println!("{}", expected3);
-}

src/linalg/convolution.rs

@@ -1,20 +1,19 @@
 use base::allocator::Allocator;
 use base::default_allocator::DefaultAllocator;
-use base::dimension::{DimAdd, DimDiff, DimMax, DimMaximum, DimName, DimSub, DimSum,Dim};
+use base::dimension::{Dim, DimAdd, DimDiff, DimMax, DimMaximum, DimSub, DimSum};
 use std::cmp;
 use storage::Storage;
 use {zero, Real, Vector, VectorN, U1};
 
-/// Returns the convolution of the vector and a kernel
+/// Returns the convolution of the target vector and a kernel
 ///
 /// # Arguments
 ///
 /// * `vector` - A Vector with size > 0
 /// * `kernel` - A Vector with size > 0
 ///
-///  This function is commutative. If kernel > vector,
+/// # Errors
-///  they will swap their roles as in
+/// Inputs must statisfy `vector.len() >= kernel.len() > 0`.
-///  (self, kernel) = (kernel,self)
 ///
 pub fn convolve_full<N, D1, D2, S1, S2>(
     vector: Vector<N, D1, S1>,
@@ -27,18 +26,13 @@ where
     DimSum<D1, D2>: DimSub<U1>,
     S1: Storage<N, D1>,
     S2: Storage<N, D2>,
-    DimSum<D1, D2>: Dim,
     DefaultAllocator: Allocator<N, DimDiff<DimSum<D1, D2>, U1>>,
 {
     let vec = vector.len();
     let ker = kernel.len();
 
-    if vec == 0 || ker == 0 {
+    if ker == 0 || ker > vec {
-        panic!("Convolve's inputs must not be 0-sized. ");
+        panic!("convolve_full expects `vector.len() >= kernel.len() > 0`, received {} and {} respectively.",vec,ker);
-    }
-
-    if ker > vec {
-        return convolve_full(kernel, vector);
     }
 
     let result_len = vector.data.shape().0.add(kernel.data.shape().0).sub(U1);
@@ -61,45 +55,38 @@ where
     conv
 }
 
-
-
 /// Returns the convolution of the vector and a kernel
 /// The output convolution consists only of those elements that do not rely on the zero-padding.
 /// # Arguments
 ///
 /// * `vector` - A Vector with size > 0
 /// * `kernel` - A Vector with size > 0
-/// 
+///
-/// This function is commutative. If kernel > vector,
+///
-/// they will swap their roles as in
+/// # Errors
-/// (self, kernel) = (kernel,self)
+/// Inputs must statisfy `vector.len() >= kernel.len() > 0`.
 ///
 pub fn convolve_valid<N, D1, D2, S1, S2>(
     vector: Vector<N, D1, S1>,
     kernel: Vector<N, D2, S2>,
-) -> VectorN<N, DimSum<DimDiff<D1, D2>, U1>>
+) -> VectorN<N, DimDiff<DimSum<D1, U1>, D2>>
 where
     N: Real,
-    D1: DimSub<D2>,
+    D1: DimAdd<U1>,
-    D2: DimSub<D1, Output = DimDiff<D1, D2>>,
+    D2: Dim,
-    DimDiff<D1, D2>: DimAdd<U1>,
+    DimSum<D1, U1>: DimSub<D2>,
     S1: Storage<N, D1>,
     S2: Storage<N, D2>,
-    DimDiff<D1, D2>: DimName,
+    DefaultAllocator: Allocator<N, DimDiff<DimSum<D1, U1>, D2>>,
-    DefaultAllocator: Allocator<N, DimSum<DimDiff<D1, D2>, U1>>
 {
-
     let vec = vector.len();
     let ker = kernel.len();
 
-    if vec == 0 || ker == 0 {
+    if ker == 0 || ker > vec {
-        panic!("Convolve's inputs must not be 0-sized. ");
+        panic!("convolve_valid expects `vector.len() >= kernel.len() > 0`, received {} and {} respectively.",vec,ker);
     }
 
-    if ker > vec {
+    let result_len = vector.data.shape().0.add(U1).sub(kernel.data.shape().0);
-        return convolve_valid(kernel, vector);
-    }
-    let result_len = vector.data.shape().0.sub(kernel.data.shape().0).add(U1);
     let mut conv = VectorN::zeros_generic(result_len, U1);
 
     for i in 0..(vec - ker + 1) {
@@ -117,10 +104,8 @@ where
 /// * `vector` - A Vector with size > 0
 /// * `kernel` - A Vector with size > 0
 ///
-/// This function is commutative. If kernel > vector,
+/// # Errors
-/// they will swap their roles as in
+/// Inputs must statisfy `vector.len() >= kernel.len() > 0`.
-/// (self, kernel) = (kernel,self)
-///
 pub fn convolve_same<N, D1, D2, S1, S2>(
     vector: Vector<N, D1, S1>,
     kernel: Vector<N, D2, S2>,
@@ -131,18 +116,13 @@ where
     D2: DimMax<D1, Output = DimMaximum<D1, D2>>,
     S1: Storage<N, D1>,
     S2: Storage<N, D2>,
-    DimMaximum<D1, D2>: Dim,
     DefaultAllocator: Allocator<N, DimMaximum<D1, D2>>,
 {
     let vec = vector.len();
     let ker = kernel.len();
 
-    if vec == 0 || ker == 0 {
+    if ker == 0 || ker > vec {
-        panic!("Convolve's inputs must not be 0-sized. ");
+        panic!("convolve_same expects `vector.len() >= kernel.len() > 0`, received {} and {} respectively.",vec,ker);
-    }
-
-    if ker > vec {
-        return convolve_same(kernel, vector);
     }
 
     let result_len = vector.data.shape().0.max(kernel.data.shape().0);

tests/linalg/convolution.rs

@@ -1,7 +1,6 @@
-#[allow(unused_imports)] // remove after fixing unit test
 use na::linalg::{convolve_full,convolve_valid,convolve_same};
-#[allow(unused_imports)]
 use na::{Vector2,Vector3,Vector4,Vector5,DVector};
+use std::panic;
 
 //
 // Should mimic calculations in Python's scipy library
@@ -12,70 +11,110 @@ use na::{Vector2,Vector3,Vector4,Vector5,DVector};
 // array([ 1,  4,  7, 10])
 #[test]
 fn convolve_same_check(){
-    let vec_s = Vector4::new(1.0,2.0,3.0,4.0);
+    // Static Tests
-    let ker_s = Vector2::new(1.0,2.0);
-
     let actual_s =  Vector4::from_vec(vec![1.0,4.0,7.0,10.0]);
-
+    let expected_s = convolve_same(Vector4::new(1.0,2.0,3.0,4.0), Vector2::new(1.0,2.0));
-    let expected_s = convolve_same(vec_s,ker_s);
-    let expected_s_r = convolve_same(ker_s,vec_s);
 
     assert!(relative_eq!(actual_s, expected_s, epsilon = 1.0e-7));
-    assert!(relative_eq!(actual_s, expected_s_r, epsilon = 1.0e-7));
-       
-    let vec_d = DVector::from_vec(4,vec![1.0,2.0,3.0,4.0]);
-    let ker_d = DVector::from_vec(2,vec![1.0,2.0]);
 
-    let actual_d =  DVector::from_vec(4,vec![1.0,4.0,7.0,10.0]);
+    // Dynamic Tests
-
+    let actual_d =  DVector::from_vec(vec![1.0,4.0,7.0,10.0]);
-    let expected_d = convolve_same(vec_d.clone(),ker_d.clone());
+    let expected_d = convolve_same(DVector::from_vec(vec![1.0,2.0,3.0,4.0]),DVector::from_vec(vec![1.0,2.0]));
-    let expected_d_r = convolve_same(ker_d,vec_d);
 
     assert!(relative_eq!(actual_d, expected_d, epsilon = 1.0e-7));
-    assert!(relative_eq!(actual_d, expected_d_r, epsilon = 1.0e-7));
+
+    // Panic Tests
+    // These really only apply to dynamic sized vectors
+    assert!(
+        panic::catch_unwind(|| {
+            convolve_same(DVector::from_vec(vec![1.0,2.0]), DVector::from_vec(vec![1.0,2.0,3.0,4.0]));
+        }).is_err()
+    );
+
+    assert!(
+        panic::catch_unwind(|| {
+            convolve_same(DVector::<f32>::from_vec(vec![]), DVector::from_vec(vec![1.0,2.0,3.0,4.0]));
+        }).is_err()
+    );
+
+    assert!(
+        panic::catch_unwind(|| {
+            convolve_same(DVector::from_vec(vec![1.0,2.0,3.0,4.0]),DVector::<f32>::from_vec(vec![]));
+        }).is_err()
+    );
 }
 
 // >>> convolve([1,2,3,4],[1,2],"full")
 // array([ 1, 4,  7, 10, 8])
 #[test]
 fn convolve_full_check(){
-    let vec_s = Vector4::new(1.0,2.0,3.0,4.0);
+    // Static Tests
-    let ker_s = Vector2::new(1.0,2.0);
-
     let actual_s =  Vector5::new(1.0,4.0,7.0,10.0,8.0);
-
+    let expected_s = convolve_full(Vector4::new(1.0,2.0,3.0,4.0), Vector2::new(1.0,2.0));
-    let expected_s = convolve_full(vec_s,ker_s);
-    let expected_s_r = convolve_full(ker_s,vec_s);
 
     assert!(relative_eq!(actual_s, expected_s, epsilon = 1.0e-7));
-    assert!(relative_eq!(actual_s, expected_s_r, epsilon = 1.0e-7));
-    
-    let vec_d = DVector::from_vec(4,vec![1.0,2.0,3.0,4.0]);
-    let ker_d = DVector::from_vec(2,vec![1.0,2.0]);
 
-    let actual_d =  DVector::from_vec(5,vec![1.0,4.0,7.0,10.0,8.0]);
+    // Dynamic Tests
-
+    let actual_d =  DVector::from_vec(vec![1.0,4.0,7.0,10.0,8.0]);
-    let expected_d = convolve_full(vec_d.clone(),ker_d.clone());
+    let expected_d = convolve_full(DVector::from_vec(vec![1.0,2.0,3.0,4.0]), DVector::from_vec(vec![1.0,2.0]));
-    let expected_d_r = convolve_full(ker_d,vec_d);
 
     assert!(relative_eq!(actual_d, expected_d, epsilon = 1.0e-7));
-    assert!(relative_eq!(actual_d, expected_d_r, epsilon = 1.0e-7));
+
+    // Panic Tests
+    // These really only apply to dynamic sized vectors
+    assert!(
+        panic::catch_unwind(|| {
+            convolve_full(DVector::from_vec(vec![1.0,2.0]), DVector::from_vec(vec![1.0,2.0,3.0,4.0]));
+        }).is_err()
+    );
+
+    assert!(
+        panic::catch_unwind(|| {
+            convolve_full(DVector::<f32>::from_vec(vec![]), DVector::from_vec(vec![1.0,2.0,3.0,4.0]));
+        }).is_err()
+    );
+
+    assert!(
+        panic::catch_unwind(|| {
+            convolve_full(DVector::from_vec(vec![1.0,2.0,3.0,4.0]),DVector::<f32>::from_vec(vec![]));
+        }).is_err()
+    );
 }
 
 // >>> convolve([1,2,3,4],[1,2],"valid")
 // array([ 4,  7, 10])
-// #[test]
+#[test]
-// fn convolve_valid_check(){
+fn convolve_valid_check(){
-//     let vec = Vector4::new(1.0,2.0,3.0,4.0);
+    // Static Tests
-//     let ker = Vector2::new(1.0,2.0);
+    let actual_s =  Vector3::from_vec(vec![4.0,7.0,10.0]);
+    let expected_s = convolve_valid( Vector4::new(1.0,2.0,3.0,4.0), Vector2::new(1.0,2.0));
 
-//     let actual =  Vector3::from_vec(vec![4.0,7.0,10.0]);
+    assert!(relative_eq!(actual_s, expected_s, epsilon = 1.0e-7));
 
-//     let expected1 = convolve_valid(vec, ker);
+    // Dynamic Tests
-//     let expected2 = convolve_valid(ker, vec);
+    let actual_d =  DVector::from_vec(vec![4.0,7.0,10.0]);
+    let expected_d = convolve_valid(DVector::from_vec(vec![1.0,2.0,3.0,4.0]), DVector::from_vec(vec![1.0,2.0]));
 
+    assert!(relative_eq!(actual_d, expected_d, epsilon = 1.0e-7));
 
-//     assert!(relative_eq!(actual, expected1, epsilon = 1.0e-7));
+    // Panic Tests
-//     assert!(relative_eq!(actual, expected2, epsilon = 1.0e-7));
+    // These really only apply to dynamic sized vectors
+    assert!(
+        panic::catch_unwind(|| {
+            convolve_valid(DVector::from_vec(vec![1.0,2.0]), DVector::from_vec(vec![1.0,2.0,3.0,4.0]));
+        }).is_err()
+    );
 
-// }
+    assert!(
+        panic::catch_unwind(|| {
+            convolve_valid(DVector::<f32>::from_vec(vec![]), DVector::from_vec(vec![1.0,2.0,3.0,4.0]));
+        }).is_err()
+    );
+
+    assert!(
+        panic::catch_unwind(|| {
+            convolve_valid(DVector::from_vec(vec![1.0,2.0,3.0,4.0]),DVector::<f32>::from_vec(vec![]));
+        }).is_err()
+    );
+
+}