csr_matrix_times_vector2 now uses csr_matrix_times_vector

[SteveBronder]

Following the conversation on a stanc3 pr this modifies csr_times_matrix_vector2 to have the same signature as the new stanc3 generated signature. It also just uses csr_matrix_times_vector since in the newest Stan math there is a reverse mode specialization for the csr_matrix_times_vector function.

[jgabry]

Thanks Steve! Will let @bgoodri review this since it's more in his wheelhouse

[hsbadr]

Following the conversation on a stanc3 pr this modifies csr_times_matrix_vector2 to have the same signature as the new stanc3 generated signature. It also just uses csr_matrix_times_vector since in the newest Stan math there is a reverse mode specialization for the csr_matrix_times_vector function.

@SteveBronder I confirm that this patch fixes the following error:

** testing if installed package can be loaded from temporary location
Error: package or namespace load failed for ‘rstanarm’ in dyn.load(file, DLLpath = DLLpath, ...):
 unable to load shared object 'rstanarm.so':
  rstanarm.so: undefined symbol: _ZN24model_binomial_namespace24csr_matrix_times_vector2IN5Eigen3MapINS1_6MatrixIdLin1ELi1ELi0ELin1ELi1EEELi0ENS1_6StrideILi0ELi0EEEEES4_EENS3_IN5boost4math5tools12promote_argsIN4stan10value_typeIT_vE4typeENSD_IT0_vE4typeEffffE4typeELin1ELi1ELi0ELin1ELi1EEERKiSO_RKSE_RKSt6vectorIiSaIiEESV_RKSH_PSo
Error: loading failed

[SteveBronder]

Yeah it looks like the failure is something about the USELTO field in the description. I think the github action needs updated to a newer version of R

[jgabry]

I think the github action needs updated to a newer version of R

It's already using the latest release and development versions:

https://github.com/stan-dev/rstanarm/blob/ef08f58b1caeecff1a96e1e595f6741798ebbe5f/.github/workflows/R-CMD-check.yaml#L25-L26

but it does seem to be complaining about the UseLTO in the DESCRIPTION file. @bgoodri Any idea why R cmd check (via GitHub actions) says

Unknown, possibly mis-spelled, fields in DESCRIPTION:
  ‘UseLTO’

even though it's running the latest R?

It's also entirely possible that the R cmd check failure has nothing to do with this and it's something else that's causing it to fail (I can't quite tell from the workflow logs if the UseLTO issue is an Error or a Note)

[bgoodri]

Might need R devel or 4.1 when it comes out, but that isn't an error.

On Fri, May 7, 2021 at 3:42 PM Jonah Gabry @.***> wrote:

I think the github action needs updated to a newer version of R

It's already using the latest release and development versions:

https://github.com/stan-dev/rstanarm/blob/ef08f58b1caeecff1a96e1e595f6741798ebbe5f/.github/workflows/R-CMD-check.yaml#L25-L26

but it does seem to be complaining about the UseLTO in the DESCRIPTION file. @bgoodri https://github.com/bgoodri Any idea why R cmd check (via GitHub actions) says

Unknown, possibly mis-spelled, fields in DESCRIPTION:

‘UseLTO’

even though it's running the latest R?

It's also entirely possible that the R cmd check failure has nothing to do with this and it's something else that's causing it to fail (I can't quite tell from the workflow logs if the UseLTO issue is an Error or a Note)

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

[hsbadr]

Yeah it looks like the failure is something about the USELTO field in the description. I think the github action needs updated to a newer version of R

It's just a NOTE, not error, about UseLTO. I think it fails here:

Wrote C++ file "stan_files/polr.cc"
g++ -std=gnu++14 -I"/opt/R/4.0.5/lib/R/include" -DNDEBUG -I"../inst/include" -I"/home/runner/work/_temp/Library/StanHeaders/include/src" -DBOOST_DISABLE_ASSERTS -DEIGEN_NO_DEBUG -I'/home/runner/work/_temp/Library/StanHeaders/include' -I'/home/runner/work/_temp/Library/rstan/include' -I'/home/runner/work/_temp/Library/BH/include' -I'/home/runner/work/_temp/Library/Rcpp/include' -I'/home/runner/work/_temp/Library/RcppEigen/include' -I'/home/runner/work/_temp/Library/RcppParallel/include' -I/usr/local/include  `"/opt/R/4.0.5/lib/R/bin/Rscript" -e "RcppParallel::CxxFlags()"` `"/opt/R/4.0.5/lib/R/bin/Rscript" -e "StanHeaders:::CxxFlags()"` -fpic  -g -O2  -c init.cpp -o init.o
Wrote C++ file "stan_files/continuous.cc"
Error in readRDS("/tmp/RtmpJsRlRF/file755878ce8f61") : 
  error reading from connection
Calls: .Last -> readRDS
Execution halted
make: *** [Makevars:23: stan_files/continuous.cc] Error 1
make: *** Waiting for unfinished jobs....

@jgabry Where did it call readRDS()? I haven't looked at the code yet.

[jgabry]

Might need R devel or 4.1 when it comes out, but that isn't an error. …

