-
Notifications
You must be signed in to change notification settings - Fork 864
Description
Eigen recently released v5.0.0 (and v3.4.1) after four years of no official releases: https://gitlab.com/libeigen/eigen/-/releases/5.0.0
From my experience of adding Eigen v5 support to roughly 100 projects for Conan Kiln, it did not introduce any significant breaking changes in its API, at least as far as compile-time is concerned. Only a couple of projects encountered non-trivial incompatibilities (i.e. anything besides a missing include or an older default C++ standard being used). GTSAM happens to be one of these few.
GTSAM compilation fails with the following errors when v5 is used:
In file included from gtsam/base/Vector.cpp:20:
gtsam/base/Vector.h:46:82: error: conversion from ‘CwiseNullaryOp<Eigen::internal::scalar_zero_op<double>,[...]>’ to non-scalar type ‘CwiseNullaryOp<Eigen::internal::scalar_constant_op<double>,[...]>’ requested
46 | static const Eigen::MatrixBase<Vector2>::ConstantReturnType Z_2x1 = Vector2::Zero();
| ~~~~~~~~~~~~~^~
gtsam/base/Vector.h:47:82: error: conversion from ‘CwiseNullaryOp<Eigen::internal::scalar_zero_op<double>,[...]>’ to non-scalar type ‘CwiseNullaryOp<Eigen::internal::scalar_constant_op<double>,[...]>’ requested
47 | static const Eigen::MatrixBase<Vector3>::ConstantReturnType Z_3x1 = Vector3::Zero();
| ~~~~~~~~~~~~~^~
gtsam/base/Vector.h:53:92: error: conversion from ‘CwiseNullaryOp<Eigen::internal::scalar_zero_op<double>,[...]>’ to non-scalar type ‘CwiseNullaryOp<Eigen::internal::scalar_constant_op<double>,[...]>’ requested
53 | static const Eigen::MatrixBase<Vector##N>::ConstantReturnType Z_##N##x1 = Vector##N::Zero();
| ~~~~~~~~~~~~~~~^~
gtsam/base/Vector.h:55:1: note: in expansion of macro ‘GTSAM_MAKE_VECTOR_DEFS’
55 | GTSAM_MAKE_VECTOR_DEFS(4)
| ^~~~~~~~~~~~~~~~~~~~~~
... etc ...
The relevant change in Eigen causing this is https://gitlab.com/libeigen/eigen/-/merge_requests/1779
Two valid fixes that I can see that keep the code mostly as-is are:
- Replace
::Zero()
withConstant(0.0)
. - Replace
ConstantReturnType
withZeroReturnType
conditionally on#if EIGEN_MAJOR_VERSION >= 5
.
I would personally go with the latter as it enables some minor additional optimizations (the use of fill_n
and memset
for initialization) as mentioned in the merge request comment.
Related to #2265.