stan-dev / math

The Stan Math Library is a C++ template library for automatic differentiation of any order using forward, reverse, and mixed modes. It includes a range of built-in functions for probabilistic modeling, linear algebra, and equation solving.
https://mc-stan.org
BSD 3-Clause "New" or "Revised" License
750 stars 189 forks source link

update to Eigen 3.3.9 #2235

Closed spinkney closed 3 years ago

spinkney commented 3 years ago

I know there was an update not too long ago but there seems to be plenty of bug fixes since. See the changelog.

rok-cesnovar commented 3 years ago

Thanks!

We need to wait for 3.3.8 to also be available in RcppEigen because that is what rstan uses and we want to be in-sync. We could also just upgrade as:

I asked the Rcppeigen devs on the timeline and if any of us can help: https://github.com/RcppCore/RcppEigen/issues/91

bgoodri commented 3 years ago

There is a build failure with Eigen 3.3.9 (and possibly 3.3.8). I get

In file included from /home/ben/r-devel/library/StanHeaders/include/stan/math/rev/core/matrix_vari.hpp:4,
                 from /home/ben/r-devel/library/StanHeaders/include/stan/math/rev/core.hpp:16,
                 from ./stan/model/model_base.hpp:5,
                 from Module.cpp:2:
/home/ben/r-devel/library/StanHeaders/include/stan/math/rev/mat/fun/Eigen_NumTraits.hpp:200:77: error: wrong number of template arguments (8, should be 9)
                                      RhsStorageOrder, ConjugateRhs, ColMajor> {
                                                                             ^
rok-cesnovar commented 3 years ago

This is with Stan Headers set to develop Stan Math or Stan Math 3.4.0? Or current CRAN StanHeaders?

bgoodri commented 3 years ago

Current everything on CRAN except drat RcppEigen 3.3.9. I should have probably included more of the error message:

In file included from /home/ben/r-devel/library/StanHeaders/include/stan/math/rev/core/matrix_vari.hpp:4,
                 from /home/ben/r-devel/library/StanHeaders/include/stan/math/rev/core.hpp:16,
                 from ./stan/model/model_base.hpp:5,
                 from Module.cpp:2:
/home/ben/r-devel/library/StanHeaders/include/stan/math/rev/mat/fun/Eigen_NumTraits.hpp:200:77: error: wrong number of template arguments (8, should be 9)
                                      RhsStorageOrder, ConjugateRhs, ColMajor> {
                                                                             ^
In file included from /home/ben/r-devel/library/RcppEigen/include/Eigen/Core:454,
                 from /home/ben/r-devel/library/RcppEigen/include/Eigen/Dense:1,
                 from /home/ben/r-devel/library/StanHeaders/include/stan/math/prim/mat/fun/Eigen.hpp:13,
                 from /home/ben/r-devel/library/StanHeaders/include/stan/math/prim/mat/meta/append_return_type.hpp:4,
                 from /home/ben/r-devel/library/StanHeaders/include/stan/math/prim/meta.hpp:9,
                 from /home/ben/r-devel/library/StanHeaders/include/stan/math/memory/stack_alloc.hpp:7,
                 from /home/ben/r-devel/library/StanHeaders/include/stan/math/rev/core/autodiffstackstorage.hpp:4,
                 from /home/ben/r-devel/library/StanHeaders/include/stan/math/rev/core.hpp:4,
                 from ./stan/model/model_base.hpp:5,
                 from Module.cpp:2:
/home/ben/r-devel/library/RcppEigen/include/Eigen/src/Core/util/BlasUtil.h:35:8: note: provided for ‘template<class Index, class LhsScalar, int LhsStorageOrder, bool ConjugateLhs, class Rh$
 struct general_matrix_matrix_product;
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
bgoodri commented 3 years ago

It seems that an emergency StanHeaders will have to uploaded to CRAN that branches on EIGEN_MINOR_VERSION in order to preserve compatibility with all the existing CRAN packages and pave the way for RcppEigen to upgrade to 3.3.9. Do we at least understand what we have to do with the 8 vs. 9 template arguments to the NumTraits thing?