rstanarm icon indicating copy to clipboard operation
rstanarm copied to clipboard

Feature/survival

Open sambrilleman opened this issue 6 years ago • 15 comments

I think this is pretty much ready for a pull request.

R CMD check seems to passing without any obvious issues.

I've added the splines2 package to Imports, and the simsurv package to Suggests. I've commented out some stan_surv testthat tests to avoid adding more packages to Suggests.

There is quite a bit of overlap with stan_jm stuff, but I got into a mess trying to refactor all the stan_jm code, so for now they mostly use different workhorse and helper functions. One day I will try and refactor some of the stan_jm code to use more of the stan_surv helpers, since the stan_surv code is much neater I think.

I've not packaged surv.stan code into \#include statements since the code doesn't really overlap with other stan files. But I could do so if need be.

sambrilleman avatar Nov 01 '18 06:11 sambrilleman

Hi @bgoodri and @jgabry -- any updates / possible timeline on this one? Cheers, Sam.

sambrilleman avatar Feb 03 '19 23:02 sambrilleman

Now that the CRTP stuff is in the version of RStan on CRAN, we might be able to get this into rstanarm in about a month. I had to not compile jm.stan on 32bit Windows, which isn't really a problem because no one uses 32bit Windows, but it would not compile it with less than the limit of 2GB of address space. The same might be true of surv.stan. Also, I had to not run any examples or tests on 32bit Windows in order to get check time under 10 minutes, so see the last few commits on develop for how to do that.

bgoodri avatar Jul 22 '20 02:07 bgoodri

ok, cheers @bgoodri. I have added the wrapper around stan_surv examples to avoid them running on 32bit Windows.

sambrilleman avatar Jul 26 '20 06:07 sambrilleman

I think the 32bit Windows thing is not a major concern anymore.

On Sun, Jul 26, 2020 at 2:48 AM Sam Brilleman [email protected] wrote:

ok, cheers @bgoodri https://github.com/bgoodri. I have added the wrapper around stan_surv examples to avoid them running on 32bit Windows.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/stan-dev/rstanarm/pull/323#issuecomment-663944652, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAZ2XKUSJKDMWHFU4ICVFXTR5PGVDANCNFSM4GAXKQSQ .

bgoodri avatar Aug 09 '20 04:08 bgoodri

I think the 32bit Windows thing is not a major concern anymore. On Sun, Jul 26, 2020 at 2:48 AM Sam Brilleman @.***> wrote: ok, cheers @bgoodri https://github.com/bgoodri. I have added the wrapper around stan_surv examples to avoid them running on 32bit Windows. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#323 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAZ2XKUSJKDMWHFU4ICVFXTR5PGVDANCNFSM4GAXKQSQ .

Does that mean just leave it as-is? Or you have removed those 32-bit wrappers around the examples on master?

sambrilleman avatar Aug 09 '20 04:08 sambrilleman

We still can't afford to run tests or examples on 32bit Windows, but we can compile almost anything within the address space limitations as long as we use LTO. LTO might not work on R 3.x, but we seemed to be able to squeeze everything in with the RTools35 toolchain.

On Sun, Aug 9, 2020 at 12:23 AM Sam Brilleman [email protected] wrote:

I think the 32bit Windows thing is not a major concern anymore. … <#m_7428270053154382209_> On Sun, Jul 26, 2020 at 2:48 AM Sam Brilleman @.***> wrote: ok, cheers @bgoodri https://github.com/bgoodri https://github.com/bgoodri. I have added the wrapper around stan_surv examples to avoid them running on 32bit Windows. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#323 (comment) https://github.com/stan-dev/rstanarm/pull/323#issuecomment-663944652>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAZ2XKUSJKDMWHFU4ICVFXTR5PGVDANCNFSM4GAXKQSQ .

Does that mean just leave it as-is? Or you have removed those 32-bit wrappers around the examples on master?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/stan-dev/rstanarm/pull/323#issuecomment-671004459, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAZ2XKXEELAYAKEIEB2B7ADR7YQE7ANCNFSM4GAXKQSQ .

bgoodri avatar Aug 09 '20 04:08 bgoodri

@jburos I think the GitHub actions were already enabled for the survival branch but were not being executed due to the merge conflicts. I fixed those and now R CMD check runs, although there are some test failures.

bgoodri avatar Feb 07 '23 16:02 bgoodri

@bgoodri Thanks this is really helpful!

It looks like this PR now includes a few older versions of stan_mvmer- and stan_jm-related files, which represent at lesat some of the failing tests. (e.g. changes to src/stan_files/functions/jm_functions.stan, or testthat/tests/test_stan_jm.R).