It has the same Note about UseLTO with R-devel (we have GitHub actions checking both release and devel). So maybe it needs 4.1. But @bgoodri if it needs 4.1 then how did you get UseLTO to work when you added it? Or am I misunderstanding something?

[jgabry]

@hsbadr thanks for helping out with this!

@jgabry Where did it call readRDS()? I haven't looked at the code yet.

I'm not sure where, we don't call readRDS() in rstanarm itself but maybe R cmd check does?

Where are you finding these lines with the error? I tried searching for "readRDS" in the GitHub actions logs and it doesn't come up with any results.

[hsbadr]

@jgabry Since it doesn't fail locally, could you try to clear the R package cache by changing the number in the following lines: https://github.com/stan-dev/rstanarm/blob/ef08f58b1caeecff1a96e1e595f6741798ebbe5f/.github/workflows/R-CMD-check.yaml#L60-L61 You may try to replace -1- with -2- to build a new cache. The cached packages might need to be rebuilt after R-devel 4.2.

Where are you finding these lines with the error? I tried searching for "readRDS" in the GitHub actions logs and it doesn't come up with any results.

From the logs of the previous GHA.

[jgabry]

Thanks. I just changed the 1s to 2s in https://github.com/stan-dev/rstanarm/commit/a6a8e6b27f9666e97ccd2d4887a84c418dbd458f (let me know if I misunderstood your suggestion). Let's see what happens.

[SteveBronder]

Okay so now I think I'm seeing a real error and I have a fix for it

[hsbadr]

Thanks. I just changed the 1s to 2s in a6a8e6b (let me know if I misunderstood your suggestion). Let's see what happens.

Thanks! That's exactly it. If it fails due to the cached packages, that will fix it. Otherwise, we may find the error in the new logs.

[hsbadr]

@jgabry There's 2 test failures, in test_pp_check.R. That means the build is successful, but the tests aren't.

Error: Running examples in ‘rstanarm-Ex.R’ failed
Error: Error in eval(predvars, data, env) : object 'logBili' not found
Calls: posterior_traj ... model.offset -> model.frame -> model.frame.default -> eval -> eval
Running the tests in ‘tests/testthat.R’ failed.
Last 13 lines of output:
    5. ├─base::suppressWarnings(...)
    6. │ └─base::withCallingHandlers(...)
    7. ├─bayesplot::pp_check(...)
    8. └─rstanarm:::pp_check.stanreg(...)
    9.   ├─rstanarm:::.ppc_y_and_yrep(...)
   10.   │ ├─rstantools::posterior_predict(...)
   11.   │ └─rstanarm:::posterior_predict.stanreg(...)
   12.   │   ├─rstanarm:::pp_args(...)
   13.   │   └─rstanarm:::pp_eta(object, dat, draws, m = m)
   14.   └─rstanarm:::.set_nreps(nreps, fun = plotfun_name)
  
  [ FAIL 2 | WARN 282 | SKIP 2 | PASS 2695 ]
  Error: Test failures

Also, there's a few convergence warnings that could be related:

Warning: Markov chains did not converge! Do not analyze results!

It seems that you may need to update the examples in the tests to make sure they converge, which may be the culprit for object 'logBili' not found error.

The traceback points at bayesplot::pp_check() outside rstanarm, which may have a bug or needs an update.

[hsbadr]

@jgabry Here's the raw logs for the failed tests:

 ══ Failed tests ════════════════════════════════════════════════════════════════
 ── Error (test_pp_check.R:56:5): pp_check.stanreg creates ggplot object ────────
 ##[error]Error: Plotting function not supported. (If the plotting function is included in the output from bayesplot::available_ppc() then it should be available via pp_check and this error is probably a bug.)
 Backtrace:
      █
   1. ├─global::expect_gg(...) test_pp_check.R:56:4
   2. │ └─testthat::expect_is(x, "ggplot", info = info, label = label)
   3. │   └─testthat::quasi_label(enquo(object), label, arg = "object")
   4. │     └─rlang::eval_bare(expr, quo_get_env(quo))
   5. ├─base::suppressWarnings(pp_check(fit, plotfun = f, nreps = j))
   6. │ └─base::withCallingHandlers(...)
   7. ├─bayesplot::pp_check(fit, plotfun = f, nreps = j)
   8. └─rstanarm:::pp_check.stanreg(fit, plotfun = f, nreps = j)
   9.   ├─rstanarm:::.ppc_y_and_yrep(...)
  10.   │ ├─rstantools::posterior_predict(...)
  11.   │ └─rstanarm:::posterior_predict.stanreg(...)
  12.   │   ├─rstanarm:::pp_args(...)
  13.   │   └─rstanarm:::pp_eta(object, dat, draws, m = m)
  14.   └─rstanarm:::.set_nreps(nreps, fun = plotfun_name)

 ── Error (test_pp_check.R:64:5): pp_check.stanreg creates ggplot object for grouped functions ──
 ##[error]Error: Plotting function not supported. (If the plotting function is included in the output from bayesplot::available_ppc() then it should be available via pp_check and this error is probably a bug.)
 Backtrace:
      █
   1. ├─global::expect_gg(...) test_pp_check.R:64:4
   2. │ └─testthat::expect_is(x, "ggplot", info = info, label = label)
   3. │   └─testthat::quasi_label(enquo(object), label, arg = "object")
   4. │     └─rlang::eval_bare(expr, quo_get_env(quo))
   5. ├─base::suppressWarnings(...)
   6. │ └─base::withCallingHandlers(...)
   7. ├─bayesplot::pp_check(...)
   8. └─rstanarm:::pp_check.stanreg(...)
   9.   ├─rstanarm:::.ppc_y_and_yrep(...)
  10.   │ ├─rstantools::posterior_predict(...)
  11.   │ └─rstanarm:::posterior_predict.stanreg(...)
  12.   │   ├─rstanarm:::pp_args(...)
  13.   │   └─rstanarm:::pp_eta(object, dat, draws, m = m)
  14.   └─rstanarm:::.set_nreps(nreps, fun = plotfun_name)
 
 [ FAIL 2 | WARN 282 | SKIP 2 | PASS 2695 ]

