Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

name clashes with imported package functions #1469

Open
perrydv opened this issue Jun 24, 2024 · 5 comments
Open

name clashes with imported package functions #1469

perrydv opened this issue Jun 24, 2024 · 5 comments

Comments

@perrydv
Copy link
Contributor

perrydv commented Jun 24, 2024

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
Copy link
Contributor

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

@paciorek
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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

@perrydv we should discuss for release 1.3.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants