provide implementation of 'eng_stan' for knitr

[kevinushey]

The intention is to move the implementation of eng_stan() to the rstan package, so that the engine implementation can evolve as the rstan package evolves.

As an example, we do this with the reticulate package here:

https://github.com/rstudio/reticulate/blob/master/R/knitr-engine.R

See also: yihui/knitr#1602.

[sakrejda]

The flow-of-control by itself in this stuff makes me want to run away screaming. I mean I like that it's possible but sometimes you just want to close that sarcophagus right back up.

[bob-carpenter]

@sakrejda --- please keep the discussions constructive on our users' lists.

[bob-carpenter]

@kevinushey What is eng_stan()? Could you be more specific about what you're proposing? We want to avoid having people open issues then spend a lot of time on them if we're not going to accept them in the end. The best defense against that is to be specific about what you're proposing to add and get buy-in from the RStan devs up front. Thanks.

[sakrejda]

@bob-carpenter knitr allows inline Stan code, eng_stan() is part of processing that, that should explain why my comment, while slightly off topic, was constructive. This would be fun to support but yikes.

[bob-carpenter]

I don't like that they auto compile Stan code. I prefer to manage all that myself in my knitr docs, but would also like Stan code highlighting.

I think you may need to translate that comment for the rest of us if it was meant to be constructive rather than dismissive.

[bgoodri]

We talked about this when JJ was here. People have been able to put Stan code in chunks of a RMarkdown document for several years. *We* said to JJ that the behavior was not ideal because if you write output.var="foo", then it calls `stan_model` and assigns the output to the R symbol `foo`, but it is not evident from the knitted document where `foo` was defined. If that behavior can be improved, what is the big deal about putting it into the rstan package?

…

On Mon, Sep 3, 2018 at 5:32 AM Bob Carpenter ***@***.***> wrote: I don't like that they auto compile Stan code. I prefer to manage all that myself in my knitr docs, but would also like Stan code highlighting. I think you may need to translate that comment for the rest of us if it was meant to be constructive rather than dismissive. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#555 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADOrqjTlcDgP3EqCZGwit28TP31BtfXTks5uXPcVgaJpZM4WVg26> .

[bob-carpenter]

I remember the discussion. I just don't know what eng_stan() does. In general (as in not considering any specifics of this case), my preference would be to put package-specific management tools in the relevant downstream package, not in the core of RStan. Things that go in the core of RStan get dependencies from other packages and slow down our release cycle as well as increasing our maintenance and testing burden. Maybe that's desirable for dependencies from RStudio, which is why I thought the RStan devs needed to decide the specifics.

…

On Sep 3, 2018, at 12:32 PM, bgoodri ***@***.***> wrote: We talked about this when JJ was here. People have been able to put Stan code in chunks of a RMarkdown document for several years. *We* said to JJ that the behavior was not ideal because if you write output.var="foo", then it calls `stan_model` and assigns the output to the R symbol `foo`, but it is not evident from the knitted document where `foo` was defined. If that behavior can be improved, what is the big deal about putting it into the rstan package? On Mon, Sep 3, 2018 at 5:32 AM Bob Carpenter ***@***.***> wrote: > I don't like that they auto compile Stan code. I prefer to manage all that > myself in my knitr docs, but would also like Stan code highlighting. > > I think you may need to translate that comment for the rest of us if it > was meant to be constructive rather than dismissive. > > — > You are receiving this because you are subscribed to this thread. > Reply to this email directly, view it on GitHub > <#555 (comment)>, or mute > the thread > <https://github.com/notifications/unsubscribe-auth/ADOrqjTlcDgP3EqCZGwit28TP31BtfXTks5uXPcVgaJpZM4WVg26> > . > — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[sakrejda]

Re: translating: I think @kevinushey was suggesting that we should take control (and therefore reponsibility) for the rstan code. I think it would be cool but daunting, that's all.

Overall I don't think the knitr-compatible engine for processing Stan code chunks (eng_stain()) is an rstan-specific thing (unless we want rstan to become the dumping ground for all things Stan-related in R). With maintenance burden and all I don't think we want that. It probably would make sense as a separate small package.

[bgoodri]

