Add checks for NA in return status

[rok-cesnovar]

Submission Checklist

• Run unit tests
• Declare copyright holder and agree to license (see below)

Summary

Fixes #543

Copyright and Licensing

Please list the copyright holder for the work you are submitting
(this will be you or your assignee, such as a university or company):
Rok Češnovar

By submitting this pull request, the copyright holder is agreeing to
license the submitted work under the following licenses:

• Code: BSD 3-clause (https://opensource.org/licenses/BSD-3-Clause)
• Documentation: CC-BY 4.0 (https://creativecommons.org/licenses/by/4.0/)

[rok-cesnovar]

This is an easy fix so I think there is no need for reviews, will merge once tests pass.

Thanks @myshkin and @bachlaw for reporting and suggesting the fix.

[codecov-commenter]

Codecov Report

Merging #544 (7c7a2df) into master (ab7a359) will decrease coverage by 1.53%.
The diff coverage is 100.00%.

❗ Current head 7c7a2df differs from pull request most recent head 8a5b87a. Consider uploading reports for the commit 8a5b87a to get more accurate results

@@            Coverage Diff             @@
##           master     #544      +/-   ##
==========================================
- Coverage   92.81%   91.27%   -1.54%     
==========================================
  Files          12       12              
  Lines        3119     3119              
==========================================
- Hits         2895     2847      -48     
- Misses        224      272      +48     

Impacted Files	Coverage Δ
R/model.R	91.00% <100.00%> (-1.05%)	⬇️
R/run.R	95.23% <100.00%> (-0.50%)	⬇️
R/install.R	58.33% <0.00%> (-10.50%)	⬇️
R/utils.R	88.88% <0.00%> (-1.39%)	⬇️
R/csv.R	97.76% <0.00%> (-0.90%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ab7a359...8a5b87a. Read the comment docs.

[jgabry]

This is an easy fix so I think there is no need for reviews, will merge once tests pass.

Thanks @myshkin and @bachlaw for reporting and suggesting the fix.

Thanks!

[rok-cesnovar]

Tests passed, except for the Mac R-devel tests, but that issue is because the R-devel binary isnt available.

[rok-cesnovar]

@myshkin you can install the latest development version now and the fix should be included.

remotes::install_github("stan-dev/cmdstanr")

[myshkin]

Thank you for the quick turnaround! This should help with debugging other issues.

[myshkin]

I will note that our output on erroring jobs now looks like

Loading required package: cmdstanr
This is cmdstanr version 0.4.0.9000
- Online documentation and vignettes at mc-stan.org/cmdstanr
- CmdStan path set to: /bp/home/bpcompute/.cmdstanr/cmdstan-2.27.0-linux-arm64
- Use set_cmdstan_path() to change the path
The C++ toolchain required for CmdStan is setup properly!
Loading required package: pacman
Error: An error occured during compilation! See the message above for more information.

which certainly helps me narrow down where the problem is, but looks misleading, since no relevant message appears above.

[rok-cesnovar]

I would advise setting

options("cmdstanr_verbose" = TRUE)

while debugging stuff like this.

[myshkin]

Thanks for the help. I hope to have another bug report for you soon. In the mean time, when I do get the new

Error: An error occured during compilation! See the message above for more information.

message, the exit code is still 0. Is that working as expected, or should there be a nonzero exit code to indicate an error?

[rok-cesnovar]

the exit code is still 0

Do you mean the exit code ($status) in R or the exit code of the R script you are running?

[myshkin]

Do you mean the exit code ($status) in R or the exit code of the R script you are running?

I meant the exit code of the script; I'm using that to detect whether to rerun the job. Having updated cmdstanr and brms to HEAD of github master, though, I am now getting exit code 1 as expected.