I'm going to try to create a sanitized version of this PR, limited to survival model functionality. This way any new failing tests are specific to this content.

jburos avatar Feb 07 '23 18:02 jburos

Hi @jburos and @sambrilleman just wondering how you are installing the survival branch atm as many of us are struggling to get stan_surv() working with random effects #588

Also thank you generally as the package is amazing.

padpadpadpad avatar Sep 14 '23 19:09 padpadpadpad

Dear all, My compilation of the rstanarm survival extension fails: I use the following code:

install.packages("rstanarm", repos = c("https://mc-stan.org/r-packages/", getOption("repos"))) devtools::install_github("stan-dev/rstanarm", ref = "feature/survival", build_vignettes = FALSE)

I am using R version 4.2 on a Windows 10 computer. It runs until this point:

C:/Users/wilhch00/AppData/Local/R/win-library/4.2/RcppEigen/include/Eigen/src/Core/DenseCoeffsBase.h:55:30: warning: ignoring attributes on template argument 'Eigen::internal::packet_traits::type' {aka '__m128d'} [-Wignored-attributes] "C:/PROGRA1/R/R-421.3/bin/x64/Rscript" -e "source(file.path('..', 'tools', 'make_cc.R')); make_cc(commandArgs(TRUE))" stan_files/surv.stan

and then the error states that:

Error in rstan::stanc(file, allow_undefined = TRUE, obfuscate_model_name = FALSE) : 0 Semantic error in 'string', line 404, column 2 to line 411, column 3: 402: } 403: 404: vector quadrature_log_cdf(vector qwts, vector log_hazard, int qnodes, int N) { ^ 405: int M = rows(log_hazard); 406: vector[M] hazard = exp(log_hazard); Real return type required for probability functions ending in _log, _lpdf, _lupdf, _lpmf, _lupmf, _cdf, _lcdf, or _lccdf. Calls: make_cc -> Execution halted make: *** [Makevars.win:28: stan_files/surv.cc] Error 1 rm stan_files/polr.cc stan_files/mvmer.cc stan_files/count.cc stan_files/lm.cc stan_files/jm.cc stan_files/bernoulli.cc stan_files/binomial.cc stan_files/continuous.cc ERROR: compilation failed for package 'rstanarm' removing 'C:/Users/wilhch00/AppData/Local/R/win-library/4.2/rstanarm' restoring previous 'C:/Users/wilhch00/AppData/Local/R/win-library/4.2/rstanarm' Warning messages: 1: In utils::untar(tarfile, ...) : ‘tar.exe -xf "C:\Users\wilhch00\AppData\Local\Temp\RtmpcphJws\file12c849956fc.tar.gz" -C "C:/Users/wilhch00/AppData/Local/Temp/RtmpcphJws/remotes12c83388f58"’ returned error code 1 2: In i.p(...) : installation of package ‘C:/Users/wilhch00/AppData/Local/Temp/RtmpcphJws/file12c83b9a3c1e/rstanarm_2.26.1.tar.gz’ had non-zero exit status

I have tried to also download the survival extension and building the package from R studio as well. But the same issue reoccurs. Any help would be greatly appreciated. Charlotte

CWilhelm-Benartzi avatar Feb 08 '24 17:02 CWilhelm-Benartzi

@bgoodri and @jgabry would you be ok if I took the lead on getting this branch up-to-date and merged? There's a decent amount of demand for it, and it's something I could use for a project as well

andrjohns avatar Feb 09 '24 11:02 andrjohns

That would be great! You could jump into the discussion in https://github.com/stan-dev/rstanarm/issues/570 to get the ball rolling if you want. Part of that discussion was about finding someone to do what you're proposing, so I'm guessing everyone would be in favor.

jgabry avatar Feb 09 '24 17:02 jgabry

Many thanks!

CWilhelm-Benartzi avatar Feb 09 '24 18:02 CWilhelm-Benartzi

That would be amazing if you had the time to do that @andrjohns! I'll post some thoughts in #570, since that seems to be where most of the discussion is happening.

sambrilleman avatar Feb 10 '24 21:02 sambrilleman

Looks like it no longer likes a function named _log_cdf returning a vector instead of a real. Maybe that is a recent change in Stan. Rather than changing the return type, I just changed the function name. But the build is still failing due to some compilation error. I can't easily see in the build log what though. 😞 image

sambrilleman avatar Feb 10 '24 23:02 sambrilleman