Me, @paul.buerkner and @avehtari have been discussing the dependency structures in the r+stan ecosystem and put together a short document on some decisions we think would be good to make the dependency structure between packages more clear.
I could not find anything written in the github wiki so I suggested adding some design decision to make it easier to contribute to the Stan + R ecosystem. The two suggested decisions are:
Use a layered dependency structure where lower level functionality (such as loo) don’t depend on higher level dependencies, such as, rstan, brms or rstanarm. See document for example. This simplifies were functionality should be put.
As a consequence of 1. we would need to have a decision that says that generic function methods for a specific object (say loo.brms()) should be defined in the same package as the constructor.
Here is the document I put together to try to explain this a little more in detail.
Thanks @mans_magnusson! I think I agree with everything in the document.
For the most part I think we’re already doing most of this right (or at least that was the intention), but there are definitely some cases (including a few you pointed to in the document) that we need to change in order to fully comply with this.
Just let me know if I can do something to help on this. The best would be that the Stan Dev team just make a decision and write it down in the Wiki. Then I know the principles to follow in API implementations. To do the changes and move functionality is not super-urgent.
Ideally shouldn’t rstan not depend on bayesplot? Or perhaps bayesplot depends on rstan but weakly only through formats motivated by what’s easy to access from rstan fit objects? That would provide another big dependence break.
It depends what we mean by depends. bayesplot doesn’t require rstan, but it does provide some optional features that call a few functions from rstan. For example bayesplot::nuts_params calls rstan::get_sampler_params() and does some manipulations to convert the list to a data frame usable with bayesplot. So, yes, to fit fully within the proposed scheme we should have rstan make that data frame for the user and have them pass it to bayesplot, instead of bayesplot calling anything from rstan directly. I think we can maintain backwards compatibility as we make the transition (e.g. deprecate but allow both). I’ll try to work out in more detail what that would look like.
I do not know exactly all details in bayesplot. I think it depends on how we regard bayesplot. If we regard bayesplot as a generic package for plotting “bayes” stuff, and plotting rstan posteriors is one instance of the generic posterior plotting, then baysplot should not depend on rstan imho. If I would like to use JAGS or Tensorflow with bayesplot, I would not like to have a rstan dependency. rstan should then be one instance of a generic baysplot and rstan specific handling (such as bayesplot::nuts_params) should be implemented in rstan, where the “constructor” exists. This is how I assumed the intention of bayesplot.
Although, if baysplot is rather a “rstanplot” package, i.e. bayesplot only have functionality for plotting rstan objects. Then I think it should only depend on rstan, and rstan should be able to ignore bayesplot all together (i.e. bayesplot should be at the level of, or just, below rstanarm and brms).
Since I do not know bayesplot, it is maybe both, and we dont want to litter rstan with plotting functionality. Then I think it would be good, in the long run, to refactor the package to two packages, one with generic bayesplotting functionality (that could be use with any framework) and one package specific for plotting rstan objects.
The important think for me when developing is to know what to do to not break things. Ideally, if I develop in loo, and as long as I do not change the loo API nothing should break higher up in the dependency levels. Also, I should not have to care about any higher order dependency or package. This also good since I probably dont know these internals very well. If I work loo, I should not (in loo) need to care about the brms implementation of loo, that is something that should be done in brms.
Im not sure if this made it more or less clear. This is of course not in a hurry, but getting a decision in place will simplify implementations for me.
More specifically, I’d like to see RStan stripped down to just the fit routines so that it doesn’t depend on any posterior analysis or plotting packages. Then I’d like to see the posterior analysis wrapped up into a generic Coda-like package that doesn’t depend on RStan. I don’t have a strong opinion on whether we need a coda-like package without graphics—if it does use graphics, it’d depend on Bayesplot.
Yeah this was always my intent and is already possible and being used that way. Currently having an RStan installation is not required to use bayesplot. There’s already at least one CRAN package wrapping JAGS that uses bayesplot for plotting and doesn’t depend on RStan.
Yeah that package is on the to-do list. Although if we want to be able to call the C++ implementations from R to calculate the diagnostics then this separate R package will at least have to depend on StanHeaders or copy the C++ code. But it definitely won’t have to depend on the RStan interface.
Right. That’s what we talked about during the roadmap meeting in the morning. It will also need Eigen—I don’t know if it depends on Boost. But it might be worth pulling those out into a simpler posterior analysis code base that just has R-hat and ESS and mean/variance/quantile calculations.