nimble-dev / nimble

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

Bug with 3-dimension parameters and getParam() ? #640

Open danielturek opened 7 years ago

danielturek commented 7 years ago

New error that comes up, as best I can tell looks like resulting from having 3-dimensional parameters to custom distribution functions.

Upon model building, this results in an error like:

> Rmodel <- nimbleModel(code, constants, data, inits, check = FALSE)
defining model...
Adding prior,condition as data for building model.
building model...
setting data and initial values...
running calculate on model (any error reports that follow may simply reflect missing values in model variables) ... 
checking model sizes and dimensions...Error in envRefInferField(x, what, getClass(class(x)), selfEnv) : 
  'getParam_3D_double' is not a valid field or method name for reference class "y_L64_UID_52"
Error in envRefInferField(x, what, getClass(class(x)), selfEnv) : 
  'getParam_3D_double' is not a valid field or method name for reference class "y_L64_UID_52"

Reproducible example: goto github repository: danielturek/HMM-MCMC (any of the "goose" or "orchid" scripts will demonstrate this error).

For example, download: HMM-MCMC/data/gooseData.RData, and execute the script: HMM-MCMC/code/goose_FilterRR.R

Sorry this example is far from "minimal".....

paciorek commented 6 years ago

We only provide up to 2D for getParam. This should be straightforward to extend.

@danielturek What is the use case / what timeline would be helpful on this?

danielturek commented 6 years ago

No rush whatsoever. I think it was only that the legacy HMM-MCMC code no longer works, since it uses custom distributions that have 3D+ parameters. Some folks were trying to use the code (posted along with the paper), and it gave errors / warnings. But I'd forgotten about this, it was more a problem for them rather than me, so it's not inconveniencing me. But, it does sound like something that we should cover, if it's not too difficult.

paciorek commented 6 years ago

ok, that's helpful to know - if we have examples out there associated with the paper that now fail, that's bad. I'll see if I have time over weekend to fix up.

paciorek commented 6 years ago

Looks like the issue here will be that getParam goes through eigenization, so we get this error even if the calculations with the 3-d param don't actually do any non-allowed math.

>     cm <- compileNimble(m)
compiling... this may take a minute. Use 'showCompilerOutput = TRUE' to see C++ compiler details.
Error : Cannot do math with arrays that have more than 2 dimensions.
 This occurred for: PARAMANSWER_
 This was part of the call:  PARAMANSWER_ <- map("model_theta",3,0,list,list)
Error: There is some problem at the Eigen processing step for this code:
 {
    nimSwitch(paramID = PARAMID_, IDoptions = 2L, theta = PARAMANSWER_ <- model$theta[1:2, 1:3, 1:4])
    return(PARAMANSWER_)
}
danielturek commented 6 years ago

So, just to check the implications here, if there isn't a work-around for this.

Is it that we can't have user-defined distributions, with parameters of dimension > 2 ?

On Fri, Jan 26, 2018 at 10:01 PM, Christopher Paciorek < notifications@github.com> wrote:

Looks like the issue here will be that getParam goes through eigenization, so we get this error even if the calculations with the 3-d param don't actually do any non-allowed math.

cm <- compileNimble(m)

compiling... this may take a minute. Use 'showCompilerOutput = TRUE' to see C++ compiler details. Error : Cannot do math with arrays that have more than 2 dimensions. This occurred for: PARAMANSWER This was part of the call: PARAMANSWER <- map("modeltheta",3,0,list,list) Error: There is some problem at the Eigen processing step for this code: { nimSwitch(paramID = PARAMID, IDoptions = 2L, theta = PARAMANSWER <- model$theta[1:2, 1:3, 1:4]) return(PARAMANSWER) }

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/nimble-dev/nimble/issues/640#issuecomment-360954990, or mute the thread https://github.com/notifications/unsubscribe-auth/AETW4Uu4njO8VQx5l4CRvb791sGuK4F5ks5tOpGbgaJpZM4QnnBr .

paciorek commented 6 years ago

My thought at the moment is that we disable getParam for 3d params.

danielturek commented 6 years ago

Sounds reasonable to me. This just means we could not use getParam() for distribution parameters of dimension > 2, right? But custom distribution functions could still be defined with parameters of dimension 3 or 4, with no problems?

On Fri, Jan 26, 2018 at 11:24 PM Christopher Paciorek < notifications@github.com> wrote:

My thought at the moment is that we disable getParam for 3d params.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/nimble-dev/nimble/issues/640#issuecomment-360958978, or mute the thread https://github.com/notifications/unsubscribe-auth/AETW4SFYdB_elXrZDFsN2BATnggoZ1DEks5tOqUXgaJpZM4QnnBr .

