problematic lookup/scoping for nested nimbleFunctions

[paciorek]

If a user defines a nimbleFunction whose name is the same as the name of a function in the nimble DSL, in compiled code we use the DSL function and not the user's nimbleFunction. E.g., exp, log1p, gamma, etc.

This is a problem for (at least) two reasons:

• A user presumably expects their defined nimbleFunction to take precedence and may not even know the function exists in the DSL. No warning is given.
• Uncompiled execution, via R scoping, will use the user's nf, so uncompiled behavior will differ from compiled.

log1p <- nimbleFunction(
    run=function(x=double(0)) {
        returnType(double(0))
        return(9999)
    })

nf <- nimbleFunction(
    run=function(x=double(0)) {
        out = log1p(x)
        print(out)
    })

cnf <- compileNimble(nf)
cnf(1)  # user would expect 9999
# 0.693147

Some options:

• Simply look to see if an RC function of the same name exists and warn the user to name it differently.
  • This is the minimally intrusive change...
• Reorder the processing in exprClasses_setSizes to look for RC functions first.
  • This feels more dangerous, but I think it is the conceptually correct thing to do.
  • If we do this, we'll need to address the fact that in line 280 of genCpp_sizeProcessing.R, the lookup (via R's scoping) when using get(code$name) will find names in base:: before names in GlobalEnv, which will cause problems with defaulting to using user-defined RC functions.

gamma() is a more complicated variation on this given we translate gamma -> gammafn earlier on...

@perrydv this is not urgent, but we'll want to discuss for 1.3.0.

[paciorek]

Followup: given nCompiler, I think I will propose:

1. We use the minimal approach and warn users.
2. We keep this in mind for nCompiler lookup and handle things more robustly there.

[paciorek]

See related issue #1469, including the possibility of using getAnywhere rather than get.

[perrydv]

@paciorek I agree with both instincts: leave as is because we'll have a revamped system coming into place; and if using getAnywhere and checking what results are actually nimbleFunctions is not too hard to do, I'm not opposed to it. There may be multiple places where this would need to be done. All of that said, I would vote for leaving as is for now.

[paciorek]

I'm error-trapping this via #1517 .