RcppCore
RcppEigen 0.3.4.0.0 preparations
So with #102 (thanks again, @yixuan, for preparing it) we have a release candidate for RcppEigen 0.3.4.0.0. It is currently running a first pass of reverse depends, and a few packages do indeed go belly up.
I suggest we pool resources between myself, @zdebruine, @yixuan, @bbolker ... and whoever wants and can help! Maybe some of the rstan folks want to pitch in?
~While the reverse depends run is still 'cooking' we have failures of these packages (and I will edit/complete this as it finishes)~ The run is now done (and fully summarized in this commit). We should focus on these ten failures out of 329 packages tested:
- [ ] ctsem (at https://github.com/cdriveraus/ctsem) (no fix :disappointed: but maintainer emailed)
- [x] Eagle (at https://github.com/cran/Eagle mirroring http://eagle.r-forge.r-project.org/) (patch emailed 2021-11-23) (merged 2021-11-29, on CRAN 2021-11-30)
- [ ] eimpute (at https://github.com/cran/eimpute, actual upstream source unclear) (patch emailed 2021-11-23)
- [ ] fssemR (at https://github.com/Ivis4ml/fssemR) (PR #3 filed 2021-11-23) (heard back 2021-12-15, see below)
- [ ] gRbase (at https://github.com/hojsgaard/gRbase) (PR #9 filed 2021-11-26)
- [ ] groupedSurv (at https://github.com/cran/groupedSurv/, actual upstream source unclear) (patch emailed 2021-11-26)
- [ ] OpenMx (at https://github.com/OpenMx/OpenMx) (PR #332 filed 2021-11-27) (merged 2021-11-30)
- ~RAINBOWR~ (unrelated to RcppEigen, see below)
- [x] RMixtCompIO (at https://github.com/modal-inria/MixtComp) (no fix :disappointed: but maintainer emailed, fixed upstream 2022-01-14, on CRAN 2022-01-17)
- [ ] skpr (at https://github.com/tylermorganwall/skpr) (PR #65 filed 2021-12-05, merged 2022-03-23)
~Let's not despair---as I write this 191 have passes, or 20x more than those few failures. (I also skipped three)~
It would best if volunteers could step forward and pick package to adopt for a fix. A release candidate version of RcppEigen is in the Rcpp drat repo and be installed via install.packages("RcppEigen", repo="https://RcppCore.github.io/drat") (and you may have to set type="source", I generally don't on Linux).
If you adopt a package, comment below and edit your comment as you move along, eventually (ideally) showing a PR to the package you adopted. We can get this done---it is a matter of effectively parcelling out the grunt work. Let's do this.
The error summary (committed as usual to the rcpp-logs repo) is
Test of RcppEigen 0.3.3.99.0 had 316 successes, 13 failures, and 3 skipped packages.
Ran from 2021-10-26 15:15:22.92 to 2021-10-27 00:09:34.52 for 8.903 hours
Average of 96.541 secs relative to 575.519 secs using 6 runners
Failed packages: breathteststan, ctsem, Eagle, eimpute, fssemR, fwildclusterboot, gRbase, groupedSurv, OpenMx, RAINBOWR, RMixtCompIO, SEMgraph, skpr
Skipped packages: hBayesDM, kmcudaR, nlmixr
None still working
None still scheduled
Error summary:
package missingPkg badInstall error fail warn note ok hasOtherIssue
1: breathteststan FALSE 0 0 0 14 0 FALSE possibly unrelated numeric errors
2: ctsem TRUE 0 0 1 13 0 FALSE error: no matching function for call to ‘isinf(const stan::math::var&)'
3: Eagle TRUE 0 0 0 4 10 FALSE error: no match for ‘operator=’
4: eimpute TRUE 0 0 0 13 1 FALSE error: no match for ‘operator/’
5: fssemR TRUE 0 0 0 13 1 FALSE error: no match for ‘operator=’
6: fwildclusterboot FALSE 0 0 0 0 14 FALSE possibly unrelated numeric errors
7: gRbase TRUE 0 0 0 6 8 FALSE error: static assertion failed: cannot convert type to SEXP
8: groupedSurv TRUE 0 0 0 4 10 FALSE error: cannot convert ‘Eigen::internal::enable_if<.....
9: OpenMx TRUE 1 0 0 13 0 TRUE error: no match for ‘operator-’
10: RAINBOWR TRUE 3 0 0 2 9 FALSE unrelated: "Error: object ‘get_aes_var’ is not exported by 'namespace:rvcheck'"
11: RMixtCompIO TRUE 0 0 0 4 10 FALSE error: static assertion failed:
12: SEMgraph 'cate', 'diffusr', 'flip' NA 0 0 0 1 13 FALSE passes once installed
13: skpr TRUE 0 0 0 4 10 FALSE error: cannot convert ‘Eigen::internal::enable_if<....
I have skipped the package that passed once its dependencies were added and two with apparently unrelated test error. That leaves ten which failed to install under 0.3.3.99.0 which is surely something we need to address if we want the package on CRAN.
I'll make sure the rstan-related ones get taken care of. @SteveBronder said that Stan is good for Eigen 3.4 in general, but some of those packages may have some old C++ code generated by Stan.
With packageVersion("RcppEigen") == 0.3.3.99.0:
Eagle: PR submitted to mute unnecessary check in a function (https://github.com/davidaknowles/eagle/pull/2). Resulting test failure was unrelated to RcppEigen 0.3.99. Not sure why this was considered passing previously, if that was the case.
eimpute: Forked this repo (https://github.com/Mamba413/eimpute) and it's building fine on my machine. There are no tests that I can find in that repo, the vignette is building fine.
Part of my sessionInfo():
> sessionInfo()
R version 4.0.3 (2020-10-10)
Platform: x86_64-w64-mingw32/x64 (64-bit)
Running under: Windows 10 x64 (build 19042)
.
.
.
other attached packages:
[1] ... RcppEigen_0.3.3.99.0
@zdebruine I see a compile failure (on Linux). As you are set up with Eagle, please try it a different platform. Maybe Windows (which you use) is for once more helpful ;-)
@zdebruine I see a compile failure (on Linux). As you are set up with
Eagle, please try it a different platform. Maybe Windows (which you use) is for once more helpful ;-)
Ah, the CRAN version of Eagle is actually ahead (2.4.5) of the GitHub version (1.0) of Eagle (davidaknowles/eagle), from which I had forked and installed. Yes, I can replicate your build issue with install.packages("Eagle") on Linux and Windows when installing from source.
I noticed that eagle is failing because it uses the long type, and the header library is missing operator definitions for long objects. This may be the case more broadly, judging from Dirk's errors showing lack of operator support. Recall what @yixuan stated in PR #102:
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.
Even if RcppEigen does not wrap or as any long objects, we still need support for long types in the header library. @yixuan am I wrong, or if we add back support for long will these packages be fine?
@zdebruine Ah, what I mentioned was only concerned with the use of long type in CHOLMOD, which was not supported by current RcppEigen either.
The problem of Eagle is that in some cases it uses a double variable to index an element of the matrix. Specifically, in lines 146 and 272 of calculate_a_and_vara_rcpp.cpp (version 2.4.5 on CRAN), indx[i] is a double variable (coming from an Rcpp::NumericVector). So the authors should convert it to an int or long when indexing a matrix:
var_ans(long(indx[i]), 0) = ans1.dot(Mt.row(indx[i]));
Hi sorry I'm running out the door right now so I'll comment in more detail tmrw, but I think the rstan on CRAN should be fine (using Stan 2.21) but Stan 2.21+ actually has some pretty major updates needed for Eigen 3.4. Early next week I can run Stan math 2.21's unit tests with Eigen 3.4 to see if anything breaks. If those pass then at least rstan should be good.
When updating the develop version of the Stan math library to use Eigen 3.4 there are a few places we won't compile because we need to use some Eigen internals to understand if we are looking at a matrix or expression and one tricky set of decompositions which are failing our tests. We were going to do a release of rstan soon with Stan 2.26 and I think that would break with 3.4. But I think ^ is probably in the realm of Stan problems and not RcppEigen problems so maybe we could sort this out ourselves without needing anything
@SteveBronder Thanks for the follow-up, but that's almost orthogonal to the question we are trying to answer here. Namely, who is going to step up and shepherd the ten known packages needing changes through making the changes. That set of packages does not include rstan (but possibly some users of stan headers -- I would have to check, the list is above).
@eddelbuettel yes sorry that seemed a bit off topic, but I'm worried there are Stan functions not tested within rstan or other R packages that will numerically fail with RcppEigen moving to 3.4 like in the tests I'm seeing in the Stan math repository. The two functions (mdivide_left and mdivide_right) that fail numeric tests in Stan math have not been changed much since the rstan version of Stan (2.21). So my concern is for the 10 known packages but also the the scripts etc. that use rstan with functions not tested in any of these.
The focus of this thread is to find a way to get the ten packages that at current do not compile under Eigen 3.4.0 to actually compile under Eigen 3.4.0. I cannot and will not do it alone, but I am willing help if others step up.
So far nobody has, and if that doesn't change there won't be Eigen 3.4.0 at CRAN any time soon as they will not admit an update breaking ten existing packages.
As a reminder, one can install RcppEigen 0.3.3.99.0 containing the snapshot in a single in dev environment, or Docker, or (with an extra argument) in libpath apart not hurting one's main system:
edd@rob:~$ docker run --rm -ti rcpp/run Rscript -e 'install.packages("RcppEigen", repos="https://rcppcore.github.io/drat")'
Installing package into ‘/usr/local/lib/R/site-library’
(as ‘lib’ is unspecified)
trying URL 'https://rcppcore.github.io/drat/src/contrib/RcppEigen_0.3.3.99.0.tar.gz'
Content type 'application/gzip' length 1741790 bytes (1.7 MB)
==================================================
downloaded 1.7 MB
[lots of the usual compiler noise removed]
gcc -std=gnu99 -I"/usr/share/R/include" -DNDEBUG -I'/usr/local/lib/R/site-library/Rcpp/include' -fpic -g -O2 -ffile-prefix-map=/build/r-base-kpovBh/r-base-4.1.1=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -g -c init.c -o init.o
g++ -std=gnu++14 -shared -L/usr/lib/R/lib -Wl,-z,relro -o RcppEigen.so RcppEigen.o RcppExports.o fastLm.o init.o -llapack -lblas -lgfortran -lm -lquadmath -L/usr/lib/R/lib -lR
installing to /usr/local/lib/R/site-library/00LOCK-RcppEigen/00new/RcppEigen/libs
** R
** inst
** byte-compile and prepare package for lazy loading
** help
*** installing help indices
** building package indices
** installing vignettes
** testing if installed package can be loaded from temporary location
** checking absolute paths in shared objects and dynamic libraries
** testing if installed package can be loaded from final location
** testing if installed package keeps a record of temporary installation path
* DONE (RcppEigen)
The downloaded source packages are in
‘/tmp/Rtmp3e8n6i/downloaded_packages’
edd@rob:~$
(This uses a convenience container that already has R and Rcpp, on an empty system you have to add the drat to options$repo rather than overwriting its setting. There is a drat package helper function for that.)
I will try to take one or two, but can't promise any hard deadlines ... (I don't, personally, much care whether RcppEigen 3.4.0 happens, but I can see that it is a good thing generally, and I see the need to distribute the workload.)
I feel the same. I still have a loooooong running Rcpp transition on my plate but this is also something that ought to get done, and ought to be doable. But it's a teaching term for me so my workload is a bit higher ...
@eddelbuettel @bbolker I'm personally invested in seeing this happen (my RcppML package would make good use of Eigen 3.4), and happy to help, but I'm in the first week of my postdoc and a novice compared to all you experts. It'll be a few weeks before I can give this what it needs.
Ah, the CRAN version of Eagle is actually ahead (2.4.5) of the GitHub version (1.0) of Eagle (davidaknowles/eagle), from which I had forked and installed.
@zdebruine I just wanted to jump in real quick to clear up some confusion. The versions are different because these are 2 different packages:
- Eagle is on CRAN and the source is hosted on R-Forge
- eagle is a GitHub-only project. While it has RcppEigen as a dependency, it is not blocking CRAN submission because it's not on CRAN
I just tried to look into Eagle and failed on
https://github.com/cran/Eagle/blob/aa31d13ff2d9181712798e994c567b2f6b0004e4/src/calculate_a_and_vara_batch_rcpp.cpp#L164
which majorly upset the template programming in Eigen and I have not found an easy way auto (via a temporary variable). Anybody?
Ok, took another stab and I now have Eagle, eimpute, and fssemR sorted out. All were a "simple" case of needing a cast for one of the row or col indices (computed from another Eigen expression) and I will create PRs. ctsem is more of a mystery, maybe someone from team rstan can take a look (hi, @SteveBronder).
Two quick updates:
-
We have the first updated package, the winner is
Eagle. -
And as we heard from a second,
groupedSurvthe fix I often made (static_cast<int>()for indices) is due to use of, say,VectorXdwhereVectorXiwould be better. I think I may even have seen a hint of 'no longer acceptdoublefor indices' in the Eigen 3.4.0 release notes but did not realize this when preparing the patch. So if you read this as an (Rcpp)Eigen user, check that the indexing comes fromVectorXi.
I have completed one pass and managed to create seven out of nine patch sets. I had to bail on ctsem (maybe @bgoodri and @SteveBronder can take a look?) as well as on RMixtCompIO (anybody ?). Second looks would be appreciated as would be gentle nags in due time to get the PRs and patches onto CRAN.
I will look at it. ctsem is one of the more complicated packges that uses Stan.
On Sun, Dec 5, 2021 at 5:41 PM Dirk Eddelbuettel @.***> wrote:
I have completed one pass and managed to create seven out of nine patch sets. I had to bail on ctsem (maybe @bgoodri https://github.com/bgoodri and @SteveBronder https://github.com/SteveBronder can take a look?) as well as on RMixtCompIO (anybody ?). Second looks would be appreciated as would be gentle nags in due time to get the PRs and patches onto CRAN.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/RcppCore/RcppEigen/issues/103#issuecomment-986314206, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAZ2XKVLNTWUV4E7L6PUFKTUPPTABANCNFSM5GZEESDQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
I'm just aware of this now, happy to try modify the stan code in ctsem but also not sure where to start. I'm available for any discussion / ideas @bgoodri ...
@cdriveraus Yep, I didn't email you til yesterday so no need to feel guilty :) And as you can see other packages "need to move too". As ctsem is ineed far from simply, help by @bgoodri and friends will be very much appreciated.
I can confirm that the compilation error with the ctsem package and Eigen 3.4 is not due to anything specific to ctsem. The same error
no known conversion for argument 1 from ‘const stan::math::var’ to ‘float’ 611 | isnan(float __x)
(and similarly for isinf) can be triggered more simply just by executing
rstan::stan_model(model_code =
"
functions {
vector get_eigenvalues(matrix A) {
return eigenvalues_sym(A);
}
}
parameters {
cov_matrix[2] A;
}
model {
vector[2] a = get_eigenvalues(A);
}
")
Stan definitely has implementations of isinf and isnan for its var types at
https://github.com/stan-dev/math/blob/develop/stan/math/rev/core/std_isinf.hpp#L16 https://github.com/stan-dev/math/blob/develop/stan/math/rev/core/std_isnan.hpp#L18
the problem is likely that those file are not getting included early enough. Stan's rev.hpp metaheader looks like
#ifndef STAN_MATH_REV_HPP
#define STAN_MATH_REV_HPP
#include <stan/math/prim/fun/Eigen.hpp> // first because of the Eigen plugin and then includes Eigen/Dense
#include <stan/math/opencl/rev.hpp>
#include <stan/math/rev/core.hpp> // includes var definition and isinf, isnan, etc.
#include <stan/math/rev/meta.hpp>
#include <stan/math/rev/fun.hpp>
#include <stan/math/rev/functor.hpp>
#include <stan/math/prim.hpp> // includes eigenvalues_sym etc.
#endif
If that is correct, then this is definitely Stan's fault, but at the moment I don't know how to fix it without breaking our Eigen plugin. I also don't know how we got away with this under the previous versions of Eigen. @SteveBronder @andrjohns
Maybe making (more explicit) use of namespace possibly conditional behind a C++17 define (i.e. something like
#if __cplusplus > 201703L
then you do not defone isinf, and hence do not clash? (Untested, obviously)
Edit I mixed up two parallel discussions that I am in -- sorry. This is not conditional on C++17 but "simply" on Eigen 3.4.0. You should still be able to test for it.
I think the issue with isinf and isnan would be fixed if I updated StanHeaders on CRAN. More recent versions of Stan put isinf and isnan in the std namespace so that the implementations for Stan's reverse mode var custom scalar type can be found by ADL. But the (much) older version of StanHeaders on CRAN defines them in the stan::math namespace and then ultimately calls std::isinf on the double value underlying the var type.
I have been planning for ages to update StanHeaders on CRAN anyway, but doing so requires a lot of patching to make it backwards compatible with the CRAN packages that use it. We have achieved backwards compatibility on everything but Windows, but then got sidetracked because some package that has rstan in its Suggests wrote a bad test that crashed R and CRAN wants me to upload a new rstan that is more user-proof. Unfortunately, I think the StanHeaders that I want to get Windows-compatible and onto CRAN will expose another issue with how Stan solves a triangular system of equations under Eigen 3.4 that I will need to figure out a workaround for.
TL;DR: If ctsem becomes the last package blocking RcppEigen, I'll upload a minor update to StanHeaders that just fixes the isinf and isnan thing.
I have fixed that and merged the fixed into the fssemR on github now. Will work on submit a new version of package onto CRAN later.
I ran another set today, first against RcppEigen at CRAN and then second against the RcppEigen release candidate for with Eigen 3.4.0:
Test of RcppEigen 0.3.3.99.0 had 323 successes, 17 failures, and 2 skipped packages.
Ran from 2021-12-25 23:35:31.69 to 2021-12-26 08:57:16.51 for 9.362 hours
Average of 98.552 secs relative to 588.151 secs using 6 runners
Failed packages: BigDataStatMeth, breathteststan, ctsem, eimpute, fssemR, fwildclusterboot, gRbase, groupedSurv, OpenMx, PReMiuM, RAINBOWR, RMixtCompIO, secr, SEMgraph, skpr, stan4bart, TDA
Skipped packages: hBayesDM, nlmixr
None still working
None still scheduled
Error summary:
package missingPkg badInstall error fail warn note ok hasOtherIssue
1: BigDataStatMeth FALSE 7 0 0 6 0 TRUE not us, issues at CRAN
2: breathteststan FALSE 0 0 0 13 0 FALSE not us, minor (?) recurring numeric test error
3: ctsem TRUE 0 0 0 13 0 FALSE help needed with known Eigen 3.4.0 issue
4: eimpute TRUE 0 0 0 12 1 FALSE patch emailed 2021-11-23
5: fssemR TRUE 0 0 0 12 1 FALSE pr filed, merged, but not at CRAN yet
6: fwildclusterboot FALSE 0 0 0 0 13 FALSE flaky, small numerical error
7: gRbase TRUE 0 0 0 6 7 FALSE pr filed 2021-11-26
8: groupedSurv TRUE 0 0 0 4 9 FALSE patch emailed 2021-11-26
9: OpenMx TRUE 0 0 0 13 0 TRUE pr filed 2021-11-27, merged 2021-11-30, but not at CRAN yet
10: PReMiuM TRUE 0 0 0 4 9 FALSE known flaky
11: RAINBOWR TRUE 2 0 0 3 8 FALSE not us, Error: object ‘get_aes_var’ is not exported by 'namespace:rvcheck'
12: RMixtCompIO TRUE 0 0 0 4 9 FALSE maintainer emailed
13: secr TRUE 9 0 0 4 0 FALSE known BH issues, needs one std::isnan
14: SEMgraph TRUE 10 0 0 3 0 FALSE not us, "Error: object ‘ggm_compare’ is not exported by 'namespace:GGMncv'"
15: skpr TRUE 0 0 0 4 9 FALSE pr file 2021-12-05
16: stan4bart TRUE 0 0 0 4 9 FALSE help needed
17: TDA TRUE 10 0 0 3 0 FALSE known BH issue, upstream contacted
We have two packages with merged changes that should be at CRAN at some point. But a number of packages have either not yet responded, or do not even have a fix.
ctsemis difficult and a known open issuestan4bartcould likely benefit from someone from the stan look into it too- some of the other packages might benefit from a nudge to the maintainer.
In short, if you want to see this package updated at CRAN, please consider adopting a task. A message here should be good enough to monitor things.
Hi, the RmixtComp's maintainer emails me today about the eigen problem.
I'm working on it, this PR should solve the problem https://github.com/modal-inria/MixtComp/pull/20
