[mlpack-git] master: * modified PlainSVD module to return normalized frobenius norm * modified PlainSVD tests * added row_col_iterator operator-- tests (2ed0c9f)

[gitdub at big.cc.gt.atl.ga.us]

Repository : https://github.com/mlpack/mlpack

On branch  : master
Link       : https://github.com/mlpack/mlpack/compare/904762495c039e345beba14c1142fd719b3bd50e...f94823c800ad6f7266995c700b1b630d5ffdcf40

>---------------------------------------------------------------

commit 2ed0c9f8a083fc89c920d423c461f2979ee7b2f6
Author: sumedhghaisas <sumedhghaisas at gmail.com>
Date:   Wed Aug 6 21:11:20 2014 +0000

    * modified PlainSVD module to return normalized frobenius norm
    * modified PlainSVD tests
    * added row_col_iterator operator-- tests


>---------------------------------------------------------------

2ed0c9f8a083fc89c920d423c461f2979ee7b2f6
 src/mlpack/core/arma_extend/Mat_extra_meat.hpp |  4 ++--
 src/mlpack/methods/cf/plain_svd.cpp            | 30 ++------------------------
 src/mlpack/tests/arma_extend_test.cpp          | 23 ++++++++++++++++++++
 src/mlpack/tests/plain_svd_test.cpp            | 25 +++++++++++++--------
 4 files changed, 43 insertions(+), 39 deletions(-)

diff --git a/src/mlpack/core/arma_extend/Mat_extra_meat.hpp b/src/mlpack/core/arma_extend/Mat_extra_meat.hpp
index ac5242d..0bf1f6c 100644
--- a/src/mlpack/core/arma_extend/Mat_extra_meat.hpp
+++ b/src/mlpack/core/arma_extend/Mat_extra_meat.hpp
@@ -88,12 +88,12 @@ template<typename eT>
 inline typename Mat<eT>::const_row_col_iterator&
 Mat<eT>::const_row_col_iterator::operator--()
     {
-  if(internal_row != 0)
+  if(internal_row > 0)
     {
     current_pos--;
     internal_row--;
     }
-  else if(internal_col != 0)
+  else if(internal_col > 0)
     {
     current_pos--;
     internal_col--;
diff --git a/src/mlpack/methods/cf/plain_svd.cpp b/src/mlpack/methods/cf/plain_svd.cpp
index 8495fe9..1ea65c2 100644
--- a/src/mlpack/methods/cf/plain_svd.cpp
+++ b/src/mlpack/methods/cf/plain_svd.cpp
@@ -18,20 +18,7 @@ double PlainSVD::Apply(const arma::mat& V,
 
   arma::mat V_rec = W * sigma * arma::trans(H);
 
-  size_t n = V.n_rows;
-  size_t m = V.n_cols;
-  double sum = 0;
-  for(size_t i = 0;i < n;i++)
-  {
-    for(size_t j = 0;j < m;j++)
-    {
-      double temp = V(i, j);
-      temp = (temp - V_rec(i, j));
-      temp = temp * temp;
-      sum += temp;
-    }
-  }
-  return sqrt(sum / (n * m));
+  return arma::norm(V - V_rec, "fro") / arma::norm(V, "fro");
 }
 
 double PlainSVD::Apply(const arma::mat& V,
@@ -60,18 +47,5 @@ double PlainSVD::Apply(const arma::mat& V,
 
   arma::mat V_rec = W * H;
 
-  size_t n = V.n_rows;
-  size_t m = V.n_cols;
-  double sum = 0;
-  for(size_t i = 0;i < n;i++)
-  {
-    for(size_t j = 0;j < m;j++)
-    {
-      double temp = V(i, j);
-      temp = (temp - V_rec(i, j));
-      temp = temp * temp;
-      sum += temp;
-    }
-  }
-  return sqrt(sum / (n * m));
+  return arma::norm(V - V_rec, "fro") / arma::norm(V, "fro");
 }
diff --git a/src/mlpack/tests/arma_extend_test.cpp b/src/mlpack/tests/arma_extend_test.cpp
index b3c6c69..67c8cd9 100644
--- a/src/mlpack/tests/arma_extend_test.cpp
+++ b/src/mlpack/tests/arma_extend_test.cpp
@@ -145,6 +145,29 @@ BOOST_AUTO_TEST_CASE(RowColIteratorTest)
   it = X.begin_row(0);
 }
 
+/**
+ * Operator-- test for mat::row_col_iterator and mat::const_row_col_iterator
+ */
+BOOST_AUTO_TEST_CASE(MatRowColIteratorDecrementOperatorTest)
+{
+  mat test = ones<mat>(5, 5);
+  
+  mat::row_col_iterator it1 = test.begin_row_col();
+  mat::row_col_iterator it2 = it1;
+  
+  // check that postfix-- does not decrement the position when position is pointing
+  // to the begining
+  it2--;
+  BOOST_REQUIRE_EQUAL(it1.row(), it2.row());
+  BOOST_REQUIRE_EQUAL(it1.col(), it2.col());
+  
+  // check that prefix-- does not decrement the position when position is pointing
+  // to the begining
+  --it2;
+  BOOST_REQUIRE_EQUAL(it1.row(), it2.row());
+  BOOST_REQUIRE_EQUAL(it1.col(), it2.col());
+}
+
 // These tests don't work when the sparse iterators hold references and not
 // pointers internally because of the lack of default constructor.
 #if ARMA_VERSION_MAJOR > 4 || \
diff --git a/src/mlpack/tests/plain_svd_test.cpp b/src/mlpack/tests/plain_svd_test.cpp
index b6577e3..bb73221 100644
--- a/src/mlpack/tests/plain_svd_test.cpp
+++ b/src/mlpack/tests/plain_svd_test.cpp
@@ -16,29 +16,36 @@ using namespace arma;
  */
 BOOST_AUTO_TEST_CASE(PlainSVDNormalFactorizationTest)
 {
-  mlpack::math::RandomSeed(10);
-  mat test = randu<mat>(5,4);
+  mat test = randu<mat>(20, 20);
 
   PlainSVD svd;
   arma::mat W, H, sigma;
   double result = svd.Apply(test, W, sigma, H);
   
-  BOOST_REQUIRE_LT(result, 1e-15);
+  BOOST_REQUIRE_LT(result, 0.01);
+  
+  test = randu<mat>(50, 50);
+  result = svd.Apply(test, W, sigma, H);
+  
+  BOOST_REQUIRE_LT(result, 0.01);
 }
 
 /**
- * Test PlainSVD as wrapper for CF.
+ * Test PlainSVD for low rank matrix factorization
  */
-BOOST_AUTO_TEST_CASE(PlainSVDCFWrapperTest)
+BOOST_AUTO_TEST_CASE(PlainSVDLowRankFactorizationTest)
 {
-  mlpack::math::RandomSeed(10);
-  mat test = randu<mat>(5,4);
+  mat W_t = randu<mat>(30, 3);
+  mat H_t = randu<mat>(3, 40);
+  
+  mat test = W_t * H_t;
 
   PlainSVD svd;
-  mat W, H;
+  arma::mat W, H;
   double result = svd.Apply(test, 3, W, H);
   
-  BOOST_REQUIRE_LT(result, 0.1);
+  BOOST_REQUIRE_LT(result, 0.01);
 }
 
+
 BOOST_AUTO_TEST_SUITE_END();