Repository navigation
Upgrade to Eigen 5.0.0 #143
Description
Activity
Thanks for opening the issue; I also saw the note about 5.0.0 being really 3.5.0 in their old scheme. All makes sense.
I have my hands a little full with an ongoing large RcppArmadillo transition plus something new for Rcpp so if someone wants to lead this I would welcome it. I am sure we can coordinate it. Going to a minimum of C++14 should be viable: CRAN is also asking maintainers to remove C++11 requirements where possible. But there are always some old(er) packages...
RcppEigen has a few homegrown patches since the very early days that need to be carried forward but that is hopefully not too onerous.
Reacted by Johan LarssonI noticed that there was also a 3.4.1 version released simultaneously with various bug fixes (https://libeigen.gitlab.io/news/eigen_3.4.1_released/). I'm not sure if it's worth to upgrade to it first, which I guess should be quite straight-forward in comparison.
I just upgraded one of my packages to Eigen 5.0.0 and it required a couple of compatibility work-arounds to get to work. For instance the move from
Eigen::alltoEigen::placeholders::all, which I am guessing will be a common issue for many of the packages.I am more or less a placeholder maintainer here as Doug moved on to all things Julia a long time ago and I do not use Eigen in my own projects so this is always a bit of a re-learning curve ... but part of the "fun" is that we have a few local patches that need to be reapplied. See this directory for them and the howToDiff file.
I agree with you that going from 3.4.0 to 3.4.1 is probably a good idea before we tackle 3.5.0 aka 5.0.0. If you have some spare cycles, do you want to tackle trying to create a 3.4.1 branch? I am sure we can count on @yixuan (who has been instrumental in the past for version updates) and @jaganmn for additional sets of eyes and tips.
Getting 5.0.0 into CRAN should be possible but transitions (as Debian calls it when a change in package or library requires a coordinated 'wave' of updates) are work. I have been on a long one for (Rcpp)Armadillo and its move to C++14 for a while now (which should conclude soon as CRAN imposed a Deadline on a number of affected packages...). It requires a bit of planning, reverse dependency checking, PR / patch creation and care. Entirely doable. But new package work comes first, of course.
Hi all, I'm a bit tied up this month, but may take a look some time in December. There are also users interested in the impact of Eigen 5.0.0 on my own Spectra package, e.g., yixuan/spectra#183.
Reacted by Dirk Eddelbuettel and Johan LarssonI would prefer not to take the reins on this, at least not at the moment. But I'd be happy to help out with all the various patches that I suspect will need to be submitted to the various dependents once we start the upgrade process.
I had a little bit of idle time and gave it a go -- new branch is here. It passes tests here, and passed the GitHub Actions (as can be expected, we only run Linux which I use locally too). Next up would be some tire-kicking and maybe seeing what r-universe gets (as it also run macOS and windows).
PS I added the branch to my r-universe setup to source and binaries for linux, macos, windows, ... are here. It passes all tests at r-universe which is a good first step.
PPS But it looks like I butchered something in the CholMod support as eg
lme4falls over :-/PPPS I got that sorted out, at least temporarily, but using the previous version of CholmodSupport.h plus a mini-fix. Using the current one is a TODO. We do however have a repurcussion with stan headers so we will need help from the stan team.
A very large number of reverse dependencies fail, and many/most/all(?) fall over StanHeaders. So paging @bgoodri : can you (or someone on the Stan team) take a look? What we have is in the branch listed in the previous comment, i.e. here as well as in this r-universe build off the branch which I dubbed 0.4.9.9-0. It's not super urgent -- we were slow reacting to the Eigen 5.0.0 release late summer -- but it would be nice to remain current.
One of the errors we see it e.g.
In file included from /usr/local/lib/R/site-library/RcppEigen/include/Eigen/src/Core/ArrayBase.h:98, from /usr/local/lib/R/site-library/RcppEigen/include/Eigen/Core:333: /home/dirk/tmp/lib/StanHeaders/include/stan/math/prim/eigen_plugins.h:51:3: error: ISO C++ forbids declaration of ‘EIGEN_EMPTY_STRUCT_CTOR’ with no type [-Wtemplate-body] 51 | EIGEN_EMPTY_STRUCT_CTOR(val_Op); | ^~~~~~~~~~~~~~~~~~~~~~~ /home/dirk/tmp/lib/StanHeaders/include/stan/math/prim/eigen_plugins.h:117:3: error: ISO C++ forbids declaration of ‘EIGEN_EMPTY_STRUCT_CTOR’ with no type [-Wtemplate-body] 117 | EIGEN_EMPTY_STRUCT_CTOR(d_Op); | ^~~~~~~~~~~~~~~~~~~~~~~ /home/dirk/tmp/lib/StanHeaders/include/stan/math/prim/eigen_plugins.h:148:3: error: ISO C++ forbids declaration of ‘EIGEN_EMPTY_STRUCT_CTOR’ with no type [-Wtemplate-body] 148 | EIGEN_EMPTY_STRUCT_CTOR(adj_Op); | ^~~~~~~~~~~~~~~~~~~~~~~ /home/dirk/tmp/lib/StanHeaders/include/stan/math/prim/eigen_plugins.h:193:3: error: ISO C++ forbids declaration of ‘EIGEN_EMPTY_STRUCT_CTOR’ with no type [-Wtemplate-body] 193 | EIGEN_EMPTY_STRUCT_CTOR(vi_Op); | ^~~~~~~~~~~~~~~~~~~~~~~I am on it, but I seem to recall @brianward or @SteveBronder saying that we have to patch the Eigen that Stan uses (outside of R) to fix some their bugs but Eigen hasn't upstreamed those yet in a release. If so, then maybe RcppEigen would want to take those or else I can place the patched Eigen files higher up the include path when rstan compiles them like we used to occasionally do to supersede bugs in Boost that BH inherited.
Reacted by Dirk EddelbuettelHi Ben et al, and lovely to hear from you! We can surely try to coordinate for stan use with / without R / CRAN. So far I only tried to get RcppEigen to the new release (and that wasn't all that hard, I could probably also try their current master, or a fork with fixes if that last approach is best for us). Let me know how I can help. What I made so far is in the branch, and the branch corresponds to what 'my' (i.e.
eddelbuettel, as opposed torcppcorewhich has the release) r-universe has.FYI, I had pivoted the r-universe build briefly to another branch to ensure it built correctly under all settings. That being the case, I now merged that branch as #145 (giving us much better OpenMP support out of the box) and rebased so we should be back in business with r-universe under my handle being a test release of this Eigen 5.0.0 branch.
PS And I marked this as 0.4.9.9-1 to make it distinct from what we had before as 0.4.9.9-0.
Hi @bgoodri and @SteveBronder: anything new? Did you get a chance to take a good look? Feasible? Big task / small task?
I suspect it is not a big task. It seems Eigen 5.0 removed the
EIGEN_EMPTY_STRUCT_CTORmacro that Stan has been using in stan/math/prim/eigen_plugins.h . There is a good chance StanHeaders can just put those lines into an#ifdefdepending on the Eigen version, but it would be good if @SteveBronder or @brianward could confirm that is unlikely to break Stan in ways I cannot anticipate.I meant @WardBrian (sorry @brianward) .
Based on my reading of the Eigen removal, simply removing the
EIGEN_EMPTY_STRUCT_CTORlines from stan-math is probably reasonable, since I don't think we support gcc 4.9 any longer.No clue if there would be other issues, I haven't had the chance to try the full 5.0 release.
Your guess is that we can just delete those
EIGEN_EMPTY_STRUCT_CTORlines and that won't cause problems under the old Eigen either, as long as a sane compiler is used?Reacted by Brian Ward78 remaining items
@bgoodri Can you clarify the situation with StanHeaders / rstan ? Do we need another update at CRAN ? When I tested the current RcppEigen release candidate I still ran into a number of run-time errors that seem to suggest we need more work. (There are also a number of relatively simple compile-time issues I will try to tackle one by one.)
These segfaults are caused by the new memset fast-path which needed patching, discussed a little earlier in the thread: #143 (comment)
@SteveBronder what's the best minimal patch for this?
Reacted by Dirk Eddelbuettel@SteveBronder Any news or insights? I am chipping away furiously at PRs for reverse dependencies that need small (often simple) updates for Eigen 5.0.1. It would be terrific if we could cover this. The Fill.h code does no longer look like #143 (comment) so this may take some digging. Can you help?
@eddelbuettel sorry for my delay.
Re #143 (comment) :
Looking at your issue 3019 I am little confused: it made one of their tests fail and they still released? Or it is a fail in 5.0.1 and wasn't in 5.0.0? I'm lost.
The unit test they had was built to catch something that it did not actually catch so they just missed it. If you bring in the commit below that should fix the memset segfault. This is fixed on the main branch of Eigen.
https://gitlab.com/libeigen/eigen/-/commit/e246f9cb68d07e8f15c60a2404ac9a625c349223
I'm confused though, I thought we imported all of the changes Stan math needed to RcppEigen at one point? Am I hallucinating that? The 3 commits below are the bug patches in Eigen that Stan needed to pass its full test suite. I tried reading through this thread but it was kind of hard to follow if these changes were brought in or not.
stan-dev/math@d241d92
stan-dev/math@ac2a7a9
stan-dev/math@79b617bThese were also brought in as patches on Eigen's main branch
https://gitlab.com/libeigen/eigen/-/commit/e246f9cb68d07e8f15c60a2404ac9a625c349223
https://gitlab.com/libeigen/eigen/-/commit/43a01f06ad52a2e07e8b522eb7106bdde5d8bb5c@SteveBronder No worries we will get this sorted and it is getting closer.
Things were a little commingled as I recall. What we have now in RcppEigen is
- Eigen 5.0.1 pretty much as is plus needed R / CRAN changes
- You had mentioned 'something something Eigen 5.0.2' at some point ...
- ... so it could be those changes are not in our 5.0.1 base
So right now rstan / StanHeaders is the odd man out. I have spent this week preparing some fourty or so PRs for everthing that failed w.r.t. Eigen 5.0.1 that was compile-time related so it sidesteps these rstan / StanHeaders issue.
It would be truly valuable if someone (you ?) could start from the Eigen 5.0.1 we have in RcppEigen 0.4.9.9-2 and creates a clean and documented set of patches so that we have something to re-apply for later Eigen releases that may / may not have that in their release. Application and motivation-wise speaking, I think that ball is over in the corner of 'team STAN' and I would greatly appreciate any assistance in getting us closer to releasing this on CRAN.
PS:
I tried reading through this thread but it was kind of hard to follow if these changes were brought in or not.
Yes. The thread is a mess. The repo for RcppEigen should be much easier to discern.
That sounds good. I will import the commits in Eigen above to patches for RcppEigen
Reacted by Dirk EddelbuettelGreat. We can start one by one. The first one you mention https://gitlab.com/libeigen/eigen/-/commit/e246f9cb68d07e8f15c60a2404ac9a625c349223 lays a foundation via a trait likely missing in my repo (of "just Eigen 5.0.1", more or less) and the others build on it. So maybe an updated RcppEigen is likely the next step, and then (if needed) another StanHeaders. More next week....
@SteveBronder On a lurch, first step
- RcppEigen as in the repo, plus the gitlab patch against Fill.h: ❌ as
baggrstill segfaults in tests
To be continued but I'll more away from keyboard than at keyboard the coming days.
- RcppEigen as in the repo, plus the gitlab patch against Fill.h: ❌ as
@SteveBronder @bgoodri @andrjohns I may have a fix! Though you may want to refine this. Read on ...
In short, the comprises two parts:
- the gitlab patch against Fill.h in (Rcpp)Eigen, along
- a one-line change in
stan/math/rev/core/Eigen_NumTraits.hppsettingRequireInitialization = 1,(diff below)
This was found by the same model I used during the wave of fourty-some PRs I created this week, namely DeepSeek V4 Flash-0731. You may have access to more powerful ones. But I basically told it to examine the
baggrsegfault, and to focus on RcppEigen and StanHeaders, notbaggritself. I validated it by runningR CMD checkagainst birdie which also segfaulted -- and now passes.I believe you have some other (pending ?) patching related to
NumTraitsso maybe this can be achieved without flipping the constant as I did here.The diff follows.
--- /tmp/Eigen_NumTraits.hpp.bak 2026-10-02 01:44:49.784997766 +0000 +++ stan/math/rev/core/Eigen_NumTraits.hpp 2026-10-02 01:44:49.786284935 +0000 @@ -59,7 +59,7 @@ /** * stan::math::var does not require initialization. */ - RequireInitialization = 0, + RequireInitialization = 1, /** * Twice the cost of copying a double.
a one-line change in stan/math/rev/core/Eigen_NumTraits.hpp setting RequireInitialization = 1, (diff below)
That was done in Stan math for update to Eigen 5.0+ @andrjohns did you port the stan math changes I did in stan-dev/math#3271 to StanHeaders?
Also Dirk I'm traveling today, but should have time to do the patches either later in the afternoon or Monday at the latest.
Reacted by Dirk EddelbuettelIf the
RequireInitialization = 1was supposed to be included in the StanHeaders 2.39.1 on CRAN, then I must have missed it. But I can do a .2 release any time.Reacted by Dirk EddelbuettelNo rush, was AFK all day too. Looks likewheels are in motion and maybe that fix is just what we need.
Hi @SteveBronder @bgoodri @andrjohns : So what worked for me is
- small patch suggested by @SteveBronder and currently in (now committed / pushed) branch fix/stanheaders
- the 'one character change' of setting
RequireInitializationto1as described above
It would be terrific if you could confirm this / ship a
StanHeaderswith it. It feels that we are quite close.@eddelbuettel See the PR below which has the two patches from Eigen that Stan needed to compile correctly.
@bgoodri the stan math changes for Eigen 5.0.1 are all in this commit stan-dev/math@d6833b8 . Did this get ported to StanHeaders?
@SteveBronder Great. I streamlined this a little (the PR probably wanted to be against the branch already containing one of its two commits) and brought that to the RcppEigen main branch. I will increment the micro version. With that RcppEigen should be ready.
@bgoodri I will await word from you about when
StanHeaderson CRAN hasRequireInitialization = 1;as needed, or which ever equivalent changes you deem best. You should be able to base all your tests on the main branch ofRcppEigen, and you should have corresonding R-universe builds in an hour or two.
Eigen 5.0.0 was released recently. They changed versioning scheme, so the version bump is not as dramatic as it first seems.
Here's a list of the breaking changes. I suspect the requirement on C++14 may be the most problematic among these.