nimble-dev / nimble

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

issue with error trapping deterministic data #1324

Closed paciorek closed 1 year ago

paciorek commented 1 year ago

Report from Daniel:

Example below works correctly in previous versions.

code <- nimbleCode({
    for(i in 1:N) {
        x[i,1] ~ dbern(0.5)
        for(j in 1:M) {
            y[i,j] ~ dnorm(x[i,j], 1)
        }
    }
})

N <- 2
M <- 3
constants <- list(N=N, M=M)
data <- list(
    x = matrix(c(0,  0, 0,
                 NA, 0, 0), byrow = TRUE, nrow = 2),
    y = array(0, c(N,M))
)
inits <- list(
    x = matrix(c(NA, NA, NA,
                 0,  NA, NA), byrow = TRUE, nrow = 2)
)

Rmodel <- nimbleModel(code, constants, data, inits)
Rmodel$calculate()   ## NA
Rmodel$x             ## not set
Rmodel$isDataEnv$x   ## not correct
danielturek commented 1 year ago

Problem appears to be the result of commit 46e273cc0034c9b6c4ac7cefe90c429bd8b9c7c4

perrydv commented 1 year ago

What's happening is that the NAs in inits are over-writing the data values for the RHSonly nodes, in model$setData. This happens only in the case that the variable has a mix of RHSonly and stochastic nodes. The problem arises because in that case model$setData copies only the non-data elements into the variable, and the RHSonly elements are now non-data (they were previously data), so they are copied from inits, and that results in the NAs being used when they shouldn't be.

A workaround is to replace the NAs in the inits with the same data values for the RHSonly portion of the variable. In the above example, this change appears to fix it:

inits <- list(
  x = matrix(c(NA, 0, 0,
               0,  0, 0), byrow = TRUE, nrow = 2)
)
paciorek commented 1 year ago

I'll say more in the design document on nimbleCoreTeam, but just a couple notes here that I am seeing things differently than @perrydv :

code <- nimbleCode({y ~ dnorm(mu,1)})
m=nimbleModel(code, data = list(y=1, mu=1), inits = list(mu=0))
m$mu   # `0` as of version 1.0.0; `1` previously