Fix compatibility for Eigen 5.0.0

[cybaol]

No description provided.

[valgur]

Here's the approach I went with for a GTSAM Conan recipe, which keeps the original type:

--- a/gtsam/base/Vector.h
+++ b/gtsam/base/Vector.h
@@ -43,14 +43,14 @@
 typedef Eigen::Vector2d Vector2;
 typedef Eigen::Vector3d Vector3;
 
-static const Eigen::MatrixBase<Vector2>::ConstantReturnType Z_2x1 = Vector2::Zero();
-static const Eigen::MatrixBase<Vector3>::ConstantReturnType Z_3x1 = Vector3::Zero();
+static const Eigen::MatrixBase<Vector2>::ConstantReturnType Z_2x1 = Vector2::Constant(0.0);
+static const Eigen::MatrixBase<Vector3>::ConstantReturnType Z_3x1 = Vector3::Constant(0.0);
 
 // Create handy typedefs and constants for vectors with N>3
 // VectorN and Z_Nx1, for N=1..9
 #define GTSAM_MAKE_VECTOR_DEFS(N)                \
   using Vector##N = Eigen::Matrix<double, N, 1>; \
-  static const Eigen::MatrixBase<Vector##N>::ConstantReturnType Z_##N##x1 = Vector##N::Zero();
+  static const Eigen::MatrixBase<Vector##N>::ConstantReturnType Z_##N##x1 = Vector##N::Constant(0.0);
 
 GTSAM_MAKE_VECTOR_DEFS(4)
 GTSAM_MAKE_VECTOR_DEFS(5)
--- a/gtsam/base/Matrix.h
+++ b/gtsam/base/Matrix.h
@@ -53,7 +53,7 @@
 using Matrix8##N = Eigen::Matrix<double, 8, N>;  \
 using Matrix9##N = Eigen::Matrix<double, 9, N>;  \
 static const Eigen::MatrixBase<Matrix##N>::IdentityReturnType I_##N##x##N = Matrix##N::Identity(); \
-static const Eigen::MatrixBase<Matrix##N>::ConstantReturnType Z_##N##x##N = Matrix##N::Zero();
+static const Eigen::MatrixBase<Matrix##N>::ConstantReturnType Z_##N##x##N = Matrix##N::Constant(0.0);
 
 GTSAM_MAKE_MATRIX_DEFS(1)
 GTSAM_MAKE_MATRIX_DEFS(2)

I'm not saying it's necessarily more correct. I'm not familiar with why ConstantReturnType was needed for these types in the first place.

[cybaol]

@valgur I guess the original reason was to introduce lazy evaluation, but for such small matrices the compiler can optimize it away directly (through inlining, constant propagation, etc.). In modern compilers, this overhead is completely negligible. From my test results, it seems that in Eigen 5.0.0 it has already been optimized into zero overhead.

[dellaert]

I don't see an issue that is PR is resolving. Happy to make one for you if you can describe what problem you ran into, and what change in Eigen 5 necessitated this.

Yeah, the reason for the return type was to have Eigen expression templates get rid of multiplying with identity explicitly, etc.

Also, on the return type: you say in Eigen 5 these things are optimized out, but what about older versions? Could it be that in order to accommodate Eigen 5 we actually slow down people with older Eigen versions (e.g., the one supplied with GTSAM)? In which case we might want to think about brabching on Eigen version, or some solution that works for both old and new.

[cybaol]

I have patched @valgur's solution with no semantic changes. While maintaining compatibility with both Eigen 3.x and 5.x.

[cybaol]

@dellaert In Eigen 3.x, Matrix##N::Zero() returns ConstantReturnType while in Eigen 5.x returns ZeroReturnType.

In Eigen 3.x

template<typename Derived>
EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE const typename DenseBase<Derived>::ConstantReturnType
DenseBase<Derived>::Zero(Index size)
{
  return Constant(size, Scalar(0));
}

And In Eigen 5.x

template <typename Derived>
EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE const typename DenseBase<Derived>::ZeroReturnType DenseBase<Derived>::Zero() {
  return ZeroReturnType(RowsAtCompileTime, ColsAtCompileTime);
}

[dellaert]

OK, CI passes, so I'll merge this solution for now.

[dellaert]

Looks good, many thanks!!!