upgrade to eigen 3.3.9 #2238

rok-cesnovar · 2020-12-06T18:10:22Z

Summary

RcppEigen has upgraded to 3.3.9 (they skipped 3.3.8) RcppCore/RcppEigen#92 so we should too.

Hopefully, this also helps with debugging the issue @bgoodri reported here

Changelog for 3.3.9 and 3.3.8 here: http://eigen.tuxfamily.org/index.php?title=ChangeLog#Eigen_3.3.9

Tests

/

Side Effects

Hopefully none.

Release notes

Upgraded to Eigen 3.3.9.

Checklist

Math issue update to Eigen 3.3.9 #2235
Copyright holder: Rok Češnovar

The copyright holder is typically you or your assignee, such as a university or company. By submitting this pull request, the copyright holder is agreeing to the license the submitted work under the following licenses:
- Code: BSD 3-clause (https://opensource.org/licenses/BSD-3-Clause)
- Documentation: CC-BY 4.0 (https://creativecommons.org/licenses/by/4.0/)

rok-cesnovar · 2020-12-06T18:14:48Z

Ok, the header tests are already showing the same issue @bgoodri posted so this should make it simpler to debug.

Paging resident Eigen experts @t4c1 @andrjohns @SteveBronder @bbbales2

bgoodri · 2020-12-06T18:45:17Z

It is a backwards incompatible change relating to NumTraits:

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;

rok-cesnovar · 2020-12-06T18:46:28Z

That is a bit bizzare for a patch release.

rok-cesnovar · 2020-12-06T18:55:43Z

I am not sure it helps in any way but this same error pops up with 3.3.8 as well (tried locally).

SteveBronder · 2020-12-06T19:04:18Z

Yes I saw and am pretty sure I know the fixes. I can tackle this on Monday

bgoodri · 2020-12-06T19:12:24Z

@SteveBronder If you could describe the fix, that would help me upload ASAP a StanHeaders that branches to be backwards compatible and forwards compatible with the Eigen version.

SteveBronder · 2020-12-06T19:17:05Z

Oh I think the only major thing I think is the general matrix matrix thing. That's the only major thing I saw when reading over dirks patches and the new eigen changes

bgoodri · 2020-12-06T19:20:17Z

OK, I presume Monday is fine, but we're holding up whatever is good about 3.3.8 and 3.3.9 migrating to CRAN.

…

On Sun, Dec 6, 2020 at 2:17 PM Steve Bronder ***@***.***> wrote: Oh I think the only major thing I think is the general matrix matrix thing. That's the only major thing I saw when reading over dirks patches and the new eigen changes — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2238 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAZ2XKSWUNERZXWPXNREEKLSTPKD5ANCNFSM4UPPWT7A> .

rok-cesnovar · 2020-12-06T19:21:49Z

Yes, general_matrix_matrix_product has gained a template parameter int ResInnerStride. This is internal Eigen stuff so take makes more sense why it could change in a patch release.

Setting it to 1 enables me to run a few tests. A quick grep of the eigen files shows they always set it to 1 as well.

rok-cesnovar · 2020-12-06T19:36:31Z

Adding the following

#if EIGEN_VERSION_AT_LEAST(3,3,8)
template <typename Index, int LhsStorageOrder, bool ConjugateLhs,
          int RhsStorageOrder, bool ConjugateRhs, int ResInnerStride>
struct general_matrix_matrix_product<Index, stan::math::var, LhsStorageOrder,
                                     ConjugateLhs, stan::math::var,
                                     RhsStorageOrder, ConjugateRhs, ColMajor, ResInnerStride> {
#else
template <typename Index, int LhsStorageOrder, bool ConjugateLhs,
          int RhsStorageOrder, bool ConjugateRhs>
struct general_matrix_matrix_product<Index, stan::math::var, LhsStorageOrder,
                                     ConjugateLhs, stan::math::var,
                                     RhsStorageOrder, ConjugateRhs, ColMajor> {
#endif

instead of lines 392-396 here https://github.com/stan-dev/math/blob/develop/stan/math/rev/core/Eigen_NumTraits.hpp#L392 compiles with both 3.3.7 and 3.3.9. I am guessing you need that fix for Stan Headers?

bgoodri · 2020-12-06T19:43:17Z

@rok-cesnovar Thanks. I'll put that branching into StanHeaders once our unit tests pass.

rok-cesnovar · 2020-12-06T20:05:23Z

There seems to be more stuff, unfortunately...

rok-cesnovar · 2020-12-06T21:31:21Z

With 8f4d51c this passes the full unit tests, so I think this might be it. The full test suite should be over in about 5 hours.

stan-buildbot · 2020-12-07T01:19:42Z

Name	Old Result	New Result	Ratio	Performance change( 1 - new / old )
gp_pois_regr/gp_pois_regr.stan	3.49	3.59	0.97	-3.09% slower
low_dim_corr_gauss/low_dim_corr_gauss.stan	0.02	0.02	0.99	-0.83% slower
eight_schools/eight_schools.stan	0.11	0.11	0.99	-1.0% slower
gp_regr/gp_regr.stan	0.16	0.16	1.01	0.75% faster
irt_2pl/irt_2pl.stan	5.82	5.77	1.01	0.88% faster
performance.compilation	87.12	86.15	1.01	1.11% faster
low_dim_gauss_mix_collapse/low_dim_gauss_mix_collapse.stan	8.4	8.39	1.0	0.08% faster
pkpd/one_comp_mm_elim_abs.stan	29.98	29.43	1.02	1.84% faster
sir/sir.stan	131.35	136.31	0.96	-3.77% slower
gp_regr/gen_gp_data.stan	0.04	0.04	1.0	0.05% faster
low_dim_gauss_mix/low_dim_gauss_mix.stan	2.93	2.94	0.99	-0.58% slower
pkpd/sim_one_comp_mm_elim_abs.stan	0.4	0.39	1.04	3.92% faster
arK/arK.stan	1.78	1.79	0.99	-0.53% slower
arma/arma.stan	0.73	0.7	1.04	4.21% faster
garch/garch.stan	0.61	0.64	0.96	-4.67% slower
Mean result: 0.999487088763

Jenkins Console Log
Blue Ocean
Commit hash: 8f4d51c

Machine information

ProductName: Mac OS X ProductVersion: 10.11.6 BuildVersion: 15G22010

CPU:
Intel(R) Xeon(R) CPU E5-1680 v2 @ 3.00GHz

G++:
Configured with: --prefix=/Applications/Xcode.app/Contents/Developer/usr --with-gxx-include-dir=/usr/include/c++/4.2.1
Apple LLVM version 7.0.2 (clang-700.1.81)
Target: x86_64-apple-darwin15.6.0
Thread model: posix

Clang:
Apple LLVM version 7.0.2 (clang-700.1.81)
Target: x86_64-apple-darwin15.6.0
Thread model: posix

SteveBronder · 2020-12-07T17:01:46Z

Changes look fine to me just need to update the readme and should be good

stan-buildbot · 2020-12-07T23:49:34Z

Name	Old Result	New Result	Ratio	Performance change( 1 - new / old )
gp_pois_regr/gp_pois_regr.stan	3.5	3.42	1.02	2.35% faster
low_dim_corr_gauss/low_dim_corr_gauss.stan	0.02	0.02	0.96	-4.2% slower
eight_schools/eight_schools.stan	0.11	0.11	0.99	-0.57% slower
gp_regr/gp_regr.stan	0.16	0.17	0.99	-1.51% slower
irt_2pl/irt_2pl.stan	5.89	5.78	1.02	1.79% faster
performance.compilation	87.09	86.21	1.01	1.0% faster
low_dim_gauss_mix_collapse/low_dim_gauss_mix_collapse.stan	8.44	8.41	1.0	0.35% faster
pkpd/one_comp_mm_elim_abs.stan	29.76	28.7	1.04	3.54% faster
sir/sir.stan	128.81	127.53	1.01	1.0% faster
gp_regr/gen_gp_data.stan	0.04	0.04	1.02	1.71% faster
low_dim_gauss_mix/low_dim_gauss_mix.stan	2.94	2.92	1.01	0.79% faster
pkpd/sim_one_comp_mm_elim_abs.stan	0.38	0.38	1.0	-0.31% slower
arK/arK.stan	1.78	1.79	1.0	-0.31% slower
arma/arma.stan	0.74	0.71	1.05	4.32% faster
garch/garch.stan	0.62	0.64	0.96	-4.2% slower
Mean result: 1.00440927274

Jenkins Console Log
Blue Ocean
Commit hash: 7dce138

Machine information

ProductName: Mac OS X ProductVersion: 10.11.6 BuildVersion: 15G22010

CPU:
Intel(R) Xeon(R) CPU E5-1680 v2 @ 3.00GHz

G++:
Configured with: --prefix=/Applications/Xcode.app/Contents/Developer/usr --with-gxx-include-dir=/usr/include/c++/4.2.1
Apple LLVM version 7.0.2 (clang-700.1.81)
Target: x86_64-apple-darwin15.6.0
Thread model: posix

Clang:
Apple LLVM version 7.0.2 (clang-700.1.81)
Target: x86_64-apple-darwin15.6.0
Thread model: posix

upgrade to eigen 3.3.9

95748de

rok-cesnovar force-pushed the eigen339 branch from 8539f06 to 95748de Compare December 6, 2020 18:12

rok-cesnovar force-pushed the eigen339 branch from d3a72bc to e30247a Compare December 6, 2020 20:30

fixes for general_matrix_matrix_product

8f4d51c

rok-cesnovar force-pushed the eigen339 branch from e58504c to 8f4d51c Compare December 6, 2020 20:41

Merge branch 'develop' into eigen339

7dce138

SteveBronder approved these changes Dec 7, 2020

View reviewed changes

rok-cesnovar merged commit ab3403a into develop Dec 8, 2020

rok-cesnovar deleted the eigen339 branch December 8, 2020 05:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

upgrade to eigen 3.3.9 #2238

upgrade to eigen 3.3.9 #2238

rok-cesnovar commented Dec 6, 2020 •

edited

Loading

rok-cesnovar commented Dec 6, 2020

bgoodri commented Dec 6, 2020

rok-cesnovar commented Dec 6, 2020

rok-cesnovar commented Dec 6, 2020 •

edited

Loading

SteveBronder commented Dec 6, 2020

bgoodri commented Dec 6, 2020

SteveBronder commented Dec 6, 2020

bgoodri commented Dec 6, 2020 via email

rok-cesnovar commented Dec 6, 2020

rok-cesnovar commented Dec 6, 2020

bgoodri commented Dec 6, 2020

rok-cesnovar commented Dec 6, 2020

rok-cesnovar commented Dec 6, 2020

stan-buildbot commented Dec 7, 2020

SteveBronder commented Dec 7, 2020

stan-buildbot commented Dec 7, 2020

upgrade to eigen 3.3.9 #2238

upgrade to eigen 3.3.9 #2238

Conversation

rok-cesnovar commented Dec 6, 2020 • edited Loading

Summary

Tests

Side Effects

Release notes

Checklist

rok-cesnovar commented Dec 6, 2020

bgoodri commented Dec 6, 2020

rok-cesnovar commented Dec 6, 2020

rok-cesnovar commented Dec 6, 2020 • edited Loading

SteveBronder commented Dec 6, 2020

bgoodri commented Dec 6, 2020

SteveBronder commented Dec 6, 2020

bgoodri commented Dec 6, 2020 via email

rok-cesnovar commented Dec 6, 2020

rok-cesnovar commented Dec 6, 2020

bgoodri commented Dec 6, 2020

rok-cesnovar commented Dec 6, 2020

rok-cesnovar commented Dec 6, 2020

stan-buildbot commented Dec 7, 2020

SteveBronder commented Dec 7, 2020

stan-buildbot commented Dec 7, 2020

rok-cesnovar commented Dec 6, 2020 •

edited

Loading

rok-cesnovar commented Dec 6, 2020 •

edited

Loading