RcppCore / RcppEigen

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

Update to Eigen 3.3.9 (closes #91) #92

Closed eddelbuettel closed 3 years ago

eddelbuettel commented 3 years ago

So procrastination does pay off after all. By "waiting" and being unware of the 3.3.8 all this time it so happens that 3.3.9 came out yesterday.

Carrying the changes over was fairly painless, and I snuck in a few of the regular updates. I will put this in the [Rcpp drat]() repo in a few minutes so that you can get a tarball from there but before I upload to CRAN (or even get to reverse dependency checks, currently running one for Rcpp which will eat up a few more hours) I would love for your guys at 'team stan' to test it. So flagging a few of you with review requests. Hope that is ok. @SteveBronder @bgoodri

PS Done. You can install it via install.packages("RcppEigen", repos="https://rcppcore.github.io/drat")' as usual. And yes, compilation is still as noisy as always because that, for better or worse, is how CRAN rules.

bgoodri commented 3 years ago

I am getting a build error with rstan ( https://github.com/stan-dev/math/issues/2235 ) that seems to be due to changing the number of required number of template arguments in some traits-related thing. We'll try to fix it ASAP.

eddelbuettel commented 3 years ago

Thanks for checking so promptly! I guess I would have run into this too by rev.dep-ing (when I get to it). There is no rush, as you know I was following on 3.3.7 exactly overnight... We'll do this the right way.

yixuan commented 3 years ago

This is super awesome, Dirk!

eddelbuettel commented 3 years ago

@yixuan I have no choice as you set such an good example so many times ;-)

yixuan commented 3 years ago

In fact I created a branch yesterday trying to update the new version, as I felt so guilty that I didn't have much time on this for the last few releases. And then I surprisingly found that you have finished everything in such a short time. 😂

eddelbuettel commented 3 years ago

Hi @bgoodri I think I ran into the same issue reverse-depends checking the new version -- a lot of StanHeaders clashing.

Let's see over the next few days if we can come up an upgrade path of updating StanHeaders first and then re-assessing what other changes a new RcppEigen may trigger.

In the meantime, I may merge this PR because the package is ok per se but just not ready to interact with the rest of CRAN.

bgoodri commented 3 years ago

@eddelbuettel Yes, I would anticipate that almost everything related to Stan on CRAN will fail to build because this change affects the numerical traits of custom-defined scalar types that we use in Stan for automatic differentiation (but non-Stan packages that just use int, double, etc. are probably not affected). It seems likely that StanHeaders will have to branch on EIGEN_MINOR_VERSION in order for Stan stuff to build with the current RcppEigen on CRAN and the future RcppEigen.

eddelbuettel commented 3 years ago

@bgoodri @stevebronder Any news about the StanHeaders change? Keep me posted...

bgoodri commented 3 years ago

I think we're good but haven't run the CRAN checks yet (I'm braving the airports today so won't be able to kick them off until tonight)

On Thu, Dec 10, 2020, 7:21 AM Dirk Eddelbuettel notifications@github.com wrote:

@bgoodri https://github.com/bgoodri @SteveBronder https://github.com/SteveBronder Any news about the StanHeaders change? Keep me posted...

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/RcppCore/RcppEigen/pull/92#issuecomment-742517259, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAZ2XKT65BYLJMTQ5C6LC2TSUDDN5ANCNFSM4UOVJ5BA .

eddelbuettel commented 3 years ago

Thanks for the update. There is obviously no urgency, I just wanted to see where it's at.

So we should be able to do a two-step: once updated, StanHeaders goes to CRAN, I resume my rev.dep tests which should look better ;-) and then hopefully RcppEigen 0.3.3.9.* at CRAN.

SteveBronder commented 3 years ago

Yes Rok just updated stan math to support 3.3.9 so should be hopefully be smooth(er) sailing. Thanks for updating this Dirk much appreciated!

eddelbuettel commented 3 years ago

I think I saw that in the (large) referenced commit.

But I guess I'd need an updated StanHeaders package on CRAN so that the reverse dependencies checks get that update along with my RcppEigen 0.3.3.9.* check. I presume I will see it via CRANberries but any timeline visibility over from your end about when/if it may ship to CRAN?

bgoodri commented 3 years ago

I started running the reverse dependency checks for StanHeaders yesterday. The new StanHeaders with the old RcppEigen seems to be OK. I will start the checks with the new StanHeaders and the new RcppEigen now.

eddelbuettel commented 3 years ago

Nice, thanks -- I could turn my instance on as well if you let me have the candidate tarball. Is there a drat repo anywhere?

In other news, and because the fun never stops, I am right now preparing BH 1.75.0-0 based on yesterday's Boost 1.75.0.

bgoodri commented 3 years ago

Here is a tarball: StanHeaders_2.21.0-7.tar.gz. So

install.packages("https://github.com/RcppCore/RcppEigen/files/5683396/StanHeaders_2.21.0-7.tar.gz", repos = NULL)

should work.

eddelbuettel commented 3 years ago

Nice, thanks! Currently noodling over BH 1.75.0-0 which will take a moment but should be able to turn this on later today.

eddelbuettel commented 3 years ago

Started a reverse depends run, will know full results tomorrow but so far, so good: the first handful of (early in the full set) failures for the last round now pass -- so looks like this is doing what it was supposed to do :+1:

eddelbuettel commented 3 years ago

'All clear' from my end, see the (now committed) result summary -- no compilation issues or test regressions over 270 packages.

So once StanHeaders is on CRAN, RcppEigen can follow. Thanks so much to @rok-cesnovar for the prompt update.

rok-cesnovar commented 3 years ago

Glad to help. Thank you for maintaining this great package!

eddelbuettel commented 3 years ago

How are things looking with respect to a CRAN upload for StanHeaders?

@bgoodri @SteveBronder @rok-cesnovar : any insight? This is the critical path for RcppEigen which I updated thanks to your nudging but cannot actually upload unless you update first.

rok-cesnovar commented 3 years ago

@bgoodri is handling CRAN matters, I think he was traveling last week, not sure.

bgoodri commented 3 years ago

I am going to try to upload it to CRAN this afternoon. I ran it with the rstan reverse dependencies because there are 50 or so more packages that are depending on the libraries through rstan, but they seem to be fine too.

On Wed, Dec 16, 2020 at 8:59 AM Rok Češnovar notifications@github.com wrote:

@bgoodri https://github.com/bgoodri is handling CRAN matters, I think he was traveling last week, not sure.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/RcppCore/RcppEigen/pull/92#issuecomment-746329464, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAZ2XKWTHJNTDUBXAQRIRRLSVC4NXANCNFSM4UOVJ5BA .

eddelbuettel commented 3 years ago

That would be great. As your results are clean too (apart from the usual directory size) it may auto-process after reverse depends checks gving RcppEigen a chance to follow suit and sneak in before the gate closes for two weeks.

bgoodri commented 3 years ago

The new StanHeaders made it to CRAN, so you should be good to upload RcppEigen now.

On Wed, Dec 16, 2020, 6:33 PM Dirk Eddelbuettel notifications@github.com wrote:

That would be great. As your results are clean too (apart from the usual directory size) it may auto-process after reverse depends checks gving RcppEigen a chance to follow suit and sneak in before the gate closes for two weeks.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/RcppCore/RcppEigen/pull/92#issuecomment-747124993, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAZ2XKXO4AQDY5TYDXQBZWLSVFGT3ANCNFSM4UOVJ5BA .

eddelbuettel commented 3 years ago

New RcppEigen prepared, submitted, pretested, and fell victim to one false positive as hsstan has a NOTE on "GNU make" which is not us. So should be on CRAN "soon".