Merge pull request #1096 from jdumas/fix-randperm

Fix logic error for URNG in igl::randperm

include/igl/randperm.cpp

@@ -1,9 +1,9 @@
 // This file is part of libigl, a simple c++ geometry processing library.
-// 
+//
 // Copyright (C) 2013 Alec Jacobson <alecjacobson@gmail.com>
-// 
-// This Source Code Form is subject to the terms of the Mozilla Public License 
-// v. 2.0. If a copy of the MPL was not distributed with this file, You can 
+//
+// This Source Code Form is subject to the terms of the Mozilla Public License
+// v. 2.0. If a copy of the MPL was not distributed with this file, You can
 // obtain one at http://mozilla.org/MPL/2.0/.
 #include "randperm.h"
 #include "colon.h"
@@ -13,7 +13,7 @@ template <typename DerivedI, typename URBG>
 IGL_INLINE void igl::randperm(
   const int n,
   Eigen::PlainObjectBase<DerivedI> & I,
-  URBG urbg)
+  URBG && urbg)
 {
   Eigen::VectorXi II;
   igl::colon(0,1,n-1,II);
@@ -32,24 +32,42 @@ IGL_INLINE void igl::randperm(
 
 #ifdef IGL_STATIC_LIBRARY
 // Explicit template instantiation
-template void igl::randperm<Eigen::Matrix<int, -1, 1, 0, -1, 1>, std::minstd_rand0>(int, Eigen::PlainObjectBase<Eigen::Matrix<int, -1, 1, 0, -1, 1> >&, std::minstd_rand0);
-template void igl::randperm<Eigen::Matrix<int, -1, -1, 0, -1, -1>, std::minstd_rand0>(int, Eigen::PlainObjectBase<Eigen::Matrix<int, -1, -1, 0, -1, -1> >&, std::minstd_rand0);
-template void igl::randperm<Eigen::Matrix<int, -1, 1, 0, -1, 1>, std::minstd_rand>(int, Eigen::PlainObjectBase<Eigen::Matrix<int, -1, 1, 0, -1, 1> >&, std::minstd_rand);
-template void igl::randperm<Eigen::Matrix<int, -1, -1, 0, -1, -1>, std::minstd_rand>(int, Eigen::PlainObjectBase<Eigen::Matrix<int, -1, -1, 0, -1, -1> >&, std::minstd_rand);
-template void igl::randperm<Eigen::Matrix<int, -1, 1, 0, -1, 1>, std::mt19937>(int, Eigen::PlainObjectBase<Eigen::Matrix<int, -1, 1, 0, -1, 1> >&, std::mt19937);
-template void igl::randperm<Eigen::Matrix<int, -1, -1, 0, -1, -1>, std::mt19937>(int, Eigen::PlainObjectBase<Eigen::Matrix<int, -1, -1, 0, -1, -1> >&, std::mt19937);
-template void igl::randperm<Eigen::Matrix<int, -1, 1, 0, -1, 1>, std::mt19937_64>(int, Eigen::PlainObjectBase<Eigen::Matrix<int, -1, 1, 0, -1, 1> >&, std::mt19937_64);
-template void igl::randperm<Eigen::Matrix<int, -1, -1, 0, -1, -1>, std::mt19937_64>(int, Eigen::PlainObjectBase<Eigen::Matrix<int, -1, -1, 0, -1, -1> >&, std::mt19937_64);
-template void igl::randperm<Eigen::Matrix<int, -1, 1, 0, -1, 1>, std::ranlux24_base>(int, Eigen::PlainObjectBase<Eigen::Matrix<int, -1, 1, 0, -1, 1> >&, std::ranlux24_base);
-template void igl::randperm<Eigen::Matrix<int, -1, -1, 0, -1, -1>, std::ranlux24_base>(int, Eigen::PlainObjectBase<Eigen::Matrix<int, -1, -1, 0, -1, -1> >&, std::ranlux24_base);
-template void igl::randperm<Eigen::Matrix<int, -1, 1, 0, -1, 1>, std::ranlux48_base>(int, Eigen::PlainObjectBase<Eigen::Matrix<int, -1, 1, 0, -1, 1> >&, std::ranlux48_base);
-template void igl::randperm<Eigen::Matrix<int, -1, -1, 0, -1, -1>, std::ranlux48_base>(int, Eigen::PlainObjectBase<Eigen::Matrix<int, -1, -1, 0, -1, -1> >&, std::ranlux48_base);
-template void igl::randperm<Eigen::Matrix<int, -1, 1, 0, -1, 1>, std::ranlux24>(int, Eigen::PlainObjectBase<Eigen::Matrix<int, -1, 1, 0, -1, 1> >&, std::ranlux24);
-template void igl::randperm<Eigen::Matrix<int, -1, -1, 0, -1, -1>, std::ranlux24>(int, Eigen::PlainObjectBase<Eigen::Matrix<int, -1, -1, 0, -1, -1> >&, std::ranlux24);
-template void igl::randperm<Eigen::Matrix<int, -1, 1, 0, -1, 1>, std::ranlux48>(int, Eigen::PlainObjectBase<Eigen::Matrix<int, -1, 1, 0, -1, 1> >&, std::ranlux48);
-template void igl::randperm<Eigen::Matrix<int, -1, -1, 0, -1, -1>, std::ranlux48>(int, Eigen::PlainObjectBase<Eigen::Matrix<int, -1, -1, 0, -1, -1> >&, std::ranlux48);
-template void igl::randperm<Eigen::Matrix<int, -1, 1, 0, -1, 1>, std::knuth_b>(int, Eigen::PlainObjectBase<Eigen::Matrix<int, -1, 1, 0, -1, 1> >&, std::knuth_b);
-template void igl::randperm<Eigen::Matrix<int, -1, -1, 0, -1, -1>, std::knuth_b>(int, Eigen::PlainObjectBase<Eigen::Matrix<int, -1, -1, 0, -1, -1> >&, std::knuth_b);
+template void igl::randperm<Eigen::Matrix<int, -1, 1, 0, -1, 1>, std::minstd_rand0>(int, Eigen::PlainObjectBase<Eigen::Matrix<int, -1, 1, 0, -1, 1> >&, std::minstd_rand0 &&);
+template void igl::randperm<Eigen::Matrix<int, -1, 1, 0, -1, 1>, std::minstd_rand0 &>(int, Eigen::PlainObjectBase<Eigen::Matrix<int, -1, 1, 0, -1, 1> >&, std::minstd_rand0 &);
+template void igl::randperm<Eigen::Matrix<int, -1, -1, 0, -1, -1>, std::minstd_rand0>(int, Eigen::PlainObjectBase<Eigen::Matrix<int, -1, -1, 0, -1, -1> >&, std::minstd_rand0 &&);
+template void igl::randperm<Eigen::Matrix<int, -1, -1, 0, -1, -1>, std::minstd_rand0 &>(int, Eigen::PlainObjectBase<Eigen::Matrix<int, -1, -1, 0, -1, -1> >&, std::minstd_rand0 &);
+template void igl::randperm<Eigen::Matrix<int, -1, 1, 0, -1, 1>, std::minstd_rand>(int, Eigen::PlainObjectBase<Eigen::Matrix<int, -1, 1, 0, -1, 1> >&, std::minstd_rand &&);
+template void igl::randperm<Eigen::Matrix<int, -1, 1, 0, -1, 1>, std::minstd_rand &>(int, Eigen::PlainObjectBase<Eigen::Matrix<int, -1, 1, 0, -1, 1> >&, std::minstd_rand &);
+template void igl::randperm<Eigen::Matrix<int, -1, -1, 0, -1, -1>, std::minstd_rand>(int, Eigen::PlainObjectBase<Eigen::Matrix<int, -1, -1, 0, -1, -1> >&, std::minstd_rand &&);
+template void igl::randperm<Eigen::Matrix<int, -1, -1, 0, -1, -1>, std::minstd_rand &>(int, Eigen::PlainObjectBase<Eigen::Matrix<int, -1, -1, 0, -1, -1> >&, std::minstd_rand &);
+template void igl::randperm<Eigen::Matrix<int, -1, 1, 0, -1, 1>, std::mt19937>(int, Eigen::PlainObjectBase<Eigen::Matrix<int, -1, 1, 0, -1, 1> >&, std::mt19937 &&);
+template void igl::randperm<Eigen::Matrix<int, -1, 1, 0, -1, 1>, std::mt19937 &>(int, Eigen::PlainObjectBase<Eigen::Matrix<int, -1, 1, 0, -1, 1> >&, std::mt19937 &);
+template void igl::randperm<Eigen::Matrix<int, -1, -1, 0, -1, -1>, std::mt19937>(int, Eigen::PlainObjectBase<Eigen::Matrix<int, -1, -1, 0, -1, -1> >&, std::mt19937 &&);
+template void igl::randperm<Eigen::Matrix<int, -1, -1, 0, -1, -1>, std::mt19937 &>(int, Eigen::PlainObjectBase<Eigen::Matrix<int, -1, -1, 0, -1, -1> >&, std::mt19937 &);
+template void igl::randperm<Eigen::Matrix<int, -1, 1, 0, -1, 1>, std::mt19937_64>(int, Eigen::PlainObjectBase<Eigen::Matrix<int, -1, 1, 0, -1, 1> >&, std::mt19937_64 &&);
+template void igl::randperm<Eigen::Matrix<int, -1, 1, 0, -1, 1>, std::mt19937_64 &>(int, Eigen::PlainObjectBase<Eigen::Matrix<int, -1, 1, 0, -1, 1> >&, std::mt19937_64 &);
+template void igl::randperm<Eigen::Matrix<int, -1, -1, 0, -1, -1>, std::mt19937_64>(int, Eigen::PlainObjectBase<Eigen::Matrix<int, -1, -1, 0, -1, -1> >&, std::mt19937_64 &&);
+template void igl::randperm<Eigen::Matrix<int, -1, -1, 0, -1, -1>, std::mt19937_64 &>(int, Eigen::PlainObjectBase<Eigen::Matrix<int, -1, -1, 0, -1, -1> >&, std::mt19937_64 &);
+template void igl::randperm<Eigen::Matrix<int, -1, 1, 0, -1, 1>, std::ranlux24_base>(int, Eigen::PlainObjectBase<Eigen::Matrix<int, -1, 1, 0, -1, 1> >&, std::ranlux24_base &&);
+template void igl::randperm<Eigen::Matrix<int, -1, 1, 0, -1, 1>, std::ranlux24_base &>(int, Eigen::PlainObjectBase<Eigen::Matrix<int, -1, 1, 0, -1, 1> >&, std::ranlux24_base &);
+template void igl::randperm<Eigen::Matrix<int, -1, -1, 0, -1, -1>, std::ranlux24_base>(int, Eigen::PlainObjectBase<Eigen::Matrix<int, -1, -1, 0, -1, -1> >&, std::ranlux24_base &&);
+template void igl::randperm<Eigen::Matrix<int, -1, -1, 0, -1, -1>, std::ranlux24_base &>(int, Eigen::PlainObjectBase<Eigen::Matrix<int, -1, -1, 0, -1, -1> >&, std::ranlux24_base &);
+template void igl::randperm<Eigen::Matrix<int, -1, 1, 0, -1, 1>, std::ranlux48_base>(int, Eigen::PlainObjectBase<Eigen::Matrix<int, -1, 1, 0, -1, 1> >&, std::ranlux48_base &&);
+template void igl::randperm<Eigen::Matrix<int, -1, 1, 0, -1, 1>, std::ranlux48_base &>(int, Eigen::PlainObjectBase<Eigen::Matrix<int, -1, 1, 0, -1, 1> >&, std::ranlux48_base &);
+template void igl::randperm<Eigen::Matrix<int, -1, -1, 0, -1, -1>, std::ranlux48_base>(int, Eigen::PlainObjectBase<Eigen::Matrix<int, -1, -1, 0, -1, -1> >&, std::ranlux48_base &&);
+template void igl::randperm<Eigen::Matrix<int, -1, -1, 0, -1, -1>, std::ranlux48_base &>(int, Eigen::PlainObjectBase<Eigen::Matrix<int, -1, -1, 0, -1, -1> >&, std::ranlux48_base &);
+template void igl::randperm<Eigen::Matrix<int, -1, 1, 0, -1, 1>, std::ranlux24>(int, Eigen::PlainObjectBase<Eigen::Matrix<int, -1, 1, 0, -1, 1> >&, std::ranlux24 &&);
+template void igl::randperm<Eigen::Matrix<int, -1, 1, 0, -1, 1>, std::ranlux24 &>(int, Eigen::PlainObjectBase<Eigen::Matrix<int, -1, 1, 0, -1, 1> >&, std::ranlux24 &);
+template void igl::randperm<Eigen::Matrix<int, -1, -1, 0, -1, -1>, std::ranlux24>(int, Eigen::PlainObjectBase<Eigen::Matrix<int, -1, -1, 0, -1, -1> >&, std::ranlux24 &&);
+template void igl::randperm<Eigen::Matrix<int, -1, -1, 0, -1, -1>, std::ranlux24 &>(int, Eigen::PlainObjectBase<Eigen::Matrix<int, -1, -1, 0, -1, -1> >&, std::ranlux24 &);
+template void igl::randperm<Eigen::Matrix<int, -1, 1, 0, -1, 1>, std::ranlux48>(int, Eigen::PlainObjectBase<Eigen::Matrix<int, -1, 1, 0, -1, 1> >&, std::ranlux48 &&);
+template void igl::randperm<Eigen::Matrix<int, -1, 1, 0, -1, 1>, std::ranlux48 &>(int, Eigen::PlainObjectBase<Eigen::Matrix<int, -1, 1, 0, -1, 1> >&, std::ranlux48 &);
+template void igl::randperm<Eigen::Matrix<int, -1, -1, 0, -1, -1>, std::ranlux48>(int, Eigen::PlainObjectBase<Eigen::Matrix<int, -1, -1, 0, -1, -1> >&, std::ranlux48 &&);
+template void igl::randperm<Eigen::Matrix<int, -1, -1, 0, -1, -1>, std::ranlux48 &>(int, Eigen::PlainObjectBase<Eigen::Matrix<int, -1, -1, 0, -1, -1> >&, std::ranlux48 &);
+template void igl::randperm<Eigen::Matrix<int, -1, 1, 0, -1, 1>, std::knuth_b>(int, Eigen::PlainObjectBase<Eigen::Matrix<int, -1, 1, 0, -1, 1> >&, std::knuth_b &&);
+template void igl::randperm<Eigen::Matrix<int, -1, 1, 0, -1, 1>, std::knuth_b &>(int, Eigen::PlainObjectBase<Eigen::Matrix<int, -1, 1, 0, -1, 1> >&, std::knuth_b &);
+template void igl::randperm<Eigen::Matrix<int, -1, -1, 0, -1, -1>, std::knuth_b>(int, Eigen::PlainObjectBase<Eigen::Matrix<int, -1, -1, 0, -1, -1> >&, std::knuth_b &&);
+template void igl::randperm<Eigen::Matrix<int, -1, -1, 0, -1, -1>, std::knuth_b &>(int, Eigen::PlainObjectBase<Eigen::Matrix<int, -1, -1, 0, -1, -1> >&, std::knuth_b &);
 template void igl::randperm<Eigen::Matrix<int, -1, 1, 0, -1, 1>>(int, Eigen::PlainObjectBase<Eigen::Matrix<int, -1, 1, 0, -1, 1> >&);
 template void igl::randperm<Eigen::Matrix<int, -1, -1, 0, -1, -1>>(int, Eigen::PlainObjectBase<Eigen::Matrix<int, -1, -1, 0, -1, -1> >&);
 #endif

include/igl/randperm.h

@@ -1,9 +1,9 @@
 // This file is part of libigl, a simple c++ geometry processing library.
-// 
+//
 // Copyright (C) 2013 Alec Jacobson <alecjacobson@gmail.com>
-// 
-// This Source Code Form is subject to the terms of the Mozilla Public License 
-// v. 2.0. If a copy of the MPL was not distributed with this file, You can 
+//
+// This Source Code Form is subject to the terms of the Mozilla Public License
+// v. 2.0. If a copy of the MPL was not distributed with this file, You can
 // obtain one at http://mozilla.org/MPL/2.0/.
 #ifndef IGL_RANDPERM_H
 #define IGL_RANDPERM_H
@@ -26,7 +26,8 @@ namespace igl
   IGL_INLINE void randperm(
     const int n,
     Eigen::PlainObjectBase<DerivedI> & I,
-    URBG urbg);
+    URBG && urbg);
+
   template <typename DerivedI>
   IGL_INLINE void randperm(
     const int n,

tests/include/igl/randperm.cpp

@@ -34,6 +34,9 @@ namespace randperm
 
     test_common::assert_eq(I1, I2);
     test_common::assert_eq(Ix1, Ix2);
+
+    test_common::assert_neq(I1, Ix1);
+    test_common::assert_neq(I2, Ix2);
   }
 }
 

tests/test_common.h

@@ -96,6 +96,7 @@ namespace test_common
   {
     igl::readDMAT(data_path(filename), M);
   }
+
   template <typename DerivedA, typename DerivedB>
   void assert_eq(
     const Eigen::MatrixBase<DerivedA> & A,
@@ -116,6 +117,29 @@ namespace test_common
       }
     }
   }
+
+  template <typename DerivedA, typename DerivedB>
+  void assert_neq(
+    const Eigen::MatrixBase<DerivedA> & A,
+    const Eigen::MatrixBase<DerivedB> & B)
+  {
+    // Sizes should match
+    REQUIRE(A.rows() == B.rows());
+    REQUIRE(A.cols() == B.cols());
+    bool all_equals = true;
+    for(int i = 0;i<A.rows();i++)
+    {
+      for(int j = 0;j<A.cols();j++)
+      {
+        if (A(i,j) != B(i,j))
+        {
+          all_equals = false;
+        }
+      }
+    }
+    REQUIRE_FALSE(all_equals);
+  }
+
   template <typename DerivedA, typename DerivedB>
   void assert_eq(
     const Eigen::SparseMatrix<DerivedA> & A,