CRAN NOTE for package using Stan (suggest C++17)

Developers
1

Hi everyone,
I’m in the process of submitting a package which uses Stan to CRAN (Comprehensive R Archive Network), however, I get the following note:
“Specified C++14: please update to current default of C++17”

I have almost zero knowledge of C++ but I think rstantools is using C++14 as in the Makevars file in the src folder it says “CXX_STD = CXX14”.

My question is whether I can get the compiler to use C++17?
My guess is no C++17, but I wanted to double check before I resubmit to CRAN.

Many Thanks,
Philip

1 Like
2

@andrjohns @bgoodri @jonah

1 Like
3

I’m not sure. I’ll check with Ben but @andrjohns might also know.

1 Like
4

I don’t believe the Math version in the CRAN rstan is c++17 compatible, since we only patched the Math library last June (PR here).

@phcooney you can tell CRAN that rstan-based packages are not yet c++17-compatible

2 Likes
5

Hi!

Ha… “tell CRAN to do XY”… is sometimes super hard to do (we are still at 2.21 on CRAN for a reason). @andrjohns how easy is it to have your patch being applied to math 2.21 and/or 2.26? The 2.21 is the almost immortal version on CRAN and 2.26 is the version being about ready for CRAN (for quite some time now). So I wonder what’s the easiest way forward? Tagging @bgoodri here.

Sebastian

6

Ah very true. I can do some testing with 2.26 to see how much work is needed. @bgoodri how are things going with 2.26 and CRAN? Should I start with 2.21 instead?

7

Just flagging, I also ran into this and tried the above (though the wording softened obviously). Currently in a conversation about setting to c++17 and ignoring the compilation warnings (which I obviously don’t really want to do).

8

I would be interested to hear the outcome of your conversation with CRAN.

2 Likes
9

@phcooney, @seabbs, @edm

If your packages are able to compile and install under c++17, then you should be fine to use the c++17 standard. Only a subset of Stan’s c++ had to be updated to compile under c+±17, so your if packages’ models aren’t using any of that problematic c++ then you’ll be fine.

I’m currently running the reverse-dependency checks for updating the rstantools-provided c+±spec to c+±17 and most packages continue to compile and pass, so it doesn’t look like a widespread issue. If the revdep failures are minimal/none, then we’ll look to update rstantools so that packages can avoid this NOTE (fyi @jonah that I’ll be opening a PR/discussion soon)

3 Likes
10

@andrjohns: Thanks for the suggestion. Just to confirm, in order to compile with C++17 I have to change the following flag in my Makevars files (in my R package and not rstantools)?

CXX_STD = CXX17

Thanks,
Philip

1 Like
11

Yep that’s right

3 Likes
12

@andrjohns I’m also in the process of submitting and R package using rstan to CRAN, so I greatly appreciate @phcooney starting this thread. However, when I change the line in the Makevars.win and Makevars files noted by @phcooney and rerun devtools::check(), no errors or warnings are reported but when I look at the Makevars files, they are both edited in the process back to having: CXX_STD = CXX14

I am able to push the Makevars changes (CXX14 → CXX17) to github and see that my github actions still succeed with this change, suggesting there aren’t any problems using C++17.

There don’t seem to be any notes in the output of devtools::check() that suggest when this editing is occurring. Is there something else that has to be changed in the R package (or some other aspect of my system) to prevent this reversion? I am running the checks on a laptop with Windows 11 version 21H2; R version 4.2.1; rstan version 2.26.13; Rcpp 1.0.9; rstantools 2.2.0; and Rtools 4.2.

Best,
Isaac

1 Like
13

@iwv_rna @andrjohns. I may be mistaken but I think you have to change the Makevars in the rstantools (the sub folder in R). When I did that I the Makevars in the package had the CXX17 whenever I built/checked/installed it.

