Eigen version issue - Githubissues

oleg-alexandrov commented 3 years ago

The CERES solver is quite particular about Eigen. If a conda forge recipe for CERES is built with a certain version of Eigen, and my conda environment has a different version of Eigen, then when I build a package which depends on CERES it will fail.

Here is the source of the problem: https://github.com/ceres-solver/ceres-solver/blob/ec4f2995bbde911d6861fb5c9bb7353ad796e02b/cmake/CeresConfig.cmake.in#L200

The only solution I can think of is for conda packages for CERES to exist for multiple versions of Eigen, and each such conda package should make it very clear what version of Eigen it will work with. As of now however meta.yml has no version of Eigen, which makes the CERES conda package install well but then things fail later.

Tobias-Fischer commented 3 years ago

Indeed we're running into issues when building cartographer at the moment: https://github.com/conda-forge/cartographer-feedstock/pull/20/checks?check_run_id=3629013766

Although we have a run_export, an incompatible version of eigen is pulled in. Any ideas why that is @conda-forge/ceres-solver @wolfv @traversaro?

traversaro commented 3 years ago

Although we have a run_export, an incompatible version of eigen is pulled in. Any ideas why that is @conda-forge/ceres-solver @wolfv @traversaro?

The pin_compatible directive works fine, i.e. if you open the info/run_exports.json file in the ceres package you find:

{"weak": ["ceres-solver >=2.0.0,<2.1.0a0", "eigen >=3.3.9,<3.3.10.0a0"]}

However, this will only ensure that any package that depends on ceres will have eigen >=3.3.9,<3.3.10.0a0 as run dependency, it does not enforces anything on the version to be installed in an environment in which only ceres is installed, such as the build environment of cartographer. Not sure if this can be considered a bug of how pin_compatible works, or we should simply also have similar constriants with eigen as run dependency of ceres.

However, more in general I am bit puzzled by this check of ceres. In all the conda-forge-related packaging efforts we have being doing for robotics, we always implicitly assumed to be all Eigen 3.* release to never introduce ABI incompatibilities with respect to earlier versions. Note that with "ABI incompatibilities" I mean the ABI of downstream compiled libraries that use Eigen in their public interface, as Eigen by itself does not have an ABI as it is an header-only library. This is probably going to be a big problem if an ABI-incompatible Eigen is going to be released (Eigen 4?), but for the time being never gave problems, at least as I am able to remember. If this is not true, and instead Eigen is ABI incompatible even at the patch level, this is really a ticking bomb. However, I tried to find the exact problem that lead to add this check in ceres, and I was able to only find these commits:

https://github.com/ceres-solver/ceres-solver/commit/78cc2c4719a68fea2597fce6346cb79443536367 This is a large CMake refactor, that however "silently" (i.e. no mention of it in the commit message) adds the EXACT option to find_package(Eigen ...), effectively requiring Eigen version used to build downstream projects to be exactly the one used by Ceres.
https://github.com/ceres-solver/ceres-solver/commit/3527c0e32446fde59f96948b6282d357185fec33 This commit does not change the check, but clarify the error message. However, no specific detail on which Eigen version have ABI incompatibility (beside a reference to ODR violations) are given.

Tobias-Fischer commented 3 years ago

Thanks for your analysis, @traversaro!

From reading what you say, I suggest there are two ways forward: 1) Removing the check in ceres-solver that enforces the exact same version of Eigen 2) Adding the constraint on the exact patch level of eigen to the run dependencies of ceres-solver

Considering that - as you said - in all other robotics related packages we assume that Eigen 3.* does not introduce ABI incompatibilities, my gut feeling is that we should go with 1).

Thoughts?

traversaro commented 3 years ago

1) at least is a way to understand if the check is actually necessary or not. The nice thing is that they mention ODR-related problems, so any related problem should be at compilation level instead of runtime, so easier to detect. So unless someone has objections, it seems that 1) could be a good idea.

oleg-alexandrov commented 3 years ago

Indeed, all Eigen versions are rather compatible and option 1 seems to be the best. Option 2 would be a little terrible, as for large projects it can result in conflicts. If option 2 is implemented there needs to be released one version of ceres for each version of Eigen.

jschueller commented 3 years ago

or just bump ceres package build number when eigen package is upgraded, that's what I did recently

traversaro commented 3 years ago

or just bump ceres package build number when eigen package is upgraded, that's what I did recently

Indeed, this can be done easily and does not prevent to do everything else discussed in this issue later, let's do it while we wait for feedback on this issue: https://github.com/conda-forge/ceres-solver-feedstock/pull/22 .

traversaro commented 2 years ago

Indeed in https://github.com/conda-forge/dartsim-feedstock/pull/16 we experienced a problem that seems to be related to Eigen ABI (even if it appears only on Windows):

When dartsim v6.11.0 is built with Eigen v3.4.0 and fcl v0.6.1 (that is build with Eigen v3.3.9), some tests fails. As soon as Eigen is downgraded to v3.3.9, the test pass instead.

traversaro commented 1 year ago

Possible fix: https://github.com/conda-forge/eigen-feedstock/pull/41 .

conda-forge / ceres-solver-feedstock

Eigen version issue #19