Too much potential for model node name conflicts from lifted nodes
The title says it all. Just like how we rename the internal (lifted, and otherwise) node functions with suffixes such as "xxx_ L11", and also for lifted nodes with prefixes such as "lifted_xxx", I think we need to be more careful to give unique names to new nodes created from LHS transformations.
For example, I don't think the lifted nodes named "log_a" and "logit_b" in this example are unique enough. I would suggest, perhaps, the suffix with line numbers, like "log_a_L2", or something like that, if not using the "unique ID" system.
Example:
library(nimble)
code <- nimbleCode({
log(a) ~ dnorm(0, 1)
logit(b) ~ dnorm(0, 1)
})
Rmodel <- nimbleModel(code)
Rmodel$getNodeNames()
## [1] "log_a" "logit_b" "a" "b"
Additional historical perspective: this current system for handling the LHS-transformations dates way back, to the "new model def" system, c 2014.
While clear and functional, all our other systems for auto-generation of names have since been updated and modernized to prevent accidental conflicts. But this system appears to have remained in its original form.
For LHS nodes as in your examples, we could do that, but I'm not convinced we need to. Is there a case where a name clash can be demonstrated?
For RHS nodes, the lifted names are used to determine redundancies, so that the same lifting is not done multiple times. E.g.
for(i in 1:5) {
x[i] <- dnorm(mu_x, tau)
}
for(i in 1:5) {
y[i] <- dnorm(mu_y, tau)
}
The lifted sd node for 1/sqrt(tau) will be lifted only once, and the matching name transformations are what identify the second case as having the same need as the first case. The intention was that the lifted node name is a uniquely string-ified version of the expression.
Example of name clash:
library(nimble)
code <- nimbleCode({
log(a) ~ dnorm(0, 1)
logit(b) ~ dnorm(0, 1)
logit_b ~ dexp(3)
})
Rmodel <- nimbleModel(code)
## Error: There are multiple definitions for nodes:logit_b
## If your model has macros or if-then-else blocks
## you can inspect the processed model code by doing
## nimbleOptions(stop_after_processing_model_code = TRUE)
## before calling nimbleModel.
Thanks. That's helpful. How about the lifted node can end in "lifted", e.g. "logit_b_lifted_"?
Sure, sounds good. A unique identifier postfix "UID##" would be additionally safe.
I think the "UID##" suggestion would make it difficult to use the name as a one-to-one encoding of the expression for purposes of determining when a lifted node for some expression already exists.