[jgabry]

Thanks a lot @hsbadr!

I just fixed the pp_check test on master (I had added a new plotting function in bayesplot that rstanarm didn't know how to handle but the test was set up to try all plotting functions) and I merged the fix into this PR. So now the only failure should be the logBili thing and you can ignore that for now (we know why it happens, just haven't finished fixing it yet). So if this PR passes everything except for the logBili error we can consider that a pass.

[jgabry]

Okay so now I think I'm seeing a real error and I have a fix for it

@SteveBronder Do you mean in one of the tests or when it was trying to build/install?

[hsbadr]

I just fixed the pp_check test on master

Thank you for the quick fix!

So if this PR passes everything except for the logBili error we can consider that a pass.

Yes, I think if it fails on tests after successful build, this PR is ready to go. If not, we know it's due to backward compatibility and should be fixed as suggested above. I've already tested this PR locally with the development versions of Math/Stan and the nightly stanc3.

[jgabry]

I've already tested this PR locally with the development versions of Math/Stan and the nightly stanc3.

Awesome, thank you!

[SteveBronder]

Nvm what I said thank ypu v much @jgabry and @hsbadr

[hsbadr]

Nvm what I said thank ypu v much @jgabry and @hsbadr

As expected, this change isn't backward compatible:

##[error]Error: package or namespace load failed for ‘rstanarm’ in dyn.load(file, DLLpath = DLLpath, ...):
unable to load shared object '/home/runner/work/rstanarm/rstanarm/check/rstanarm.Rcheck/00LOCK-rstanarm/00new/rstanarm/libs/rstanarm.so':
/home/runner/work/rstanarm/rstanarm/check/rstanarm.Rcheck/00LOCK-rstanarm/00new/rstanarm/libs/rstanarm.so:
  undefined symbol: _ZN21model_count_namespace24csr_matrix_times_vector2IddEEN5Eigen6MatrixIN5boost4math5tools12promote_argsIT_T0_ffffE4typeELin1ELi1ELi0ELin1ELi1EEERKiSD_RKNS2_IS7_Lin1ELi1ELi0ELin1ELi1EEERKSt6vectorIiSaIiEESL_RKNS2_IS8_Lin1ELi1ELi0ELin1ELi1EEEPSo
##[error]Error: loading failed

You may use my suggestion above, assuming that stanc3 > 2.26.1 isn't required or somehow check stanc3 version as well.

PS: You can parse the stanc3 version from system.file("stanc.js", package = "rstan") if it exists and define USE_STANC3 accordingly. If it doesn't, then use the old code.

[SteveBronder]

I'm afk but I can have a fix that should makes this backwards compatible

[hsbadr]

I'm afk but I can have a fix that should makes this backwards compatible

Sounds good. I've just mentioned that, in general, if system.file("stanc.js", package = "rstan") exists, we can parse stanc3 version from it and define USE_STANC3.

[hsbadr]

@SteveBronder @jgabry LGTM. It's successfully built with both the CRAN and dev versions of rstan (Stan/Math/stanc3).

[jgabry]

@SteveBronder @jgabry LGTM. It's successfully built with both the CRAN and dev versions of rstan (Stan/Math/stanc3).

Awesome, thanks for checking!

[hsbadr]

@bgoodri @jgabry Is there any reason why this isn't merged yet? We need it to upgrade RStan to 2.27.

[bgoodri]

If it is going to call stan::math::csr_matrix_times_vector, then we might as well just change the Stan code to do that. The only reason we had csr_matrix_times_vector2 was to avoid the slowdown from repeatedly checking constants, but I presume those checks are still happening.

On Sun, Jun 20, 2021 at 6:45 PM Hamada S. Badr @.***> wrote:

@bgoodri https://github.com/bgoodri @jgabry https://github.com/jgabry Is there any reason why this isn't merged yet? We need it to upgrade RStan to 2.27.

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

[hsbadr]

If it is going to call stan::math::csr_matrix_times_vector, then we might as well just change the Stan code to do that.

Agreed. I'd prefer to change it in the Stan code and completely remove csr_matrix_times_vector2.