behavior of getParents and implications for setupMargNodes

[paciorek]

I've got nimbleModel's version of setupMargNodes running and am working through basic debugging using the tests in nimble's test-setupMargNodes.R, in particular lines 42-44.

@perrydv I've run into a bit of a tangle that relates to seemingly strange behavior of getParents in nimble and uncertainty on my part about what setupMargNodes is supposed to be doing.

Note that in line 42 of test-setupMargNodes.R, the calcNodes are y[1], and z[1]. In setupMargNodes, we call getParents in this invocation:

reNodesDefault <- intersect(reNodesDefault, model$getParents(calcNodes, upstream = TRUE, stochOnly = TRUE))

It seems that nimble does include y[1] as a parent of calcNodes even though it is in calcNodes, because it is a parent of z[1]. However, this is incompatible with our documentation for the behavior of self=FALSE in getParents and also inconsistent with the behavior of self=FALSE in getDependencies,as shown in this simplified example:

code=nimbleCode({
    x ~ dnorm(0,1)
    y ~ dnorm(x,1)
    z ~ dnorm(y,1)
 })
 m = nimbleModel(code)
m$getParents(c('z','y'),self=FALSE)
# "x", "y"
m$getDependencies(c('x','y'),self=FALSE)
# "z"

I presume (but haven't thought carefully) that in the lines 42-44 test that nimble::setupMargNodes correctly gives y[1] as part of randomEffectsNodes (but I am having trouble parsing the paragraph in ?setupMargNodes that starts with "If randomEffectsNodes are missing" as it's not clear to me what it is saying should happen if paramNodes is not provided, since (iii) seems to be part of the text relating to the case of paramNodes being provided.

Additionally, I'll note that in nimble::setupMargNodes there are various calls to getParents. Some have self=FALSE and some have self missing, which is odd as the default is FALSE. So it makes it seem like it was written thinking that self=TRUE is the default.

Because nimbleModel::getParents excludes y[1] in the case above, it produces an empty randomEffectsNodes, which is inconsistent with nimble and the test in line 44.

So one path forward for nimbleModel::setupMargNodes may be to explicitly have self=TRUE for getParents in those calls where self is missing, but I haven't worked through the logic of whether that is actually correct.

As far as nimble, I'll just leave it that I'm confused as to what we might/should do.

No urgency as I'm just working a few more hours before signing off for my trip and have plenty else to work on.