Do we need to require rjags? Forces users to install JAGS, seems cumbersome

[jkarreth]

Just coming across this now while teaching using Stan. Several participants report this error message when installing BayesPostEst:

The rjags package is just an interface to the JAGS library
Make sure you have installed JAGS-4.x.y.exe (for any x >=0, y>=0) from
http://www.sourceforge.net/projects/mcmc-jags/files"

rjags generates this error. It's a sensible one, but an unnecessary step for users who never use JAGS.

I'm sorry about my bad memory, but I recall that CRAN asked us to require installation of rjags because it shows up in the vignette, is that possible? If that's the only reason, can we remove R2jags and rjags examples from the vignette? Or are there other easy fixes?

[andybega]

This stackoverflow question suggests that it might be ok as long as rjags is in the DESCRIPTION Suggests, not Imports??

That should allow the package to load without causing an error if JAGS itself is not installed, but maybe the vignette will need some changes, e.g. some variant of if (!requireNamespace("rjags")) ....

[jkarreth]

Sounds good to me! @ShanaScogin would you agree that moving rjags to Suggests should work? Or was there a CRAN request that I'm forgetting?

[ShanaScogin]

I think it sounds great - I honestly can't remember if there was an error or something thrown up at some point, or if it got moved in some mass shuffle. I'll mess with the description and vignettes following @andybega's suggestions this afternoon.

[ShanaScogin]

Hi both - Looks like we already have rjags in Suggests. Not having it there will probably throw an error, but I can mess around with it/google a bit more

[jkarreth]

You're right @ShanaScogin !

But I just noticed that we do require R2jags, which in turn depends on rjags. Ugh!

Can we do without R2jags? I thought the sole reason for it is mcmcRocPrc, right? @andybega and @jayrobwilliams , another reason to follow your lead and replace it with mcmcRocPrc. However, I don't even think we technically not need R2jags for mcmcRocPrc even if it only takes rjags objects.

The only place where I can find an R2jags:: or rjags:: are the four jags_... functions in the data examples (data_raw). Is it possible to remove these functions? Or does that mess with the self-contained nature of the jags example data? 🤯

[andybega]

I played around with the last commit on the develop branch.

I don't think in principle there should be an issue with moving all packages that require additional software like JAGS to be installed from Imports to Suggests. This is what for example ggmcmc seems to be doing. However, it seems there will have to be some changes in various portions of the package.

For example, I changed DESCRIPTION by moving R2jags and runjags from Imports to Suggests, and I also took out this bit from BayesPostEst.R:

#' @importFrom R2jags jags
NULL

This ends up breaking a lot of the tests and examples, because converting a "rjags" fitted object with as.mcmc() actually works like this: coda::as.mcmc() -> R2jags:::as.mcmc.rjags(). It's even worse because coda::as.mcmc() will return a "mcmc" object, but it's actually not a proper "mcmc" object, so the error is not obvious until further down in the code.

From the top portion of test_mcmcAveProb.r, after the changes I listed above:

data("jags_logit")
fit <- jags_logit

data("sim_data")
datjags <- as.list(sim_data)

### average value approach
xmat <- model.matrix(Y ~ X1 + X2, data = sim_data)
mcmc <- coda::as.mcmc(fit)
mcmc_mat <- as.matrix(mcmc)[, 1:ncol(xmat)]  

# Error in character(ncol(y)) : invalid 'length' argument

The problem is actually in the line above:

mcmc <- coda::as.mcmc(fit)
str(mcmc)
List of 6
 $ model             :List of 8
  ..$ ptr      :function ()  
  ..$ data     :function ()  
  ..$ model    :function ()  
  ..$ state    :function (internal = FALSE)  
  ..$ nchain   :function ()  
  ..$ iter     :function ()  
  ..$ sync     :function ()  
  ..$ recompile:function ()  
  ..- attr(*, "class")= chr "jags"
 $ BUGSoutput        :List of 24
  ..$ n.chains       : int 2
  ..$ n.iter         : num 2000

