stan-dev / rstan

RStan, the R interface to Stan
https://mc-stan.org
1.04k stars 269 forks source link

Deprecate plotting functions in RStan or just replace backends? #517

Open jgabry opened 6 years ago

jgabry commented 6 years ago

Since we have bayesplot now there's no reason to also maintain all the old ggplot2 code currently in RStan. I guess we can

or we can

Personally I'm in favor of (A) since it simplifies RStan's code base (we'd keep the deprecated functions for quite some time of course, but wouldn't plan to maintain that code in the long term). I'd like to do one or the other soon. What do people think of (A), (B), or some other option I haven't thought of?

sakrejda commented 6 years ago

Deprecate! We can always backtrack on that if someone decides to go to town and update rstan to use bayesplot.

jgabry commented 6 years ago

The only reason I can currently think of for not deprecating is that one feature in particular, unconstraining parameters, needs to be done with a stanfit object rather than the inputs bayesplot accepts. I suppose getting around that would involve providing a user friendly way to get a matrix/array of posterior draws in the unconstrained space to pass to bayesplot? It’s super annoying to do that currently.

sakrejda commented 6 years ago

@jgabry Is the current method documented somewhere (sorry, I mostly use CmdStan...)

avehtari commented 6 years ago

I suppose getting around that would involve providing a user friendly way to get a matrix/array of posterior draws in the unconstrained space

That would be a helpful function anyway.

jgabry commented 6 years ago

@sakrejda unconstrain_pars()is the relevant RStan function.

http://mc-stan.org/rstan/reference/stanfit-method-logprob.html

But it’s annoying to use.

Sent with GitHawk

jgabry commented 6 years ago

Here are the two main problems with unconstrain_pars() in my opinion: (1) the user interface is confusing and cumbersome. (2) can’t be used after reloading a model fit in a previous R session. We can fix (1) more easily than (2) but if RStan knew of and could perform the transformations used by Stan then we could fix (2) also. That should be possible but I don’t think we can do it easily in rstan without changes to Stan itself first.

Sent with GitHawk

jgabry commented 6 years ago

@paul-buerkner I think in addition to bayesplot brms used to use some of the RStan plotting functions. Is that still the case? If so would you be ok with deprecating those in brms too if we go the route of deprecating the plotting in RStan?

Sent with GitHawk

sakrejda commented 6 years ago

@jgabry riiiiiight.. this can't really be pulled apart until a) rstan tags parameter names with transforms; and b) transforms become available, ideally by being pulled out from the var_context stuff in Stan itself. I think this is doable over a few months (so like end of summer :) )

jgabry commented 6 years ago

@sakrejda yeah that’s exactly what we need!

jgabry commented 5 years ago

I will work on this soon. Except for the var_contextstuff that @sarkeda mentioned, which would be great but not my area of expertise.

jgabry commented 5 years ago

I’m thinking deprecate with a message pointing to the analogous bayesplot function to the one the user called in rstan. Perhaps I can even get the message to be the exact call they would need for bayesplot.

jgabry commented 5 years ago

Any opposition to that?