RcppCore / RcppEigen

Rcpp integration for the Eigen templated linear algebra library
Other
111 stars 40 forks source link

Eigen 3.3.4 #49

Closed yixuan closed 6 years ago

yixuan commented 6 years ago

This PR updates the included Eigen to 3.3.4, and also applies some upstream patches used to fix #48 .

@eddelbuettel I haven't got the time to run a reverse-dependence check for this PR. Could you run it on the automated server and show me the report? I'd like to look into the cause of potential failures as usual. Thanks!

eddelbuettel commented 6 years ago

Awesome. I'll start a rev.dep run in a moment.

eddelbuettel commented 6 years ago

Looks good. I ran one, and saw no compilation issue. There were a few tests failing where packages had Suggests: yet required the package present---I will skip those packages in a second run.

eddelbuettel commented 6 years ago

One test failure (Cyclops) which gets the same failure under RcppEigen 0.3.3.3.1, so no regression.

eddelbuettel commented 6 years ago

Hu @yixuan should we care about this? Logs from r-devel on 0.3.3.4.0: https://win-builder.r-project.org/bWF1JR2EGjK6/00check.log

I guess we may be able to argue the long long away ("them, not us"). Should we deal with the pragma issue?

yixuan commented 6 years ago

My bad. I accidentally overwrote this commit: https://github.com/RcppCore/RcppEigen/commit/a56f72dc1cf42aaf8c9f4d5beb466ab46fa89f1e. Could you make a quick fix? I think I should report it to Eigen so that next time we do not need to make a manual patch.

eddelbuettel commented 6 years ago

No worries. You can either commit to the same PR (I think) or just open a new one (maybe simpler).

eddelbuettel commented 6 years ago

I just reapplied https://github.com/RcppCore/RcppEigen/commit/a56f72dc1cf42aaf8c9f4d5beb466ab46fa89f1e as d934b12c12886fece65b7bc57030e61c735566a3 but am a little worried about the remainder of the build log:

* checking pragmas in C/C++ headers and code ... WARNING
File which contains pragma(s) suppressing important diagnostics:
  'inst/include/Eigen/src/Core/arch/SSE/Complex.h'
File which contains pragma(s) suppressing diagnostics:
  'inst/include/Eigen/src/Core/util/DisableStupidWarnings.h'
* checking compiled code ... OK

We may have to comment the pragmas out. :-/

eddelbuettel commented 6 years ago

Done in 6247fd632ad6ee78daffa0acff9ff1af8d7aaaa5 and I will run another set of reverse depends tests later; easy enough now thanks to prrd.

yixuan commented 6 years ago

Thanks for the help! I'll document them in my notes and remember to apply these manual patches next time. prrd looks great!

eddelbuettel commented 6 years ago

Shall we, just for our reference, create a recursive diff of "our" Eigen 3.3.4 versus upstream? We could stick it into a top-level directory excluded via .Rbuildignore.

yixuan commented 6 years ago

Great. Will do that later today.

eddelbuettel commented 6 years ago

BTW suppressing the #pragma makes even R CMD INSTALL RcppEigen extremely "noisy" :-/

yixuan commented 6 years ago

Yeah, I see that... Even the 0.3.3.3.1 package now has warnings under R-devel: https://cran.r-project.org/web/checks/check_results_RcppEigen.html.

eddelbuettel commented 6 years ago

Yes, those checks are new-ish. (I run one robot diff'ing CRAN Policy and tweeting / committing changes. I think this may have been added last November or December.)

eddelbuettel commented 6 years ago

I cherry-picked your commit with the diff to Eigen 3.3.4. Minor change to directory layout but nothing substantial. Thanks for setting that up.

yixuan commented 6 years ago

Thank you Dirk!

cdeterman commented 6 years ago

@yixuan if I may ask how do you decide what to include or how you figure out what is in each version? I saw this previous pull request on the Eigen bitbucket here that contains additional changes (among a few additional small changes also in the github mirror) I need for some of my work. I have made the updates in my own fork of RcppEigen but I don't know how you want to manage pull requests.

yixuan commented 6 years ago

As I have explained in #50 , RcppEigen now tracks the stable 3.3 branch of Eigen. So if you need some features that are in the default branch of Eigen, you can report to the developers to back-port them to the 3.3 branch.

cdeterman commented 6 years ago

@yixuan Do you know how else I can get in touch with the Eigen developers? I tried subscribing to the mailing list but I have yet to hear anything back over the last two weeks. Wasn't sure if there was another way to reach out.

yixuan commented 6 years ago

@cdeterman Gael has a public email gael.guennebaud@gmail.com, but from the commit log of Eigen I guess he was tied up by something else and didn't get the time for Eigen development at this moment.