stan-dev / stanc3

The Stan transpiler (from Stan to C++ and beyond).
BSD 3-Clause "New" or "Revised" License
143 stars 47 forks source link

Update Stan manual for stanc3 with changes #302

Closed seantalts closed 4 years ago

seantalts commented 5 years ago

Changes are listed here: https://github.com/stan-dev/stanc3/wiki/changes-from-stanc2

seantalts commented 4 years ago

@VMatthijs would you mind tackling this one? I think you're much more familiar with the specific changes and the manual since you did all of the implementation :)

seantalts commented 4 years ago

Hey @VMatthijs, happy new year! Do you think you might be able to get to this in the next week or two? We'll need this in before we can release the compiler and the next release is Jan 18.

VMatthijs commented 4 years ago

Should this go in the stan-dev/docs repo? You probably know as much as I do, as I'd just be working from the list under the wiki. I will try to find time, but can't promise anything I'm afraid as things are a bit hectic.

seantalts commented 4 years ago

Yep, the Stan manual lives in the docs repo now. I think that wiki is all stuff that you wrote based on discrepancies or unspecified behavior you had previously discovered between the old compiler and the manual, so I think you'd have a more efficient time of it if you can spare time! If not I can use the excuse to learn more about that side of things.

VMatthijs commented 4 years ago

I just went through the wiki and I think the following are the only things that need changing in the manual (as the rest were just bug fixes):

On a related note, I expect rstanarm and perhaps brms to break as they were exploiting some bugs/undesired behaviour in stanc2. In particular, I know that rstanarm was producing some models missing semi-colons in places where they should be required and it was using lower and upper etc as variable names.

seantalts commented 4 years ago

Oh awesome, thank you! Tagging @bgoodri and @paul-buerkner

VMatthijs commented 4 years ago

Ah, actually, perhaps one more thing needs changing, though I am not actually 100% sure this behaviour is different from stanc2. Perhaps, @bgoodri can weigh in on this:

Based on the manual it seemed like stanc2 would not allow recursive includes. However, I almost find that hard to believe.

bgoodri commented 4 years ago

Yeah, rstan had recursive includes before includes were introduced into the Stan language, but I think Stan actually does allow them. Anyway, I don't think it matters for rstan going forward because the Javascript version of the parser cannot access the filesystem, so we have to use the C preprocessor to handle the includes which gives us recursion automatically. The only thing that I think needs to be added to the language is to permit quotation marks in file paths like

include "foo.stan"

rather than what we have been doing which is requiring

include foo.stan

Actually, I doubt the old parser works when the path contains spaces, as in

include C:/My Documents/foo.stan

On Sun, Jan 5, 2020 at 12:47 PM Matthijs Vákár notifications@github.com wrote:

Ah, actually, perhaps one more thing needs changing, though I am not actually 100% sure this behaviour is different from stanc2. Perhaps, @bgoodri https://github.com/bgoodri can weigh in on this:

  • stanc3 allows recursive includes: that is a.stan includes b.stan includes c.stan; the only thing it disallows are circular includes: that is a.stan includes b.stan includes c.stan includes a.stan;

Based on the manual it seemed like stanc2 would not allow recursive includes. However, I almost find that hard to believe.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/stan-dev/stanc3/issues/302?email_source=notifications&email_token=AAZ2XKQW3UTRNX4K46CRTSLQ4IMJXA5CNFSM4IZMVQ3KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEID32SA#issuecomment-570932552, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAZ2XKXO2BQ2MSLL7PXFII3Q4IMJXANCNFSM4IZMVQ3A .

bob-carpenter commented 4 years ago

On Jan 5, 2020, at 12:40 PM, Matthijs Vákár notifications@github.com wrote:

I just went through the wiki and I think the following are the only things that need changing in the manual (as the rest were just bug fixes):

• variable declarations and statements can now be mixed in an arbitrary order (before variable declarations had to precede statements);

That should be OK as no old behavior will change because it would've been illegal before.

• target, lower, upper, offset, multiplier are now reserved keywords and no longer be used as variable names;

That's also OK.

• there now is a pretty printing / formatting mode in stanc (but I'm not even sure if it should go in a doc and if so which one); there are also a bunch of other new flags to stanc for stuff like debugging;

stanc itself isn't really user-facing, but it might make sense to write something like a man page for the interface devs so they know how to call it.

On a related note, I expect rstanarm and perhaps brms to break as they were exploiting some bugs/undesired behaviour in stanc2. In particular, I know that rstanarm was producing some models missing semi-colons in places where they should be required and it was using lower and upper etc as variable names.

Do the rstanarm and brms folks know what they need to fix? If not, would you mind pinging them and letting them know. Thanks.

bob-carpenter commented 4 years ago

Why do I keep missing replies---they show up after I send out messages! I really didn't see you'd already tagged Ben and Paul.

On Jan 5, 2020, at 12:44 PM, seantalts notifications@github.com wrote:

Oh awesome, thank you! Tagging @bgoodri and @paul-buerkner

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

bob-carpenter commented 4 years ago

The original stanc allows includes to have includes. That's where all the complicated logic for error messages came from and that static stack of include orders that was previously generated.

On Jan 5, 2020, at 12:47 PM, Matthijs Vákár notifications@github.com wrote:

Ah, actually, perhaps one more thing needs changing, though I am not actually 100% sure this behaviour is different from stanc2. Perhaps, @bgoodri can weigh in on this:

• stanc3 allows recursive includes: that is a.stan includes b.stan includes c.stan; the only thing it disallows are circular includes: that is a.stan includes b.stan includes c.stan includes a.stan; Based on the manual it seemed like stanc2 would not allow recursive includes. However, I almost find that hard to believe.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

bob-carpenter commented 4 years ago

On Jan 5, 2020, at 1:17 PM, bgoodri notifications@github.com wrote:

Yeah, rstan had recursive includes before includes were introduced into the Stan language, but I think Stan actually does allow them. Anyway, I don't think it matters for rstan going forward because the Javascript version of the parser cannot access the filesystem, so we have to use the C preprocessor to handle the includes which gives us recursion automatically.

What happens to error messages under that scheme?

The only thing that I think needs to be added to the language is to permit quotation marks in file paths like

include "foo.stan"

rather than what we have been doing which is requiring

include foo.stan

Actually, I doubt the old parser works when the path contains spaces, as in

include C:/My Documents/foo.stan

I think it just takes the entire line and trims it. But what the old parser does is moot at this point.

bgoodri commented 4 years ago

What happens to error messages under that scheme?

Assuming the #include statement is valid (hence why we need to permit quotation marks), then the C preprocessor will make a monolithic Stan program after reconciling the included files. So, the line numbers and whatnot of syntax errors would be relative to the monolithic file. I'll see what I can do with it under the RStudio API.