New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Eigen 3.3.4 #49

Merged
merged 4 commits into from Feb 4, 2018

Conversation

Projects
None yet
3 participants
@yixuan
Contributor

yixuan commented Feb 4, 2018

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

This comment has been minimized.

Member

eddelbuettel commented Feb 4, 2018

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

@yixuan yixuan referenced this pull request Feb 4, 2018

Closed

Update Frequency? #48

@eddelbuettel

This comment has been minimized.

Member

eddelbuettel commented Feb 4, 2018

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

This comment has been minimized.

Member

eddelbuettel commented Feb 4, 2018

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

@eddelbuettel eddelbuettel merged commit 3e02a03 into RcppCore:master Feb 4, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@eddelbuettel

This comment has been minimized.

Member

eddelbuettel commented Feb 4, 2018

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

This comment has been minimized.

Contributor

yixuan commented Feb 5, 2018

My bad. I accidentally overwrote this commit: a56f72d. 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

This comment has been minimized.

Member

eddelbuettel commented Feb 5, 2018

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

@eddelbuettel

This comment has been minimized.

Member

eddelbuettel commented Feb 5, 2018

I just reapplied a56f72d as d934b12 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

This comment has been minimized.

Member

eddelbuettel commented Feb 5, 2018

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

@yixuan

This comment has been minimized.

Contributor

yixuan commented Feb 5, 2018

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

@eddelbuettel

This comment has been minimized.

Member

eddelbuettel commented Feb 5, 2018

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

This comment has been minimized.

Contributor

yixuan commented Feb 5, 2018

Great. Will do that later today.

@eddelbuettel

This comment has been minimized.

Member

eddelbuettel commented Feb 5, 2018

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

@yixuan

This comment has been minimized.

Contributor

yixuan commented Feb 5, 2018

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

This comment has been minimized.

Member

eddelbuettel commented Feb 5, 2018

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

This comment has been minimized.

Member

eddelbuettel commented Feb 7, 2018

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

This comment has been minimized.

Contributor

yixuan commented Feb 7, 2018

Thank you Dirk!

@cdeterman

This comment has been minimized.

cdeterman commented Feb 9, 2018

@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

This comment has been minimized.

Contributor

yixuan commented Feb 12, 2018

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

This comment has been minimized.

cdeterman commented Mar 1, 2018

@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

This comment has been minimized.

Contributor

yixuan commented Mar 2, 2018

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment