Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix adaptive proposal logic #221

Merged
merged 1 commit into from Jun 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions DESCRIPTION
@@ -1,6 +1,6 @@
Package: spimalot
Title: Support for 'sircovid'
Version: 0.7.28
Version: 0.7.29
Authors@R:
c(person(given = "Marc",
family = "Baguelin",
Expand Down Expand Up @@ -49,7 +49,7 @@ Description: Code that supports that use of sircovid for our regular
License: MIT + file LICENSE
Encoding: UTF-8
Roxygen: list(markdown = TRUE)
RoxygenNote: 7.1.2
RoxygenNote: 7.2.0
URL: https://github.com/mrc-ide/spimalot
BugReports: https://github.com/mrc-ide/spimalot/issues
Depends: R (>= 3.5.0)
Expand Down
9 changes: 5 additions & 4 deletions R/control.R
Expand Up @@ -48,9 +48,9 @@
##' @param mcmc_path Path to store the mcmc results in
##'
##' @param adaptive_proposal Control the adaptive proposal. By default
##' this is disabled (value of `NULL`). This can only be enabled for
##' determinsitic models. Pass either `TRUE` here or the results
##' from [mcstate::adaptive_proposal_control()]
##' this is disabled (value of `NULL` or `FALSE`). This can only be
##' enabled for determinsitic models. Pass either `TRUE` here or the
##' results from [mcstate::adaptive_proposal_control()]
##'
##' @param verbose Logical, indicating if we should print information
##' about the parallel configuration
Expand Down Expand Up @@ -85,7 +85,8 @@ spim_control <- function(short_run, n_chains, deterministic = FALSE,

## Once happy here, you could change the default behaviour, so that
## if determinsitic and not `FALSE` we set this up.
if (!is.null(adaptive_proposal) && !deterministic) {
adaptive_proposal <- adaptive_proposal %||% FALSE
if (isTRUE(adaptive_proposal) && !deterministic) {
stop("Can't use adaptive_proposal with non-deterministic models")
}

Expand Down
6 changes: 3 additions & 3 deletions man/spim_control.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

29 changes: 21 additions & 8 deletions tests/testthat/test-control.R
Expand Up @@ -167,14 +167,27 @@ test_that("Control compiled compare", {


test_that("Disallow adaptive propsosal with stochastic models", {
expect_error(
spim_control(TRUE, 4, adaptive_proposal = TRUE, verbose = FALSE),
"Can't use adaptive_proposal with non-deterministic models")
expect_silent(
spim_control(TRUE, 4, adaptive_proposal = NULL, verbose = FALSE))

ctl <- spim_control(TRUE, 4, adaptive_proposal = TRUE, deterministic = TRUE,
verbose = FALSE)
ctl <- spimalot::spim_control(TRUE, 4, deterministic = FALSE,
adaptive_proposal = FALSE)
expect_null(ctl$adaptive_proposal)
ctl <- spimalot::spim_control(TRUE, 4, deterministic = FALSE,
adaptive_proposal = NULL)
expect_null(ctl$adaptive_proposal)
expect_error(spimalot::spim_control(TRUE, 4, deterministic = FALSE,
adaptive_proposal = TRUE),
"Can't use adaptive_proposal with non-deterministic models")
})


test_that("control adaptive proposal with deterministic models", {
ctl <- spimalot::spim_control(TRUE, 4, deterministic = TRUE,
adaptive_proposal = FALSE)
expect_null(ctl$adaptive_proposal)
ctl <- spimalot::spim_control(TRUE, 4, deterministic = TRUE,
adaptive_proposal = NULL)
expect_null(ctl$adaptive_proposal)
ctl <- spimalot::spim_control(TRUE, 4, deterministic = TRUE,
adaptive_proposal = TRUE)
expect_equal(ctl$pmcmc$adaptive_proposal,
mcstate::adaptive_proposal_control())
})