[blah blah blah]

$ n.iter            : num 2000
 $ DIC               : logi TRUE
 - attr(*, "class")= chr "mcmc"
 - attr(*, "mcpar")= num [1:3] 1 6 1

Which is actually a "rjags" object, with the class attribute changed.

When I add library(R2jags) at the top of the test file, it works correctly. So basically that would have to be done with all tests and examples that use the rjags objects, and probably a couple of the package functions that rely on as.mcmc() will need an explicit (from Hadley's package writing book) check if they are operating on an "rjags" object:

# You need the suggested package for this function    
my_fun <- function(a, b) {
  if (!requireNamespace("pkg", quietly = TRUE)) {
    stop("Package \"pkg\" needed for this function to work. Please install it.",
      call. = FALSE)
  }
}

I did this for example for rstan here, but I guess I'll need to add more as well if R2jags moves to Suggests.

The contents of data-raw I don't think impact anything as they are not included in the built R package (see .Rbuildignore).

FWIW, although it would clutter up the examples a bit more, I think it's worth doing, still.

[andybega]

Ok, never mind. I guess since mcmcReg() & co directly dig into "rjags" object internals or import the method from the namespace for runjags, it was only necessary to add library(R2jags) in 2 files. Seems to be working, I'll make a pull request.

[ShanaScogin]

I approved the pull request and am checking it - I'm getting some errors in devtools::check with the vignette building, which was probably missed because Travis isn't checking the vignette right now.

E> Error: processing vignette 'getting_started.Rmd' failed with diagnostics:
E> there is no package called 'R2jags'

I'm going to focus on fixing the Travis issue first (assigned myself yesterday) and then I'll look at this closer. Thanks for the help, both!

[andybega]

Weird, I also did R check with devtools on my laptop, and it rebuilt the vignette successfully. Do you have R2jags installed?

[ShanaScogin]

Ah I'll check again - I used it last night so I definitely have it now, but I suppose it is actually possible I didn't have it yesterday . I recently updated my R and am trying a new thing were I only reinstall packages I need per a tweet suggestion I saw. I don't know if I'd recommend it.

[ShanaScogin]

You're right! My bad - everything now is passing in develop now. I'm still working on the vignette issue in another branch, but it shouldn't affect anything anyone is doing. Unless @andybega has any more suggestions, do you want to check and close this @jkarreth?

[jkarreth]

Sounds good! I'll try to install the develop version on a third party computer first (with fresh install of R), then will close this issue. Thanks so much to both of you for working on this.

[jkarreth]

Yay - just tried this on a computer that had R but not JAGS installed. The develop branch installed fine; it just just returned the warnings below (which, I think, are sensible, so we can leave as is):

Warning: namespace ‘rjags’ is not available and has been replaced
by .GlobalEnv when processing object ‘jags_interactive’
Warning: namespace ‘rjags’ is not available and has been replaced
by .GlobalEnv when processing object ‘jags_interactive’
Warning: namespace ‘rjags’ is not available and has been replaced
by .GlobalEnv when processing object ‘jags_interactive_cat’
Warning: namespace ‘rjags’ is not available and has been replaced
by .GlobalEnv when processing object ‘jags_interactive_cat’
Warning: namespace ‘rjags’ is not available and has been replaced
by .GlobalEnv when processing object ‘jags_logit’
Warning: namespace ‘rjags’ is not available and has been replaced
by .GlobalEnv when processing object ‘jags_logit’
Warning: namespace ‘rjags’ is not available and has been replaced
by .GlobalEnv when processing object ‘jags_probit’
Warning: namespace ‘rjags’ is not available and has been replaced
by .GlobalEnv when processing object ‘jags_probit’
Warning: namespace ‘rjags’ is not available and has been replaced
by .GlobalEnv when processing object ‘runjags_interactive’
Warning: namespace ‘rjags’ is not available and has been replaced
by .GlobalEnv when processing object ‘runjags_interactive’

So I think this can be closed once we've moved develop to master and then to CRAN. Thank you again!

[andybega]

I have two ideas/suggestions:

What about adding a bit of code to the vignette that checks for the presence of the packages listed in Suggests, but not Imports, and which are needed to knit the vignette? Basically something that would prompt a user to install them.

The second one I’m not really sure about: does it make sense to add some kind of similar check for the presence of JAGS/rjags when the package is loaded/attached to warm a user of what bits will not work? The warnings that @jkarreth posted above, and will they be able to run the examples that rely on jags_logit, etc.?

[jkarreth]

I like these ideas;

What about adding a bit of code to the vignette that checks for the presence of the packages listed in Suggests, but not Imports, and which are needed to knit the vignette? Basically something that would prompt a user to install them.

so something like, To reproduce examples using JAGS as seen in the vignette, please install JAGS from ... ? That could work.

Or we just change all the examples to MCMCpack, which I believe would be the least-complex package to allow for estimating models for examples.

The second one I’m not really sure about: does it make sense to add some kind of similar check for the presence of JAGS/rjags when the package is loaded/attached to warm a user of what bits will not work? The warnings that @jkarreth posted above, and will they be able to run the examples that rely on jags_logit, etc.?

I think that's useful for the parts that do require JAGS/rjags - but I think that's only the examples now and eventually (once #5 is complete) there won't be another part that requires JAGS/rjags... right?

Maybe I'll turn this into a new issue about handling connections to the packages that actually generate the MCMC output - what do you all think?

[andybega]

Yeah, I think this is already enough to spawn a couple of new issues.

I think that's useful for the parts that do require JAGS/rjags - but I think that's only the examples now and eventually (once #5 is complete) there won't be another part that requires JAGS/rjags... right?

Yep, just the examples and vignette.

Or we just change all the examples to MCMCpack, which I believe would be the least-complex package to allow for estimating models for examples.

Instead of a specific modeling/sampling "engine" like MCMCpack or JAGS, what about just straight up "mcmc" or "mcmc.list" objects? I mean the functionality that this package adds is for post processing of results, so how the posterior samples were obtained (/modeling) doesn't really matter. coda is in the Imports already, while MCMCpack is in Suggests.

[jkarreth]

what about just straight up "mcmc" or "mcmc.list" objects?

That's a good point, and should work! As long as the mcmc object is set up similar to output from the Bayesian estimation packages we support (that is, the first set of columns are parameters such as b[1], b[2], etc.), this should indeed work.

If anyone wants to take a stab at this, please do. Otherwise I'll get at this ASAP. For my own sake, here is what I think needs to be changed in order to achieve this:

• [ ] the example in each (!) function needs to be updated to use an mcmc.list instead of walking through the process of estimating a model with JAGS

• [x] the files in /data-raw/ can go (?)

• [ ] the vignette needs to be updated to contain mcmc.lists and process these, rather than estimate model from scratch (and describe what's going on)

• [x] /R/data.R can remain as is, @jayrobwilliams ?

[andybega]

the files in /data-raw/ can go (?)

I'm using a couple for testing. If nothing else, they can be moved to tests/testdata / tests/testdata-raw, with some corresponding adjustments in tests that currently use data(...).

[ShanaScogin]

A lot going on in this issue. Here's what I've done so far:

Re: data: I moved the files in /data to /testdata and changed them to .rds files for testing in commit f8f8e23. I re-added several tests as I was checking tests, which means the data files are large. I think we should document any more movement about data in issue #79 opened by Andy.

Re:rjags: In commit ba61eb6, I added R2jags and rjags back into depends due to an error that occurs without it (See #88 where I hit the errors again as I was trying to figure out what we were doing two years ago) until we do the mcmc.list(). I think it is possible some of the innards of the functions use this, so need to find that as well.