nimble icon indicating copy to clipboard operation
nimble copied to clipboard

tricky cases of incorrect constants

Open perrydv opened this issue 7 years ago • 5 comments

Here is another user problem reduced to minimal reproducible example:

library(nimble)

data <- list(nsites = 10, C = matrix(rnorm(20), nrow = 10))

nsite <- 11

code <- nimbleCode({
    for(s in 1:nsite) x[s] <- 2 * C[s, 1]
})

m <- nimbleModel(code, constants = data)
m <- nimbleModel(code, data = data)

There is a typo where the code uses nsite, but the data contains nsites. To complicate the situation, there is a global variable nsite.

In the first call to nimbleModel, the global nsite is accidentally picked up for one step, creating an obscure error.

In the second call, more helpful information is generated. While not pinpointing the problem, it may be enough for a user to notice their typo.

perrydv avatar Oct 13 '17 17:10 perrydv

'nsite' is being found via R's scoping rules based on the enclosing environment of constantsEnvCopy, which is passed into determineContextSize. I think we want to force the enclosing env't for constantsEnvCopy to be emptyenv(), so that we don't look outside the model inputs for values in expandContextAndReplacements().

As far as I see, it's a bug that we ever get values from the enclosing environment of the constants, though I might be missing something.

I'm happy to fix this up, but would be good to have @perrydv or @danielturek give a second eye here.

One other comment here is that the actual user experience with constants given this is that one can actually just set the index limit as an R object in the global env't, not in constants and things will "work".

paciorek avatar Jan 26 '18 21:01 paciorek

Doesn't seem ideal that NIMBLE will "find" things in the R global environment. Would be better to enforce they're properly passed in via constants. If setting the enclosing environment to an empty environment will accomplish that (I think it would?), then it sounds reasoanble to me.

danielturek avatar Jan 26 '18 21:01 danielturek

I don't have my head fully in the issue, but it sounds reasonable to me.

perrydv avatar Jan 29 '18 20:01 perrydv

Ok, well this turns out to be quite tricky, so I'm dropping this for now...

When we do the various eval's, we are eval'ing code like:

for (s in nm_seq_noDecrease((1), (nsite))) iAns <- iAns + 1

which means R needs to find for, nm_seq_noDecrease, nsite and iAns. So emptyenv() as the parent won't work. We need to be able to find stuff from baseenv() and from the nimble namespace including non-exported functions. I see no way to do that while not having GlobalEnv in the series of enclosing environments.

Moral of the story - we're operating in R, so it's hard to get around R's scoping rules.

paciorek avatar Jan 30 '18 02:01 paciorek

This came up again in a nimble-users post on 2022-12-27.

paciorek avatar Jan 02 '23 23:01 paciorek