nimble-dev / nimble

The base NIMBLE package for R
http://R-nimble.org
BSD 3-Clause "New" or "Revised" License
160 stars 24 forks source link

name clashes with imported package functions #1469

Open perrydv opened 5 months ago

perrydv commented 5 months ago

When a user defines a nimbleFunction with the same name as an imported function, compileNimble will fail due to finding the imported function name first. Here is a reprex adapted from a nimble-users message:

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

mc<-nimbleCode({
  a <- normalize(b)
})

m<-nimbleModel(mc,data = list(a = 1))
cm <- compileNimble(m) #Error: In sizeAssignAfterRecursing: 'normalize' is not available or its output type is unknown.

The problem is that the search for normalize finds igraph::normalize because that is imported into nimble's namespace.

We could refine how nimbleFunctions are found (although this has been not as simple as it seems in the past) or at least revise the error message to suggest possible name clashes.

paciorek commented 4 months ago

Only partly related, but we might only import the functions from igraph that we actually use to reduce the clashes.

paciorek commented 4 months ago

Here's my thought on how to handle this.

getAnywhere will return all objects, not just those found first. We can check with is.rcf on all of them and use whichever is an RC function.

We may need to do this in various places so we may want to create our own getRCfunction().

Here's some initial code:

objs <- getAnywhere(code$name)
rcfs <- sapply(objs$objs, is.rcf)

@perrydv let's discuss.

paciorek commented 4 months ago

One more note that is with something like the calcQF nf from BayesNSGP, getAnywhere returns a copy of calcQF from package:BayesNSGP and one from namespace:NSGP.

In general, we'd presumably favor getting the RC function from the global env.

paciorek commented 4 months ago

Interestingly the name clash does not occur for nimbleFunctions in other packages. E.g., one can use addHMC for a user-defined function even if nimbleHMC is loaded. Haven't looked in detail in terms of search path ordering.

We'd need to think about cases where an RC function might be coming from another package, presumably favoring the global env.

And a note that if a user tries to re-use the name of an RC function in nimble, such as CAR_calcC, we don't handle that gracefully. E.g., defining CAR_calcC with a single arg trips on size processing:

Error: In generalFunSizeHandlerFromSymbols: Wrong number of arguments.
 This occurred for: CAR_calcC(adj=model_b[1])
 This was part of the call:  model_a[1] <<- CAR_calcC(adj=model_b[1])
paciorek commented 1 month ago

Realizing this issue is very closely related to issue #1497 .

@perrydv we should discuss for release 1.3.0.