nimble-dev / nimble

The base NIMBLE package for R
http://R-nimble.org
BSD 3-Clause "New" or "Revised" License
156 stars 23 forks source link

Lack of sampler should err #1340

Closed BertvanderVeen closed 1 year ago

BertvanderVeen commented 1 year ago

If assigning a sampler to a non-stochastic node, nimble gives a warning and assigns no sampler. However, one can still execute the MCMC. This seems a bit odd, and I wonder if it might be better for nimble to throw an error if one/multiple of the nodes lacks a sampler. When running chains in parallel, warnings are (usually?) not returned, so that it is (very) easy to overlook this warning. Especially when assigning a block sampler to multiple stochastic nodes and a non-stochastic node. Example:

set.seed(1)
p <- 15    # number of explanatory variables
n <- 100   # number of observations
X <- matrix(rnorm(p*n), nrow = n, ncol = p) # explanatory variables
true_betas <- c(c(0.1, 0.2, 0.3, 0.4, 0.5), rep(0, p-5)) # coefficients
sigma <- 1
y <- rnorm(n, X %*% true_betas, sigma)

code <- nimbleCode({
  beta0 ~ dnorm(0, sd = 100)
  for(k in 2:p)
    beta[k] ~ dnorm(0, sd = 100)
  sigma ~ dunif(0, 100)  # prior for variance components based on Gelman (2006)
  beta[1] <- 1
  for(i in 1:n) {
    y[i] ~ dnorm(beta0 + inprod(beta[1:p], x[i, 1:p]), sd = sigma)
  }
})

X <- sweep(X, 2, colMeans(X))  # center for better MCMC performance

constants <- list(n = n, p = p, x = X)
data <- list(y = y)
inits <- list(beta0 = mean(y), beta = rep(0, p), sigma = 0.5)
mod <- nimbleModel(code, constants = constants, data = data, inits = inits)
model <- compileNimble(mod)
conf <- configureMCMC(model)
conf$removeSampler("beta")
conf$addSampler("beta", "AF_slice") # throws warning
mcmc <- buildMCMC(conf) # another warning
cmcmc <- compileNimble(mcmc, project = model)
res <- runMCMC(cmcmc,  niter=20, nburnin = 1, thin=1, 
               nchains = 1, samplesAsCodaMCMC = TRUE) # runs while all beta nodes lack a sampler
danielturek commented 1 year ago

@BertvanderVeen Thanks for raising your concern. What you're seeing is in fact the desired behavior of nimble.

In the MCMC only stochastic nodes are assigned MCMC samplers. In your model, those are the beta[i] nodes for i = 2, 3, ... 15. Deterministic nodes (beta[1] in your model) do not require, and also do not permit MCMC samplers to be assigned. Rather, the value of deterministic model node(s) are updated (when, and only when needed) during the course of MCMC sampling of the stochastic model nodes. The rational here is that stochastic nodes have prior distributions, and hence require MCMC sampling algorithms to sample from their posterior distributions; in contrast, deterministic nodes are (more simply) quantities defined in terms of other model nodes/values, so they do not require MCMC samplers, and rather have their values (deterministically) calculated when necessary (also note, the values of deterministic are always kept in-sync with the value of other model stochastic and deterministic quantities).

What's happening in your example:

## creates the default MCMC configuration,
## with MCMC samplers applied to beta[i] for i = 2,3,4, ... 15
conf <- configureMCMC(model)

## removes any & all samplers applied to any elements of beta,
## which here are the samplers operating on beta[i] elements for i = 2, 3, 4, ... 15
conf$removeSampler("beta")

## attempt to add an AF_slice sampler to *all* elements of beta, which
## includes the deterministic node beta[1], and since that is disallowed,
## no sampler is assigned.
## a warning message to this affect is issued.
conf$addSampler("beta", "AF_slice") # throws warning

You'll note that typing printing information about the configuration conf at this point also informs you that no samplers are assigned to 14 nodes, and tells you how to get these node names.

You also get this same warning at MCMC build time buildMCMC(conf).

What you should do, to get your desired behavior, is:

conf$addSampler("beta[2:15]", "AF_slice")  ## works as desired

I hope this helps explain the system, and gets you up and running.

BertvanderVeen commented 1 year ago

Thanks Daniel. I'm OK with this behavior and understand that deterministic nodes cannot have a snapper assigned.

My issue was with the fact that nimble allows me to execute the MCMC without having samplers assigned, and I personally think it would be more intuitive if this would err. Mostly (as written) because warnings and messages are not seen when using nimble in combination with parallel computing.

So to reiterate, my issue is not that nimble should allow sampling deterministic nodes (obviously, that is nonsensical), but that it might be preferred if nimble throws an error if one has no sampler assigned for stochastic nodes in the model (for whatever reason). So that (if by accident) I do end up assigning a block sampler to both stochastic nodes and deterministic nodes, either 1) nimble only assigns the sampler to the stochastic nodes in the block, or 2) throws an error and prevents me from executing the mcmc.

danielturek commented 1 year ago

@BertvanderVeen Thanks for following up. Behind the scenes, your issue also prompted me to open an internal issue (among the nimble team), the gist of which is:

Would we want an MCMC option MCMCunsampledNodesCausesError ? If TRUE, then buildMCMC would give a hard stop() if there are any unsampled nodes. Default value of this option could go either way, but maybe we would consider TRUE ?

BertvanderVeen commented 1 year ago

@danielturek awesome. Thanks for that clarification.