use cran-like conditions for memory check; update loo to use posterio…

…r predictions
nicholasjclark · May 9, 2024 · 824b7ab · 824b7ab
1 parent ca8d933
commit 824b7ab
Show file tree

Hide file tree

Showing 8 changed files with 39 additions and 20 deletions.
diff --git a/.github/workflows/memcheck.yaml b/.github/workflows/memcheck.yaml
@@ -13,6 +13,7 @@ jobs:
     runs-on: ubuntu-latest
     env:
       GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}
+      NOT_CRAN: false
 
     steps:
       - uses: actions/checkout@v2
@@ -46,20 +47,12 @@ jobs:
               collapse
               rmarkdown
               ggplot2
-              rjags
               coda
-              stan-dev/cmdstanr
               testthat
               usethis
               rcmdcheck
               devtools
 
-      - name: Build Cmdstan
-        run: |
-          cmdstanr::check_cmdstan_toolchain(fix = TRUE)
-          cmdstanr::install_cmdstan()
-        shell: Rscript {0}
-
       - name: Memory check
         run: |
           R -d "valgrind --tool=memcheck --leak-check=full" --vanilla < memcheck.R
diff --git a/NEWS.md b/NEWS.md
@@ -1,2 +1,7 @@
-# mvgam 1.10
-*   First release of `mvgam` to CRAN
+# mvgam 1.1.1
+* Changed indexing of an internal c++ function after Prof Brian Ripley’s   
+  email: Dear maintainer, Please see the problems shown on 
+  https://cran.r-project.org/web/checks/check_results_mvgam.html. Please correct   before 2024-05-22 to safely retain your package on CRAN. The CRAN Team
+
+# mvgam 1.1.0
+* First release of `mvgam` to CRAN
diff --git a/R/loo.mvgam.R b/R/loo.mvgam.R
@@ -54,7 +54,16 @@
 #'}
 #' @export
 loo.mvgam <- function(x, ...) {
-  logliks <- logLik(x, include_forecast = FALSE)
+  x$series_names <- levels(x$obs_data$series)
+  logliks <- logLik(x,
+                    linpreds = predict(x,
+                                       newdata = x$obs_data,
+                                       type = 'link',
+                                       summary = FALSE,
+                                       process_error = TRUE),
+                    newdata = x$obs_data,
+                    family_pars = mvgam:::extract_family_pars(x),
+                    include_forecast = FALSE)
   logliks <- logliks[,!apply(logliks, 2, function(x) all(!is.finite(x)))]
 
   releffs <- loo::relative_eff(exp(logliks),

diff --git a/cran-comments.md b/cran-comments.md
@@ -1,13 +1,8 @@
-## Initial release (version 1.1.0)
+## Version 1.1.1
 
 ## Response to previous check comments
-* DESCRIPTION has been shortened appropriately and the References tag placed in the correct location
-* Missing Rd tags have been added for all exported functions
-* Code that previously wrote home filespace has been removed
-* Examples that previously used non-exported functions have been fixed
-* Replacing `\dontrun` with `\donttest` throughout will almost certainly cause problems in any later CRAN checks because many of these examples take some time due to the need for 'Stan' models to compile. Specifically, using `--run-donttest` the examples take between 48 and 60 minutes, depending on the test environment. A very similar R package that is on CRAN ('brms') sticks to the `\dontrun` convention because of this, so I have elected to only use `\donttest` in the examples for the package's primary functions `mvgam()` and `get_mvgam_priors()` 
-* `onexit()` has been used as suggested to ensure the user's `par` is not changed
-* `options()` have been reset to user defaults in `man` pages as suggested
+* Changed indexing of an internal c++ function after Prof Brian Ripley’s email: Dear maintainer, Please see the problems shown on https://cran.r-project.org/web/checks/check_results_mvgam.html. Please correct before 2024-05-22 to safely retain your package on CRAN. The CRAN Team. I presume this was triggered by a memory 'Invalid read of size' message from `valgrind`, which occurred in one of the examples and one of the tests. Strangely this behaviour did not occur in other examples that use identical codes, so I suspect it was a false positive. But nevertheless I have made some changes and checked with `valgrind` (see '`valgrind` memory check results' below)
+* Also reduced sizes of vignette html files in response to several NOTEs about the large package install size
 
 ## Test environments
 * Windows install: R 4.3.1
@@ -20,4 +15,7 @@
 ## R CMD check results
 * There were no ERRORs or WARNINGs. There were 2 NOTEs due to listing 'cmdstanr' in Suggests. This package is not a dependency but provides an additional backend option for users to select when fitting 'Stan' models, if they wish. A similar package that has been available on CRAN for quite some time ('brms') uses the same convention. I have included the `Additional_repositories` field in the DESCRIPTION to appropriately tell users where they can find this package.
 
+## `valgrind` memory check results
+* Running all examples using `--run-donttest`, and all package tests (including those skipped on CRAN) with `R -d "valgrind --tool=memcheck --leak-check=full"` resulted in no WARNINGs or ERRORs
+
 Maintainer: 'Nicholas J Clark <[email protected]>'
diff --git a/src/mvgam.dll b/src/mvgam.dll
diff --git a/tests/testthat/Rplots.pdf b/tests/testthat/Rplots.pdf
diff --git a/tests/testthat/test-example_processing.R b/tests/testthat/test-example_processing.R
@@ -476,7 +476,6 @@ test_that("loo() works correctly", {
                       mvgam:::mvgam_example2,
                       model_names = c('banana')))
   expect_true(inherits(p, 'compare.loo'))
-  expect_true(all(is.na(dimnames(p)[[1]]) == c(FALSE, TRUE)))
 
   p <- SW(loo_compare(mvgam:::mvgam_example1,
                       mvgam:::mvgam_example2,

diff --git a/tests/testthat/test-sim_mvgam.R b/tests/testthat/test-sim_mvgam.R
@@ -29,3 +29,18 @@ test_that("n_lv must be a positive integer", {
                "Argument 'n_lv' must be a positive integer")
 })
 
+test_that("run sim AR and VAR functions for memory checks", {
+  expect_no_error(capture_output(sim_mvgam(family = gaussian(),
+                                           trend_model = RW())))
+  expect_no_error(capture_output(sim_mvgam(family = gaussian(),
+                                           trend_model = AR(p = 1))))
+  expect_no_error(capture_output(sim_mvgam(family = gaussian(),
+                                           trend_model = AR(p = 2))))
+  expect_no_error(capture_output(sim_mvgam(family = gaussian(),
+                                           trend_model = AR(p = 3))))
+  expect_no_error(capture_output(sim_mvgam(family = gaussian(),
+                                           trend_model = VAR())))
+  expect_no_error(capture_output(sim_mvgam(family = gaussian(),
+                                           trend_model = VAR(cor = TRUE))))
+})
+