Unfortunately the CRAN message has changed from:
“Specified C++14: please update to current default of C++17” to
“Specified C++14: please drop specification unless essential” :(

@andrjohns Do you have an idea of any other places this changed should be made?

14

Check the SystemRequirements field in your DESCRIPTION, you may be specifying it there

15

I only have SystemRequirements: GNU make in the description file.

16

Here is my brute force solution:

The same for configure.win.

The issue is that rstantools overwrites src/Makevars whenever configure is run.

2 Likes
17

I tried @wds15’s solution, and it fixed the overwriting problem, though looks as if I was wrong about package compiling with C++17 (given @wds15’s helpful explanation of the problem, github actions was obviously also overwriting Makevars during package build). I’ll post the error trace and what seems to be the major error during compilation in case it is of use to anyone, but will just go forward with trying to tell CRAN C++14 is a must for my package. Thank you @phcooney again for the thread and everyone for their help!

Error trace:

<system_command_status_error/rlib_error_3_0/rlib_error/error>
Error in `(function (command = NULL, args = character(), error_on_status = TRUE, …`:
! System command 'Rcmd.exe' failed
---
Exit status: 1
stdout & stderr: <printed>
---
Backtrace:
 1. devtools::build()
 2. pkgbuild::build(path = pkg, dest_path = path, binary = binary, …
 3. withr::with_temp_libpaths(rcmd_build_tools(options$cmd, c(options$path, …
 4. base::force(code)
 5. pkgbuild::rcmd_build_tools(options$cmd, c(options$path, options$args), …
 6. pkgbuild::with_build_tools({ …
 7. withr::with_path(rtools_path(), code)
 8. base::force(code)
 9. base::withCallingHandlers(callr::rcmd_safe(..., env = env, spinner = FALSE, …
10. callr::rcmd_safe(..., env = env, spinner = FALSE, show = FALSE, …
11. callr:::run_r(options)
12. base::with(options, with_envvar(env, do.call(processx::run, c(list(bin, …
13. base::with.default(options, with_envvar(env, do.call(processx::run, …
14. base::eval(substitute(expr), data, enclos = parent.frame())
15. base::eval(substitute(expr), data, enclos = parent.frame())
16. callr:::with_envvar(env, do.call(processx::run, c(list(bin, args = real_cmdargs, …
17. base::force(code)
18. base::do.call(processx::run, c(list(bin, args = real_cmdargs, stdout_line_callback = real_callback(s…
19. (function (command = NULL, args = character(), error_on_status = TRUE, …
20. base::throw(new_process_error(res, call = sys.call(), echo = echo, …
21. | base::signalCondition(cond)
22. (function (e) …
23. asNamespace("callr")$err$throw(e)

Error during compilation:

error: ll of overloaded 'size(const std::vector<stan::math::var_value<double>, std::allocator<stan::math::var_value<double> > >&)ambiguous
      87 |     logp -= sum(log_y) * N / size(y)
         |                              ~~~~^~~
   In file included from C:/Users/isaac/Documents/R/win-library/4.1/StanHeaders/include/stan/math/prim/err/elementwise_check.hpp:6
                    from C:/Users/isaac/Documents/R/win-library/4.1/StanHeaders/include/stan/math/prim/err/check_not_nan.hpp:5
                    from C:/Users/isaac/Documents/R/win-library/4.1/StanHeaders/include/stan/math/prim/err/check_2F1_converges.hpp:5
                    from C:/Users/isaac/Documents/R/win-library/4.1/StanHeaders/include/stan/math/prim/err.hpp:4
                    from C:/Users/isaac/Documents/R/win-library/4.1/StanHeaders/include/stan/math/prim.hpp:12
                    from C:/Users/isaac/Documents/R/win-library/4.1/StanHeaders/include/src/stan/io/dump.hpp:10
                    from C:/Users/isaac/Documents/R/win-library/4.1/rstan/include/rstan/stan_fit.hpp:43
                    from C:/Users/isaac/Documents/R/win-library/4.1/rstan/include/rstan/rstaninc.hpp:4
                    from stanExports_MCMC.h:23
                    from stanExports_MCMC.cc:5
   C:/Users/isaac/Documents/R/win-library/4.1/StanHeaders/include/stan/math/prim/fun/size.hpp:28:15:
1 Like
18

For reference I managed to get rid of the note completely by adding
SystemRequirements: C++17 (to the description file)

Based on this page it suggests that it is unnecessary to modify the Makevars file, although I haven’t tested this.

19

Yep, that’s unfortunately exactly one of the issues that required patching - the Math size() function was colliding with the std::size() function

20

Would you be able to share a link to your package source (or PM me a zip/tar file)? We might (emphasis on might) be able to provide a temporary fix through rstantools by prepending explicit overloads to the generated c++, until we can get an updated version of rstan out