Eigen 3.4.0

[yixuan]

This PR should have solved the issue mentioned in https://github.com/RcppCore/RcppEigen/issues/101.

The problem was that in Eigen 3.4.0, Eigen/src/CholmodSupport/CholmodSupport.h introduced a series of cholmod_l_* functions intended for the long integer type, which were not defined in RcppEigenCholmod.h. The fix is to remove the definitions of those functions, as they are not used by RcppEigen.

Note that I have only tested the building of RcppEigen itself and that of my own RSpectra package. I expect that there may be some issues for other packages depending on RcppEigen. We still need @eddelbuettel to schedule a (large-scale) automated test for the huge number of dependent packages.

[eddelbuettel]

Perfect. I started to poke there too, but I only altered them to get to subsequent template errors, and then it got late.

[eddelbuettel]

BTW you are a co-author so I am sure you have write permissions in this repo so you could and (maybe should) just branch here. I will put a trivial patch back to you call this 0.3.3.99.0 for now to mark it as a (unrelease) release candidate while we test.

[yixuan]

Oh that's good. I didn't realize that I have write permissions. Thanks for reminding.

[eddelbuettel]

My bad, you didn't, but should have. Rectified now, invite in your inbox.

There is actually a Rcpp reverse depends check running now, and those tend to take a moment. RcppEigen will be next; we may know more later today or tomorrow.

[bbolker]

FWIW looks like lme4 passes with new version (after one tolerance adjustment, will require new version on CRAN ...) Apparently a slight slowdown: 8m38.904s for check with 0.3.3.9.1, 9m33.881s with new version. Haven't done any more careful benchmarking. Slowdown would be too bad (and a bit of a pain for CRAN submission, we might have to skip more tests on CRAN), but not the end of the world.

[eddelbuettel]

Where did you test that? 'Performance regression testing' is known to be impossible on CI frameworks as we never know what hardware we're on each time -- or was it your own machine?

In any event I am "merely the messenger" :grinning: -- the delta is all upstream.

[bbolker]

It was my own machine. I'm not terribly concerned about this, just reporting my experience.

[eddelbuettel]

So it took a moment before the Rcpp reverse depends check complete, and the one for RcppEigen is still running ... and it brings good and bad news. In short, just like the change to 3.3.0 there is a bit of disruption. A few packages no longer build, and if we want 3.4.0 in R, we need to buckle up and adjust the packages (not unlike what I have been doing for Rcpp over here). I'll open a ticket here.

[waynelapierre]

Any updates on this PR?

[eddelbuettel]

Please see https://github.com/RcppCore/RcppEigen/issues/103 and help as you can.

[eddelbuettel]

I think I got the merge conflicts sorted the right way --- but it might benefit from another glance or two by @jaganmn at the merge commit 4db13cb44439c5f296a3310ae16ce8067494b1af .

[andrjohns]

@eddelbuettel I re-ran the revdep checks with this branch and lme4 and OpenMx both pass now (thanks for sorting that so quickly @jaganmn!).

Down to 8 packages on CRAN that break, with three having already merged patches: https://github.com/RcppCore/RcppEigen/issues/103#issuecomment-1894705892

[jaganmn]

I left one comment on Dirk's merge commit, but apart from that it seems OK.

[eddelbuettel]

Just replied to @jaganmn there and in short I was just trying to move the PR over a hump of a merge conflict. I may have done that in haste.

[eddelbuettel]

Lovely stuff. Thank you all -- will turn on a rev.dep run here too.

[eddelbuettel]

It has been shipped to CRAN following all the reverse-dependency work chronicled in #103 !!

One question: if/when this passes as we hope, how do we merge this? If we do a merge commit we get really ugly (but truthful) parallel histories in git ls (in 'graph mode'). Maybe despite the ugliness that is best to show the full history?

Else we could squash merge for linear history (which I like in other repos) but it brushed a lot under the carpet. Thoughts? Preferences? Or non-issue?

[eddelbuettel]

Thanks, on its way to CRAN.

Yay!

Thoughts on how to merge as per my previous comment?

[yixuan]

This is fantastic!!

I really have no preference on the merging strategy here. But if I have to make a choice, I may vote for the cleaner way. To be honest, I was always puzzled by git histories with multiple sources of changes. 🤣

Other thoughts?

[eddelbuettel]

Hey -- good to hear from you. I am really torn. I like 'linear' -- at work we enforce it via 'squash merge': each PR is a squash.

I don't mind standard PR with (usually) just one departure:

I am less of a fan where it gets 'wild'

Because yours started so long ago we'd have a really long parallel history. But then again we a) have never squashed and b) it's nice if your work is reflected as is.

So kinda leaning towards normal 'noisy' merge. What do you think?

[ But most importantly: it does not really matter. Code quality matters, clean tests, good ci. This is mostly cosmetic. It matters really only a little bit. ]

[yixuan]

Hi Dirk, if the consideration is on reflecting "my work" then I really do not mind squashing it. The changes mostly come from upstream Eigen, and I would not take that much credit. 😄

[eddelbuettel]

Ok -- squash it is then! (I switched in Rcpp too. It is cleaner. Will take a while til we have a 'page full' though )