Upgrade to 3.3

[wjakob]

Hey,

now that Eigen 3.3 stable is finally available, it would be great to upgrade this repository to the latest version.

Thanks,
Wenzel

[wjakob]

^ @schuellc @danielepanozzo

[alecjacobson]

Agreed. I'm trying to build up libigl's unit tests before switching. There are a few gotchas that could quietly introduce bugs in important places.

We should also move the Eigen submodule to be directly in libigl (external/Eigen) rather than just expect cmake to find it inside recursively submoduled in external/nanogui/

[wjakob]

Eigen will always be part of nanogui (as a recursive subrepo) since it depends on it. If you'd rather ship your own version in libigl, it would still be important to keep the versions in sync to avoid ABI issues.

[alecjacobson]

Sure. That's fine, but currently Eigen is rather hidden and it is the main dependency of libigl. Nanogui is currently a disabled-by-default, optional dependency of just the viewer.

We should of course try to sync what we can, but this highlights the issue: if we had some 3rd dependency relying on its own Eigen submodule then we'd be syncing that, too. Or it might even be out of our control.

[danielepanozzo]

@wjakob I will bump the version in the repo at some point this week so that you can use the new version for nanogui. I also gave you permission to push in there in case you need it urgently. It might take a while for us to be able to use it in libigl since it is not 100% backward compatible.

@alecjacobson We already have our own clone of nanogui (https://github.com/libigl/nanogui) exactly to be able to update the version of Eigen as we please, instead of relying on what is used in the official nanogui. Moving it from the nanogui folder into external is a pretty annoying procedure and I don't see any practical gain in doing it. Even if nanogui is optional, it still gets cloned by the recursive option.

[alecjacobson]

I'd wait on bumping up the version until we have more unit tests. The way understand the Eigen changeset, we could get unexpected changes in behavior. It also gives some motivation to everyone to start paying more attention to the unit tests.

What does the "annoying procedure" actually entail? I don't like the idea that we're bound to the version of Eigen that nanogui uses rather than the version we actually need/want to use. This only seems to unnecessarily restrict the main library. I would rather like to see it as libigl depends on a specific version of Eigen and any extra libraries in libigl must conform to that version (if it means creating a fork and moding then so bit).

It seems like we're already forking nanogui so why not just be sure that libigl/nanogui is always using a compatible version of Eigen as the main library?

An obvious downside is literally storing Eigen twice, but that's a price I'd be willing to pay.

[svenevs]

I know this one is kind of old, but the update to 3.3.3 doesn't seem correct:

eigen/Eigen/src/Core/util/Macros.h

Lines 14 to 16 in 1f05f51

	#define EIGEN_WORLD_VERSION 3
	#define EIGEN_MAJOR_VERSION 3
	#define EIGEN_MINOR_VERSION 90

Minor should be 3 not 90 if the 3.3.3 release tarball was used. The 90 seems to have come from here, but I'm not sure why that's even there...

Anyway, just thought I'd let you know, this prevents EIGEN_VERSION_AT_LEAST from behaving as expected.