The Stan engine is what implements what RMarkdown does with a chunk that contains Stan code. The main reason to have it in the rstan package is so that it can call internal rstan functions, which is also what happens when someone clicks on the Check icon in the RStudio editor when the file has a .stan extension. But it does not make much sense to create an additional R package that has no exposed functions but users are required to install in order to knit their documents. On Mon, Sep 3, 2018 at 6:43 AM Bob Carpenter <[email protected]> wrote:

…

I remember the discussion. I just don't know what eng_stan() does. In general (as in not considering any specifics of this case), my preference would be to put package-specific management tools in the relevant downstream package, not in the core of RStan. Things that go in the core of RStan get dependencies from other packages and slow down our release cycle as well as increasing our maintenance and testing burden. Maybe that's desirable for dependencies from RStudio, which is why I thought the RStan devs needed to decide the specifics. > On Sep 3, 2018, at 12:32 PM, bgoodri ***@***.***> wrote: > > We talked about this when JJ was here. People have been able to put Stan > code in chunks of a RMarkdown document for several years. *We* said to JJ > that the behavior was not ideal because if you write output.var="foo", then > it calls `stan_model` and assigns the output to the R symbol `foo`, but it > is not evident from the knitted document where `foo` was defined. If that > behavior can be improved, what is the big deal about putting it into the > rstan package? > > On Mon, Sep 3, 2018 at 5:32 AM Bob Carpenter ***@***.***> > wrote: > > > I don't like that they auto compile Stan code. I prefer to manage all that > > myself in my knitr docs, but would also like Stan code highlighting. > > > > I think you may need to translate that comment for the rest of us if it > > was meant to be constructive rather than dismissive. > > > > — > > You are receiving this because you are subscribed to this thread. > > Reply to this email directly, view it on GitHub > > <#555 (comment)>, or mute > > the thread > > < https://github.com/notifications/unsubscribe-auth/ADOrqjTlcDgP3EqCZGwit28TP31BtfXTks5uXPcVgaJpZM4WVg26 > > > . > > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub, or mute the thread. > — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#555 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADOrqkNjH6r7c-Id1xRArKZXmLlWTNwDks5uXQfagaJpZM4WVg26> .

[bob-carpenter]

I didn't mean a third package, I meant putting the code in RStudio, which sounds like the only client that will ever use this. If RStudio's packages won't be able to see RStan's internals, then you'll have to wrap it for them or expose a route to the necessary internals.

[bgoodri]

People can use the knitr and RMarkdown R packages outside of the RStudio client.

…

On Mon, Sep 3, 2018 at 7:06 AM Bob Carpenter ***@***.***> wrote: I didn't mean a third package, I meant putting the code in RStudio, which sounds like the only client that will ever use this. If RStudio's packages won't be able to see RStan's internals, then you'll have to wrap it for them or expose a route to the necessary internals. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#555 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADOrqsCoSg72Id0sTEb00E6rZ6MkesQ9ks5uXQ1CgaJpZM4WVg26> .

[sakrejda]

I definitely don't use RStudio but do use RMarkdown, they're unrelated, a separate package is the place to implement something like this. The key question is what @bgoodri raised: how do you really want this to look in RMarkdown output. This is a pretty general problem in knitr/RMarkdown where it's not obvious from the UI when output is available across chunks (esp. when you get into multiple languages).

[kevinushey]

Just to be clear what I'm proposing: knitr already defines an chunk engine for Stan code, called eng_stan(). That implementation currently lives here:

https://github.com/yihui/knitr/blob/4cf7a6f1ef3e083464823a15921385a452177412/R/engine.R#L248-L273

Having the engine implementation live in knitr means it's somewhat more difficult for that engine to evolve, as changes must then be accepted as a PR to knitr, and access to the new engine will only occur with the next CRAN update of knitr. My proposal is to instead move that definition back to the rstan package, and teach knitr how to call + use that engine instead (so that eng_stan() could evolve as part of the rstan package as needed).

We could of course do some extra magic in RStudio to tweak how Stan chunks are run / displayed, but I imagine that in general it would be best if Stan engine output in R Markdown documents was universal and did not depend on the environment from which it was run.

In the context of what might change in the engine, it seems like a change as simple as having eng_stan() emit a message after successful compilation of the form:

[Stan model successfully compiled and assigned to variable 'model']

and I assume a message like that would almost always be appropriate, but could potentially be controlled with a chunk option if so desired.

That said, if this is output that we'd only ever want to see when compiling a Stan chunk interactively then we can just make the requisite changes in RStudio.