paciorek commented 6 years ago

In branch getParam_3D, I monkeyed with model processing (in particular checkBasics) so that no error (just a warning) is issued for models with distributions with params > 3D. Calls to getParam for this case will return NULL in compiled or uncompiled model execution from R, but compilation of nf's that attempt to use getParam_3D will fail. I think this is a reasonable compromise that allows user-defined dists with params with > 2 dimensions but curious to hear @perrydv 's reaction.

perrydv commented 6 years ago

Right, we don't do any arithmetic in more than 2D. But we do do basic copy operations. So I think there should be a way to modify the getParam code so that if it only needs to return a 3D object, it should work (no alternative parameterizations would work). I'd have to look more carefully to see what is going on right now. Let me know if I should prioritize that tomorrow.

But in the meantime, the solution of disabling getParam in order to allow 3D parameters makes sense to me.

paciorek commented 6 years ago

I don't think it's a high priority for you, @perrydv . My thought at this point is to merge in what we have now and leave this issue open, given the extension to allowing a getParam_3D that just returns the value w/o alt params.

xfim commented 4 years ago

Dear all,

Any advance with parameters > 2D? I am having the same issue with y[a,b,c] ~ ... and would like to know if I can expect to work out of the box at some point.

Thank you.

paciorek commented 4 years ago

Hi Xavier, I'd like to better understand the issue you are having and see how it relates to this Github issue. Rather than having a lot of back and forth here, could you just email me the details of the problem you are having? In particular a small reproducible example or at least the code for your model and the error you are getting would be helpful. Please email me with my last name @ berkeley.edu.

paciorek commented 4 years ago

Just a note that Xavier's problem was unrelated to getParam().

telenskyt commented 2 years ago

Looks like I have the same problem, running pow() function over 3D array, here the nimble error output:

Error in checkForRemoteErrors(val) : 
  3 nodes produced errors; first error: There is some problem at the Eigen processing step for this code:
 {
    model$fit[1] <<- sum(pow((pow(model$F[1:39, 1:17, 1:18], 0.5) - pow(model$E.F[1:39, 1:17, 1:18], 0.5)), 2)) + sum(pow((pow(model$R[1:39, 1:16, 1:17], 0.5) - pow(model$E.R[1:39, 1:16, 1:17], 0.5)), 2))
}

The code looks like this (it is basically Posterior Predictive Check code from Marc Kéry's book - Kéry, M., Schaub, M., 2012. Bayesian Population Analysis using WinBUGS. Elsevier. https://doi.org/10.1016/C2010-0-68368):

fit <- sum(pow((pow(F[,,], 0.5)-pow(E.F[,,], 0.5)), 2)) + sum(pow((pow(R[,,], 0.5)-pow(E.R[,,], 0.5)), 2))
fit.new <- sum(pow((pow(F.new[,,], 0.5)-pow(E.F[,,], 0.5)), 2)) + sum(pow((pow(R.new[,,], 0.5)-pow(E.R[,,], 0.5)), 2))

Is there any plan to fix this?

telenskyt commented 2 years ago

Now I see that even a simple sum() over a 3-dimensional array generates this error! Does this mean that 3D arrays are completely not working in Nimble?

[1] "2021-12-08 04:30:11 CET"
Error in checkForRemoteErrors(val) : 
  3 nodes produced errors; first error: There is some problem at the Eigen processing step for this code:
 {
    model$ahoj[1] <<- sum(model$f.i.t[1:39, 1:17, 1:18])
}
paciorek commented 2 years ago

Here's a simple reprex:

code <- nimbleCode({
    for(i in 1:3)
        for(j in 1:4)
            for(k in 1:5)
                y[i,j,k] <- 1
})

m <- nimbleModel(code, inits = list(y = array(1, c(3,4,5))))

nf <- nimbleFunction(
    setup = function(model){},
    run = function() {
        returnType(double(0))
        out <- sum(model$y[1:2,1:3,1:4])
        return(out)
    })

rnf <- nf(m)
rnf$run()
cm <- compileNimble(m)
cnf <- compileNimble(rnf, project = m)
# Error in eigenizeNameStrided(code, symTab, typeEnv, workEnv) : 
#  Error, cannot eigenize a map of dimensions > 2
# Error: There is some problem at the Eigen processing step for this code:

We'll take a look. 3-d (and higher) arrays are allowed but obviously some aspect of working with them needs fixing.