From 4946d758c932ea218046371cb94b15451107b957 Mon Sep 17 00:00:00 2001 From: Chris Jones Date: Fri, 20 Nov 2015 19:58:08 +0100 Subject: [PATCH 1/7] Use a class constructor to initialize CPU cache sizes Using a static instance of a class to initialize the values for the CPU cache sizes guarantees thread-safe initialization of the values when using C++11. Therefore under C++11 it is no longer necessary to call Eigen::initParallel() before calling any eigen functions on different threads. --- .../Core/products/GeneralBlockPanelKernel.h | 41 ++++++++++--------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/Eigen/src/Core/products/GeneralBlockPanelKernel.h b/Eigen/src/Core/products/GeneralBlockPanelKernel.h index 79eaa7432..94754bf66 100644 --- a/Eigen/src/Core/products/GeneralBlockPanelKernel.h +++ b/Eigen/src/Core/products/GeneralBlockPanelKernel.h @@ -36,37 +36,40 @@ const std::ptrdiff_t defaultL3CacheSize = 512*1024; #endif /** \internal */ -inline void manage_caching_sizes(Action action, std::ptrdiff_t* l1, std::ptrdiff_t* l2, std::ptrdiff_t* l3) -{ - static bool m_cache_sizes_initialized = false; - static std::ptrdiff_t m_l1CacheSize = 0; - static std::ptrdiff_t m_l2CacheSize = 0; - static std::ptrdiff_t m_l3CacheSize = 0; - - if(!m_cache_sizes_initialized) - { +struct CacheSizes { + CacheSizes(): m_l1(-1),m_l2(-1),m_l3(-1) { int l1CacheSize, l2CacheSize, l3CacheSize; queryCacheSizes(l1CacheSize, l2CacheSize, l3CacheSize); - m_l1CacheSize = manage_caching_sizes_helper(l1CacheSize, defaultL1CacheSize); - m_l2CacheSize = manage_caching_sizes_helper(l2CacheSize, defaultL2CacheSize); - m_l3CacheSize = manage_caching_sizes_helper(l3CacheSize, defaultL3CacheSize); - m_cache_sizes_initialized = true; + m_l1 = manage_caching_sizes_helper(l1CacheSize, defaultL1CacheSize); + m_l2 = manage_caching_sizes_helper(l2CacheSize, defaultL2CacheSize); + m_l3 = manage_caching_sizes_helper(l3CacheSize, defaultL3CacheSize); } + std::ptrdiff_t m_l1; + std::ptrdiff_t m_l2; + std::ptrdiff_t m_l3; +}; + + +/** \internal */ +inline void manage_caching_sizes(Action action, std::ptrdiff_t* l1, std::ptrdiff_t* l2, std::ptrdiff_t* l3) +{ + static CacheSizes m_cacheSizes; + if(action==SetAction) { // set the cpu cache size and cache all block sizes from a global cache size in byte eigen_internal_assert(l1!=0 && l2!=0); - m_l1CacheSize = *l1; - m_l2CacheSize = *l2; - m_l3CacheSize = *l3; + m_cacheSizes.m_l1 = *l1; + m_cacheSizes.m_l2 = *l2; + m_cacheSizes.m_l3 = *l3; } else if(action==GetAction) { eigen_internal_assert(l1!=0 && l2!=0); - *l1 = m_l1CacheSize; - *l2 = m_l2CacheSize; - *l3 = m_l3CacheSize; + *l1 = m_cacheSizes.m_l1; + *l2 = m_cacheSizes.m_l2; + *l3 = m_cacheSizes.m_l3; } else { From b265979a70ba06f4d4e8ba737d5d16bfd5c27ea3 Mon Sep 17 00:00:00 2001 From: Gael Guennebaud Date: Sat, 21 Nov 2015 15:03:04 +0100 Subject: [PATCH 2/7] Make FullPivLU::solve use rank() instead of nonzeroPivots(). --- Eigen/src/LU/FullPivLU.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Eigen/src/LU/FullPivLU.h b/Eigen/src/LU/FullPivLU.h index 07a87cbc6..498df8adc 100644 --- a/Eigen/src/LU/FullPivLU.h +++ b/Eigen/src/LU/FullPivLU.h @@ -720,7 +720,7 @@ void FullPivLU<_MatrixType>::_solve_impl(const RhsType &rhs, DstType &dst) const const Index rows = this->rows(), cols = this->cols(), - nonzero_pivots = this->nonzeroPivots(); + nonzero_pivots = this->rank(); eigen_assert(rhs.rows() == rows); const Index smalldim = (std::min)(rows, cols); From 35c17a3fc8f61471590a4cf8376a6aa530e2e4bc Mon Sep 17 00:00:00 2001 From: Gael Guennebaud Date: Sun, 22 Nov 2015 22:09:57 +0100 Subject: [PATCH 3/7] Use overload instead of template full specialization to please old MSVC --- Eigen/src/Core/MathFunctions.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Eigen/src/Core/MathFunctions.h b/Eigen/src/Core/MathFunctions.h index fe1dafe83..48cf565fb 100644 --- a/Eigen/src/Core/MathFunctions.h +++ b/Eigen/src/Core/MathFunctions.h @@ -748,13 +748,13 @@ template EIGEN_DEVICE_FUNC bool isinf_msvc_helper(T x) } //MSVC defines a _isnan builtin function, but for double only -template<> EIGEN_DEVICE_FUNC inline bool isnan_impl(const long double& x) { return _isnan(x); } -template<> EIGEN_DEVICE_FUNC inline bool isnan_impl(const double& x) { return _isnan(x); } -template<> EIGEN_DEVICE_FUNC inline bool isnan_impl(const float& x) { return _isnan(x); } +EIGEN_DEVICE_FUNC inline bool isnan_impl(const long double& x) { return _isnan(x); } +EIGEN_DEVICE_FUNC inline bool isnan_impl(const double& x) { return _isnan(x); } +EIGEN_DEVICE_FUNC inline bool isnan_impl(const float& x) { return _isnan(x); } -template<> EIGEN_DEVICE_FUNC inline bool isinf_impl(const long double& x) { return isinf_msvc_helper(x); } -template<> EIGEN_DEVICE_FUNC inline bool isinf_impl(const double& x) { return isinf_msvc_helper(x); } -template<> EIGEN_DEVICE_FUNC inline bool isinf_impl(const float& x) { return isinf_msvc_helper(x); } +EIGEN_DEVICE_FUNC inline bool isinf_impl(const long double& x) { return isinf_msvc_helper(x); } +EIGEN_DEVICE_FUNC inline bool isinf_impl(const double& x) { return isinf_msvc_helper(x); } +EIGEN_DEVICE_FUNC inline bool isinf_impl(const float& x) { return isinf_msvc_helper(x); } #elif (defined __FINITE_MATH_ONLY__ && __FINITE_MATH_ONLY__ && EIGEN_COMP_GNUC) From 8a2659f0cb2570c55d39036104860c656c9d3096 Mon Sep 17 00:00:00 2001 From: Gael Guennebaud Date: Mon, 23 Nov 2015 10:53:55 +0100 Subject: [PATCH 4/7] Improve numerical robustness of some unit tests --- test/geo_transformations.cpp | 35 ++++++++++++++++++++++------------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/test/geo_transformations.cpp b/test/geo_transformations.cpp index d50c7c76a..94ed155ef 100644 --- a/test/geo_transformations.cpp +++ b/test/geo_transformations.cpp @@ -12,6 +12,12 @@ #include #include +template +Matrix angleToVec(T a) +{ + return Matrix(std::cos(a), std::sin(a)); +} + template void non_projective_only() { /* this test covers the following files: @@ -130,14 +136,16 @@ template void transformations() AngleAxisx aa = AngleAxisx(q1); VERIFY_IS_APPROX(q1 * v1, Quaternionx(aa) * v1); - if(abs(aa.angle()) > NumTraits::dummy_precision()) + // The following test is stable only if 2*angle != angle and v1 is not colinear with axis + if( (abs(aa.angle()) > test_precision()) && (abs(aa.axis().dot(v1.normalized()))<(Scalar(1)-Scalar(4)*test_precision())) ) { VERIFY( !(q1 * v1).isApprox(Quaternionx(AngleAxisx(aa.angle()*2,aa.axis())) * v1) ); } aa.fromRotationMatrix(aa.toRotationMatrix()); VERIFY_IS_APPROX(q1 * v1, Quaternionx(aa) * v1); - if(abs(aa.angle()) > NumTraits::dummy_precision()) + // The following test is stable only if 2*angle != angle and v1 is not colinear with axis + if( (abs(aa.angle()) > test_precision()) && (abs(aa.axis().dot(v1.normalized()))<(Scalar(1)-Scalar(4)*test_precision())) ) { VERIFY( !(q1 * v1).isApprox(Quaternionx(AngleAxisx(aa.angle()*2,aa.axis())) * v1) ); } @@ -214,7 +222,9 @@ template void transformations() t4 *= aa3; VERIFY_IS_APPROX(t3.matrix(), t4.matrix()); - v3 = Vector3::Random(); + do { + v3 = Vector3::Random(); + } while (v3.cwiseAbs().minCoeff()::epsilon()); Translation3 tv3(v3); Transform3 t5(tv3); t4 = tv3; @@ -414,14 +424,12 @@ template void transformations() Scalar angle = internal::random(-100,100); Rotation2D rot2(angle); VERIFY( rot2.smallestPositiveAngle() >= 0 ); - VERIFY( rot2.smallestPositiveAngle() < Scalar(2)*Scalar(EIGEN_PI) ); - VERIFY_IS_APPROX( std::cos(rot2.smallestPositiveAngle()), std::cos(rot2.angle()) ); - VERIFY_IS_APPROX( std::sin(rot2.smallestPositiveAngle()), std::sin(rot2.angle()) ); + VERIFY( rot2.smallestPositiveAngle() <= Scalar(2)*Scalar(EIGEN_PI) ); + VERIFY_IS_APPROX( angleToVec(rot2.smallestPositiveAngle()), angleToVec(rot2.angle()) ); VERIFY( rot2.smallestAngle() >= -Scalar(EIGEN_PI) ); VERIFY( rot2.smallestAngle() <= Scalar(EIGEN_PI) ); - VERIFY_IS_APPROX( std::cos(rot2.smallestAngle()), std::cos(rot2.angle()) ); - VERIFY_IS_APPROX( std::sin(rot2.smallestAngle()), std::sin(rot2.angle()) ); + VERIFY_IS_APPROX( angleToVec(rot2.smallestAngle()), angleToVec(rot2.angle()) ); } s0 = internal::random(-100,100); @@ -437,7 +445,7 @@ template void transformations() VERIFY_IS_APPROX(t20,t21); VERIFY_IS_APPROX(s0, (R0.slerp(0, R1)).angle()); - VERIFY_IS_APPROX(R1.smallestPositiveAngle(), (R0.slerp(1, R1)).smallestPositiveAngle()); + VERIFY_IS_APPROX( angleToVec(R1.smallestPositiveAngle()), angleToVec((R0.slerp(1, R1)).smallestPositiveAngle()) ); VERIFY_IS_APPROX(R0.smallestPositiveAngle(), (R0.slerp(0.5, R0)).smallestPositiveAngle()); if(std::cos(s0)>0) @@ -447,13 +455,14 @@ template void transformations() // Check path length Scalar l = 0; - for(int k=0; k<100; ++k) + int path_steps = 100; + for(int k=0; k::epsilon()*Scalar(path_steps/2))); // check basic features { From 31b661e4cad656611c322e7b61ad7b5e83c62207 Mon Sep 17 00:00:00 2001 From: Gael Guennebaud Date: Mon, 23 Nov 2015 13:28:43 +0100 Subject: [PATCH 5/7] Add a note on initParallel being optional in C++11. --- doc/TopicMultithreading.dox | 2 ++ 1 file changed, 2 insertions(+) diff --git a/doc/TopicMultithreading.dox b/doc/TopicMultithreading.dox index 95f6bf287..47c9b261f 100644 --- a/doc/TopicMultithreading.dox +++ b/doc/TopicMultithreading.dox @@ -43,6 +43,8 @@ int main(int argc, char** argv) } \endcode +\note With Eigen 3.3, and a fully C++11 compliant compiler (i.e., thread-safe static local variable initialization), then calling \c initParallel() is optional. + \warning note that all functions generating random matrices are \b not re-entrant nor thread-safe. Those include DenseBase::Random(), and DenseBase::setRandom() despite a call to Eigen::initParallel(). This is because these functions are based on std::rand which is not re-entrant. For thread-safe random generator, we recommend the use of boost::random or c++11 random feature. In the case your application is parallelized with OpenMP, you might want to disable Eigen's own parallization as detailed in the previous section. From f3dca16a1d4226982281dafa7a2f51c36698f6e5 Mon Sep 17 00:00:00 2001 From: Gael Guennebaud Date: Mon, 23 Nov 2015 14:07:52 +0100 Subject: [PATCH 6/7] bug #1117: workaround unused-local-typedefs warning when EIGEN_NO_STATIC_ASSERT and NDEBUG are both defined. --- Eigen/src/SparseCore/SparseCwiseBinaryOp.h | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Eigen/src/SparseCore/SparseCwiseBinaryOp.h b/Eigen/src/SparseCore/SparseCwiseBinaryOp.h index 90f702ee3..d9420ac63 100644 --- a/Eigen/src/SparseCore/SparseCwiseBinaryOp.h +++ b/Eigen/src/SparseCore/SparseCwiseBinaryOp.h @@ -39,10 +39,9 @@ class CwiseBinaryOpImpl EIGEN_SPARSE_PUBLIC_INTERFACE(Derived) CwiseBinaryOpImpl() { - typedef typename internal::traits::StorageKind LhsStorageKind; - typedef typename internal::traits::StorageKind RhsStorageKind; EIGEN_STATIC_ASSERT(( - (!internal::is_same::value) + (!internal::is_same::StorageKind, + typename internal::traits::StorageKind>::value) || ((Lhs::Flags&RowMajorBit) == (Rhs::Flags&RowMajorBit))), THE_STORAGE_ORDER_OF_BOTH_SIDES_MUST_MATCH); } From f9fff67a56656db4ca0a633b16a0896ee0fcfa77 Mon Sep 17 00:00:00 2001 From: Gael Guennebaud Date: Mon, 23 Nov 2015 15:03:24 +0100 Subject: [PATCH 7/7] Disable "decorated name length exceeded, name was truncated" MSVC warning. --- Eigen/src/Core/util/DisableStupidWarnings.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Eigen/src/Core/util/DisableStupidWarnings.h b/Eigen/src/Core/util/DisableStupidWarnings.h index 6a0bf0629..46c141ad5 100644 --- a/Eigen/src/Core/util/DisableStupidWarnings.h +++ b/Eigen/src/Core/util/DisableStupidWarnings.h @@ -10,6 +10,7 @@ // 4244 - 'argument' : conversion from 'type1' to 'type2', possible loss of data // 4273 - QtAlignedMalloc, inconsistent DLL linkage // 4324 - structure was padded due to declspec(align()) + // 4503 - decorated name length exceeded, name was truncated // 4512 - assignment operator could not be generated // 4522 - 'class' : multiple assignment operators specified // 4700 - uninitialized local variable 'xyz' used @@ -17,7 +18,7 @@ #ifndef EIGEN_PERMANENTLY_DISABLE_STUPID_WARNINGS #pragma warning( push ) #endif - #pragma warning( disable : 4100 4101 4127 4181 4211 4244 4273 4324 4512 4522 4700 4717 ) + #pragma warning( disable : 4100 4101 4127 4181 4211 4244 4273 4324 4503 4512 4522 4700 4717 ) #elif defined __INTEL_COMPILER // 2196 - routine is both "inline" and "noinline" ("noinline" assumed) // ICC 12 generates this warning even without any inline keyword, when defining class methods 'inline' i.e. inside of class body