From e445f5085a65ca14cba2f57efb285173429ff576 Mon Sep 17 00:00:00 2001 From: Benoit Jacob Date: Sat, 13 Oct 2007 16:56:24 +0000 Subject: [PATCH] implement the first _real_ unit-tests, testing the results for correctness instead of just checking compilation. Fix the many issues discovered by these unit-tests, by the way fixing a performance bug. --- doc/example.cpp | 16 ++++----- src/Core.h | 1 + src/Core/Column.h | 4 +-- src/Core/CopyHelper.h | 2 +- src/Core/Dot.h | 10 +++--- src/Core/Fuzzy.h | 9 ++--- src/Core/Numeric.h | 14 ++++---- src/Core/Object.h | 20 +++++++++-- src/Core/Random.h | 2 +- src/Core/Row.h | 4 +-- test/CMakeLists.txt | 4 +-- test/basicstuff.cpp | 83 +++++++++++++++++++++++++++++++++++++++++++ test/main.h | 55 +--------------------------- test/matrixmanip.cpp | 53 --------------------------- test/matrixops.cpp | 74 -------------------------------------- test/vectorops.cpp | 62 -------------------------------- 16 files changed, 132 insertions(+), 281 deletions(-) create mode 100644 test/basicstuff.cpp delete mode 100644 test/matrixmanip.cpp delete mode 100644 test/matrixops.cpp delete mode 100644 test/vectorops.cpp diff --git a/doc/example.cpp b/doc/example.cpp index b5582f006..5384c2e4f 100644 --- a/doc/example.cpp +++ b/doc/example.cpp @@ -7,24 +7,20 @@ using namespace std; template void foo(const Eigen::Object& m) { - cout << "Here's m:" << endl << m << endl; + cout << "Here's m:" << endl << m << endl; } template Eigen::ScalarMultiple twice(const Eigen::Object& m) { - return 2 * m; + return 2 * m; } int main(int, char**) { - Matrix2d m; - m(0,0)= 1; - m(1,0)= 2; - m(0,1)= 3; - m(1,1)= 4; - foo(m); - foo(twice(m)); - return 0; + Matrix2d m = Matrix2d::random(); + foo(m); + foo(twice(m)); + return 0; } diff --git a/src/Core.h b/src/Core.h index 3e14e1ad0..d0e0c05ec 100644 --- a/src/Core.h +++ b/src/Core.h @@ -27,5 +27,6 @@ namespace Eigen { #include "Core/Trace.h" #include "Core/Dot.h" #include "Core/Random.h" +#include "Core/Fuzzy.h" } // namespace Eigen diff --git a/src/Core/Column.h b/src/Core/Column.h index 5b9107619..5367c39f1 100644 --- a/src/Core/Column.h +++ b/src/Core/Column.h @@ -75,9 +75,9 @@ template class Column template Column -Object::col(int i) +Object::col(int i) const { - return Column(static_cast(this)->ref(), i); + return Column(static_cast(const_cast(this))->ref(), i); } #endif // EI_COLUMN_H diff --git a/src/Core/CopyHelper.h b/src/Core/CopyHelper.h index 8b15aef8c..b62f0aaf0 100644 --- a/src/Core/CopyHelper.h +++ b/src/Core/CopyHelper.h @@ -40,7 +40,7 @@ template struct CopyHelperUnroller } }; -template struct CopyHelperUnroller<0, Rows> +template struct CopyHelperUnroller<1, Rows> { template static void run(Derived1 &dst, const Derived2 &src) diff --git a/src/Core/Dot.h b/src/Core/Dot.h index 3bf8514ba..5f0fed8b5 100644 --- a/src/Core/Dot.h +++ b/src/Core/Dot.h @@ -32,7 +32,7 @@ struct DotUnroller static void run(const Derived1 &v1, const Derived2& v2, typename Derived1::Scalar &dot) { DotUnroller::run(v1, v2, dot); - dot += v1[Index] * Conj(v2[Index]); + dot += v1[Index] * NumTraits::conj(v2[Index]); } }; @@ -41,7 +41,7 @@ struct DotUnroller<0, Size, Derived1, Derived2> { static void run(const Derived1 &v1, const Derived2& v2, typename Derived1::Scalar &dot) { - dot = v1[0] * Conj(v2[0]); + dot = v1[0] * NumTraits::conj(v2[0]); } }; @@ -67,9 +67,9 @@ Scalar Object::dot(const OtherDerived& other) const ::run(*static_cast(this), other, res); else { - res = (*this)[0] * Conj(other[0]); + res = (*this)[0] * NumTraits::conj(other[0]); for(int i = 1; i < size(); i++) - res += (*this)[i]* Conj(other[i]); + res += (*this)[i]* NumTraits::conj(other[i]); } return res; } @@ -78,7 +78,7 @@ template typename NumTraits::Real Object::norm2() const { assert(IsVector); - return Real(dot(*this)); + return NumTraits::real(dot(*this)); } template diff --git a/src/Core/Fuzzy.h b/src/Core/Fuzzy.h index 4980de8d1..ed995dc8d 100644 --- a/src/Core/Fuzzy.h +++ b/src/Core/Fuzzy.h @@ -40,7 +40,8 @@ bool Object::isApprox( else { for(int i = 0; i < cols(); i++) - if(!col(i).isApprox(other.col(i), prec)) + if((col(i) - other.col(i)).norm2() + > std::min(col(i).norm2(), other.col(i).norm2()) * prec * prec) return false; return true; } @@ -59,7 +60,7 @@ bool Object::isMuchSmallerThan( else { for(int i = 0; i < cols(); i++) - if(!col(i).isMuchSmallerThan(other, prec)) + if(col(i).norm2() > NumTraits::abs2(other) * prec * prec) return false; return true; } @@ -79,10 +80,10 @@ bool Object::isMuchSmallerThan( else { for(int i = 0; i < cols(); i++) - if(!col(i).isMuchSmallerThan(other.col(i), prec)) + if(col(i).norm2() > other.col(i).norm2() * prec * prec) return false; return true; } } -#endif // EI_FUZZY_H \ No newline at end of file +#endif // EI_FUZZY_H diff --git a/src/Core/Numeric.h b/src/Core/Numeric.h index 04721e019..3ef3615cd 100644 --- a/src/Core/Numeric.h +++ b/src/Core/Numeric.h @@ -45,10 +45,10 @@ template<> struct NumTraits static double sqrt(const int& x) { return std::sqrt(static_cast(x)); } static int abs(const int& x) { return std::abs(x); } static int abs2(const int& x) { return x*x; } - static int rand() + static int random() { - // "rand() % n" is bad, they say, because the low-order bits are not random enough. - // However here, 21 is odd, so rand() % 21 uses the high-order bits + // "random() % n" is bad, they say, because the low-order bits are not random enough. + // However here, 21 is odd, so random() % 21 uses the high-order bits // as well, so there's no problem. return (std::rand() % 21) - 10; } @@ -86,7 +86,7 @@ template<> struct NumTraits static float sqrt(const float& x) { return std::sqrt(x); } static float abs(const float& x) { return std::abs(x); } static float abs2(const float& x) { return x*x; } - static float rand() + static float random() { return std::rand() / (RAND_MAX/20.0f) - 10.0f; } @@ -120,7 +120,7 @@ template<> struct NumTraits static double sqrt(const double& x) { return std::sqrt(x); } static double abs(const double& x) { return std::abs(x); } static double abs2(const double& x) { return x*x; } - static double rand() + static double random() { return std::rand() / (RAND_MAX/20.0) - 10.0; } @@ -158,9 +158,9 @@ template struct NumTraits > { return std::abs(static_cast(x)); } static Real abs2(const Complex& x) { return std::real(x) * std::real(x) + std::imag(x) * std::imag(x); } - static Complex rand() + static Complex random() { - return Complex(NumTraits::rand(), NumTraits::rand()); + return Complex(NumTraits::random(), NumTraits::random()); } static bool isMuchSmallerThan(const Complex& a, const Complex& b, const Real& prec = precision()) { diff --git a/src/Core/Object.h b/src/Core/Object.h index bfcdafd60..e53b5be49 100644 --- a/src/Core/Object.h +++ b/src/Core/Object.h @@ -34,6 +34,20 @@ template class Object template void _copy_helper(const Object& other); + template + bool _isApprox_helper( + const OtherDerived& other, + const typename NumTraits::Real& prec = NumTraits::precision() + ) const; + bool _isMuchSmallerThan_helper( + const Scalar& other, + const typename NumTraits::Real& prec = NumTraits::precision() + ) const; + template + bool _isMuchSmallerThan_helper( + const Object& other, + const typename NumTraits::Real& prec = NumTraits::precision() + ) const; public: static const int SizeAtCompileTime = RowsAtCompileTime == Dynamic || ColsAtCompileTime == Dynamic @@ -81,8 +95,8 @@ template class Object return *static_cast(this); } - Row row(int i); - Column col(int i); + Row row(int i) const; + Column col(int i) const; Minor minor(int row, int col); Block block(int startRow, int endRow, int startCol, int endCol); Transpose transpose(); @@ -111,7 +125,7 @@ template class Object ) const; template bool isMuchSmallerThan( - const OtherDerived& other, + const Object& other, const typename NumTraits::Real& prec = NumTraits::precision() ) const; diff --git a/src/Core/Random.h b/src/Core/Random.h index a90d69d5b..872ed6878 100644 --- a/src/Core/Random.h +++ b/src/Core/Random.h @@ -51,7 +51,7 @@ template class Random { EI_UNUSED(row); EI_UNUSED(col); - return NumTraits::rand(); + return NumTraits::random(); } protected: diff --git a/src/Core/Row.h b/src/Core/Row.h index 102890438..7b8a8a9b4 100644 --- a/src/Core/Row.h +++ b/src/Core/Row.h @@ -80,9 +80,9 @@ template class Row template Row -Object::row(int i) +Object::row(int i) const { - return Row(static_cast(this)->ref(), i); + return Row(static_cast(const_cast(this))->ref(), i); } #endif // EI_ROW_H diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 58a088eb3..21a4de5f7 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -6,9 +6,7 @@ INCLUDE_DIRECTORIES( ${QT_INCLUDE_DIR} ) SET(test_SRCS main.cpp - vectorops.cpp - matrixops.cpp - matrixmanip.cpp + basicstuff.cpp ) QT4_AUTOMOC(${test_SRCS}) diff --git a/test/basicstuff.cpp b/test/basicstuff.cpp new file mode 100644 index 000000000..345a9f3ad --- /dev/null +++ b/test/basicstuff.cpp @@ -0,0 +1,83 @@ +// This file is part of Eigen, a lightweight C++ template library +// for linear algebra. Eigen itself is part of the KDE project. +// +// Copyright (C) 2006-2007 Benoit Jacob +// +// Eigen is free software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the Free Software +// Foundation; either version 2 or (at your option) any later version. +// +// Eigen is distributed in the hope that it will be useful, but WITHOUT ANY +// WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS +// FOR A PARTICULAR PURPOSE. See the GNU General Public License for more +// details. +// +// You should have received a copy of the GNU General Public License along +// with Eigen; if not, write to the Free Software Foundation, Inc., 51 +// Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. +// +// As a special exception, if other files instantiate templates or use macros +// or functions from this file, or you compile this file and link it +// with other works to produce a work based on this file, this file does not +// by itself cause the resulting work to be covered by the GNU General Public +// License. This exception does not invalidate any other reasons why a work +// based on this file might be covered by the GNU General Public License. + +#include "main.h" + +template void basicStuff(const MatrixType& m) +{ + typedef typename MatrixType::Scalar Scalar; + typedef Matrix VectorType; + int rows = m.rows(); + int cols = m.cols(); + + MatrixType m1 = MatrixType::random(rows, cols), + m2 = MatrixType::random(rows, cols), + m3; + VectorType v1 = VectorType::random(rows, 1), + v2 = VectorType::random(rows, 1); + + Scalar s1 = NumTraits::random(), + s2 = NumTraits::random(); + + QVERIFY(v1.isApprox(v1)); + QVERIFY((v1-v1).isMuchSmallerThan(v1)); + QVERIFY((v1-v1).isMuchSmallerThan(v1.norm())); + QVERIFY(m1.isApprox(m1)); + QVERIFY((m1-m1).isMuchSmallerThan(m1)); + + QVERIFY((m1+m1).isApprox(2 * m1)); + QVERIFY((m1 * s1).isApprox(s1 * m1)); + QVERIFY(((m1 + m2) * s1).isApprox(s1 * m1 + s1 * m2)); + QVERIFY(((s1 + s2) * m1).isApprox(m1 * s1 + m1 * s2)); + + + + m3 = m2; + QVERIFY((m3 += m1).isApprox(m1 + m2)); + m3 = m2; + QVERIFY((m3 -= m1).isApprox(-m1 + m2)); + m3 = m2; + QVERIFY((m3 *= s1).isApprox(s1 * m2)); + m3 = m2; + if(NumTraits::HasFloatingPoint + && s1 != static_cast(0)) + QVERIFY((m3 /= s1).isApprox(m2 / s1)); + + QVERIFY(((m1 * m1.transpose()) * m2).isApprox(m1 * (m1.transpose() * m2))); + m3 = m1; + m3 *= (m1.transpose() * m2); + QVERIFY(m3.isApprox(m1 * (m1.transpose() * m2))); + QVERIFY(m3.isApprox(m1.lazyProduct(m1.transpose() * m2))); +} + +void EigenTest::testBasicStuff() +{ + basicStuff(Matrix()); + basicStuff(Matrix, 2, 5>()); + basicStuff(Matrix, 4, 4>()); + basicStuff(MatrixXcf(3, 3)); + basicStuff(MatrixXi(8, 12)); + basicStuff(MatrixXd(20, 20)); +} diff --git a/test/main.h b/test/main.h index fcb1b23ab..3eecec210 100644 --- a/test/main.h +++ b/test/main.h @@ -44,60 +44,7 @@ class EigenTest : public QObject EigenTest(); private slots: - void testVectorOps(); - void testMatrixOps(); - void testMatrixManip(); + void testBasicStuff(); }; -template inline typename Eigen::NumTraits::Real TestEpsilon(); -template<> inline int TestEpsilon() { return 0; } -template<> inline float TestEpsilon() { return 1e-2f; } -template<> inline double TestEpsilon() { return 1e-4; } -template<> inline int TestEpsilon >() { return TestEpsilon(); } -template<> inline float TestEpsilon >() { return TestEpsilon(); } -template<> inline double TestEpsilon >() { return TestEpsilon(); } - -template bool TestMuchSmallerThan(const T& a, const T& b) -{ - return NumTraits::isMuchSmallerThan(a, b, TestEpsilon()); -} - -template -bool TestMuchSmallerThan( - const Object& a, - const Object& b) -{ - return a.isMuchSmallerThan(b, TestEpsilon()); -} - -template bool TestApprox(const T& a, const T& b) -{ - return NumTraits::isApprox(a, b, TestEpsilon()); -} - -template -bool TestApprox( - const Object& a, - const Object& b) -{ - return a.isApprox(b, TestEpsilon()); -} - -template bool TestApproxOrLessThan(const T& a, const T& b) -{ - return NumTraits::isApproxOrLessThan(a, b, TestEpsilon()); -} - -template -bool TestApproxOrLessThan( - const Object& a, - const Object& b) -{ - return a.isApproxOrLessThan(b, TestEpsilon()); -} - -#define QVERIFY_MUCH_SMALLER_THAN(a, b) QVERIFY(TestMuchSmallerThan(a, b)) -#define QVERIFY_APPROX(a, b) QVERIFY(TestApprox(a, b)) -#define QVERIFY_APPROX_OR_LESS_THAN(a, b) QVERIFY(TestApproxOrLessThan(a, b)) - #endif // EI_TEST_MAIN_H diff --git a/test/matrixmanip.cpp b/test/matrixmanip.cpp deleted file mode 100644 index eba184179..000000000 --- a/test/matrixmanip.cpp +++ /dev/null @@ -1,53 +0,0 @@ -// This file is part of Eigen, a lightweight C++ template library -// for linear algebra. Eigen itself is part of the KDE project. -// -// Copyright (C) 2006-2007 Benoit Jacob -// -// Eigen is free software; you can redistribute it and/or modify it under the -// terms of the GNU General Public License as published by the Free Software -// Foundation; either version 2 or (at your option) any later version. -// -// Eigen is distributed in the hope that it will be useful, but WITHOUT ANY -// WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS -// FOR A PARTICULAR PURPOSE. See the GNU General Public License for more -// details. -// -// You should have received a copy of the GNU General Public License along -// with Eigen; if not, write to the Free Software Foundation, Inc., 51 -// Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. -// -// As a special exception, if other files instantiate templates or use macros -// or functions from this file, or you compile this file and link it -// with other works to produce a work based on this file, this file does not -// by itself cause the resulting work to be covered by the GNU General Public -// License. This exception does not invalidate any other reasons why a work -// based on this file might be covered by the GNU General Public License. - -#include "main.h" - -template void matrixManip(const MatrixType& m) -{ - int rows = m.rows(), cols = m.cols(); - int i = rand()%rows, j = rand()%cols; - - MatrixType a(rows, cols), b(rows, cols); - a.row(i); - a.col(j); - a.minor(i, j); - a.block(1, rows-1, 1, cols-1); - a.row(i) = b.row(i); - a.row(i) += b.row(i); - a.col(j) *= 2; - a.minor(i, j) = b.block(1, rows-1, 1, cols-1); - a.minor(i, j) -= a.block(1, rows-1, 1, cols-1).eval(); -} - -void EigenTest::testMatrixManip() -{ - matrixManip(Matrix()); - matrixManip(Matrix()); - matrixManip(Matrix, 4,3>()); - matrixManip(MatrixXi(2, 2)); - matrixManip(MatrixXd(3, 5)); - matrixManip(MatrixXcf(4, 4)); -} diff --git a/test/matrixops.cpp b/test/matrixops.cpp deleted file mode 100644 index 9c0933d89..000000000 --- a/test/matrixops.cpp +++ /dev/null @@ -1,74 +0,0 @@ -// This file is part of Eigen, a lightweight C++ template library -// for linear algebra. Eigen itself is part of the KDE project. -// -// Copyright (C) 2006-2007 Benoit Jacob -// -// Eigen is free software; you can redistribute it and/or modify it under the -// terms of the GNU General Public License as published by the Free Software -// Foundation; either version 2 or (at your option) any later version. -// -// Eigen is distributed in the hope that it will be useful, but WITHOUT ANY -// WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS -// FOR A PARTICULAR PURPOSE. See the GNU General Public License for more -// details. -// -// You should have received a copy of the GNU General Public License along -// with Eigen; if not, write to the Free Software Foundation, Inc., 51 -// Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. -// -// As a special exception, if other files instantiate templates or use macros -// or functions from this file, or you compile this file and link it -// with other works to produce a work based on this file, this file does not -// by itself cause the resulting work to be covered by the GNU General Public -// License. This exception does not invalidate any other reasons why a work -// based on this file might be covered by the GNU General Public License. - -#include "main.h" - -template void matrixOps(const MatrixType1& m1, const MatrixType2& m2) -{ - typedef typename MatrixType1::Scalar Scalar; - int rows1 = m1.rows(), cols1 = m1.cols(); - int rows2 = m2.rows(), cols2 = m2.cols(); - - MatrixType1 a(rows1, cols1), b(rows1, cols1), c(b); - Scalar s; - a * s; - s * a; - a + b; - a - b; - (a + b) * s; - s * (a + b); - a + b + c; - a = b; - a = b + c; - a = s * (b - c); - a = (a + b).eval(); - a += b; - a -= b + b; - a *= s; - if(rows1 == cols1) - { - a *= b; - a.lazyProduct(b); - } - - MatrixType1 d(rows1, cols1); - MatrixType2 e(rows2, cols2); - QVERIFY( (d * e).rows() == rows1 && (d * e).cols() == cols2 ); -} - -void EigenTest::testMatrixOps() -{ - matrixOps(Matrix(), Matrix()); - matrixOps(Matrix(), Matrix()); - matrixOps(Matrix(), Matrix()); - matrixOps(Matrix, 4,3>(), Matrix, 3,4>()); - matrixOps(MatrixXf(1, 1), MatrixXf(1, 3)); - matrixOps(MatrixXi(2, 2), MatrixXi(2, 2)); - matrixOps(MatrixXd(3, 5), MatrixXd(5, 1)); - matrixOps(MatrixXcf(4, 4), MatrixXcf(4, 4)); - matrixOps(MatrixXd(3, 5), Matrix()); - matrixOps(Matrix4cf(), MatrixXcf(4, 4)); -} diff --git a/test/vectorops.cpp b/test/vectorops.cpp deleted file mode 100644 index 77a01fbcd..000000000 --- a/test/vectorops.cpp +++ /dev/null @@ -1,62 +0,0 @@ -// This file is part of Eigen, a lightweight C++ template library -// for linear algebra. Eigen itself is part of the KDE project. -// -// Copyright (C) 2006-2007 Benoit Jacob -// -// Eigen is free software; you can redistribute it and/or modify it under the -// terms of the GNU General Public License as published by the Free Software -// Foundation; either version 2 or (at your option) any later version. -// -// Eigen is distributed in the hope that it will be useful, but WITHOUT ANY -// WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS -// FOR A PARTICULAR PURPOSE. See the GNU General Public License for more -// details. -// -// You should have received a copy of the GNU General Public License along -// with Eigen; if not, write to the Free Software Foundation, Inc., 51 -// Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. -// -// As a special exception, if other files instantiate templates or use macros -// or functions from this file, or you compile this file and link it -// with other works to produce a work based on this file, this file does not -// by itself cause the resulting work to be covered by the GNU General Public -// License. This exception does not invalidate any other reasons why a work -// based on this file might be covered by the GNU General Public License. - -#include "main.h" - -template void vectorOps(const VectorType& v) -{ - typedef typename VectorType::Scalar Scalar; - int size = v.size(); - - VectorType a(size), b(size), c(b); - Scalar s; - a * s; - s * a; - a + b; - a - b; - (a + b) * s; - s * (a + b); - a + b + c; - a = b; - a = b + c; - a = s * (b - c); - a = (s * (b - c)).eval(); - - a += b; - a -= b + b; - a *= s; - a += (a + a).eval(); -} - -void EigenTest::testVectorOps() -{ - vectorOps(Vector2i()); - vectorOps(Vector3d()); - vectorOps(Vector4cf()); - vectorOps(VectorXf(1)); - vectorOps(VectorXi(2)); - vectorOps(VectorXd(3)); - vectorOps(VectorXcf(4)); -}