New user self-registration is disabled due to spam. Please email eigen-core-team @ lists.tuxfamily.org if you need an account.
Before reporting a bug, please make sure that your Eigen version is up-to-date!
Bug 1664 - dot product with single column block fails with new static checks
Summary: dot product with single column block fails with new static checks
Status: RESOLVED FIXED
Alias: None
Product: Eigen
Classification: Unclassified
Component: Core - matrix products (show other bugs)
Version: 3.4 (development)
Hardware: All All
: High Compilation Problem
Assignee: Nobody
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 3.4
  Show dependency treegraph
 
Reported: 2019-01-18 19:14 UTC by Rasmus Munk Larsen
Modified: 2019-01-18 20:13 UTC (History)
3 users (show)



Attachments

Description Rasmus Munk Larsen 2019-01-18 19:14:42 UTC
The recent changes to static checking causes the following code

Eigen::Vector4f pl_t; 
Eigen::Vector3f fp_c;
...
pl_f (3) = -fp_c.dot (pl_f.block (0, 0, 3, 1));    

to fail with 

./third_party/eigen3/Eigen/src/Core/Dot.h:75:3: error: static_assert failed due to requirement 'Block<Eigen::Matrix<float, 4, 1, 0, 4, 1>, -1, -1, false>::IsVectorAtCompileTime' "YOU_TRIED_CALLING_A_VECTOR_METHOD_ON_A_MATRIX"
  EIGEN_STATIC_ASSERT_VECTOR_ONLY(OtherDerived)
  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
./third_party/eigen3/Eigen/src/Core/util/StaticAssert.h:142:3: note: expanded from macro 'EIGEN_STATIC_ASSERT_VECTOR_ONLY'
  EIGEN_STATIC_ASSERT(TYPE::IsVectorAtCompileTime, \
  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
./third_party/eigen3/Eigen/src/Core/util/StaticAssert.h:33:40: note: expanded from macro 'EIGEN_STATIC_ASSERT'
    #define EIGEN_STATIC_ASSERT(X,MSG) static_assert(X,#MSG);
                                       ^             ~
third_party/pcl/filters/include/pcl/filters/impl/frustum_culling.hpp:113:20: note: in instantiation of function template specialization 'Eigen::MatrixBase<Eigen::Matrix<float, 3, 1, 0, 3, 1> >::dot<Eigen::Block<Eigen::Matrix<float, 4, 1, 0, 4, 1>, -1, -1, false> >' requested here
  pl_f (3) = -fp_c.dot (pl_f.block (0, 0, 3, 1));                    // perpendicular edges of the far plane
Comment 1 Rasmus Munk Larsen 2019-01-18 19:21:00 UTC
It would probably be better to write this as

pl_f (3) = -fp_c.dot (pl_f.head(3));    

but it still seems like the check it tailing to see that this block is indeed a vector.
Comment 2 Christoph Hertzberg 2019-01-18 20:00:19 UTC
For the record, this is the breaking commit:
https://bitbucket.org/eigen/eigen/commits/3aaba77a3cc919b61

Honestly, I would prefer forcing users to use the correct block operation here (i.e., head, tail or segment in this case).

To avoid breaking code, we could extent the current `dot_nocheck` struct to have it test (in another template parameter) whether the inputs are both vectors at compile time and deprecate the variant where that is not the case.

There are however other possibilities that the EIGEN_STATIC_ASSERT_VECTOR_ONLY will fail at other places (just grep for that macro or for IsVectorAtCompileTime in our source).

Another alternative would be to relax the assertion of the macro if a certain compile-time flag is set (similar to the Eigen2 compatibility macros we had when Eigen3 started)
Comment 3 Christoph Hertzberg 2019-01-18 20:04:54 UTC
To explain why this check is actually valid: 
It prevents for example from accidentally using `pl_f.block(0,0,3,0)`
Comment 4 Rasmus Munk Larsen 2019-01-18 20:13:05 UTC
I agree. Alternatively people could use block<3,1>(0,0). I'll fix the code in question.

Note You need to log in before you can comment on or make changes to this bug.