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

some sort of name clash if 'calcNodes' used in run code without being defined #691

Open paciorek opened 6 years ago

paciorek commented 6 years ago

This seems to only occur if 'calcNodes' is used and not 'calcNodes2' so seems like some sort of naming issue with names used internally in Nimble. I.e. presumably 'calcNodes' is found in some context where it shouldn't be and the fact that 'calcNodes' does not exist is not trapped as it should be. Here's the example:

code = nimbleCode({
    y ~ dnorm(mu,1)
    mu ~ dnorm(0,1)
})

model = nimbleModel(code)
cmodel = compileNimble(model)
mv = modelValues(model, m = 2)

myfun <- nimbleFunction(
 setup = function( model, mvSaved, target) {
   calcNodes_full <- model$getDependencies(target)
 },
run = function( ) { 
   copy(from = model, to = mvSaved, row = 1, 
        nodes = calcNodes, logProb = TRUE)    ## 'calcNodes' used mistakenly instead of 'calcNodes_full'
 }
) 

rf = myfun(model, mv, 'mu')
crf =compileNimble(rf, project = model)

Error in as.character(x) : 
  cannot coerce type 'closure' to vector of type 'character'

Error occurs in: nimble/R/typesmodelValuesAccessor.R: isLogProbName <- substr(nodeNames, 1, 8) == 'logProb'

with nodeNames being a function rather than a character vector.

perrydv commented 6 years ago

There is a function called calcNodes in the nimble namespace.

Options:

  1. Decide this is not a problem worth solving, since any name that exists in the nimble namespace could cause this kind of confusion.

  2. Try to enforce that objects only be looked up locally or from setup code. This would make sense, but it may be more pervasive than just copy.

  3. For this particular case, rename the nimble:::calcNodes function, since I think it is only for internal use. This would fix the immediate confusion since calcNodes is a common name in our examples, but it would not solve the problem that names that happen to coincide with something in the nimble namespace could cause confusing error messages.