stan-dev / stanc3

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

"context" not listed as a reserved name, but should probably be #1216

Closed Alex-Cremers closed 2 years ago

Alex-Cremers commented 2 years ago

Summary:

Naming a variable "context" in the data block leads to obscure errors when compiling, so "context" should probably listed as a reserved name.

Description:

Naming a variable "context" in the data block leads to obscure errors when compiling. Naming a variable "context" in the parameters block doesn't cause any issue however. However, "context" is not listed as a reserved name in the manual (see here), and the errors returned by rstan and cmdstanr are not particularly helpful.

Reproducible Steps:

Model doesn't compile:

data {
  int<lower=0> N;
  vector[N] context;
}

parameters {
  real mu;
  real<lower=0> sigma;
}

model {
  context ~ normal(mu,sigma);
}

Model compiles without any problem:

data {
  int<lower=0> N;
  vector[N] y;
}

parameters {
  real context;
  real<lower=0> sigma;
}

model {
  y ~ normal(context,sigma);
}

Current Output:

I tried both rstan (v2.29.0.900) and cmdstanr (with cmdstan v2.29.2), both return different errors, neither of which is particularly helpful (although cmdstan at least mentioned context__ and var_context, which helped me figure out what was wrong).

rok-cesnovar commented 2 years ago

Thanks @Alex-Cremers. I can confirm this is a bug. Moving this issue to stanc3, where it will be addressed.

WardBrian commented 2 years ago

I believe we can fix this by adding it to the backend specific list here, so it will be mangled

https://github.com/stan-dev/stanc3/blob/79cdcd9bcc1d4d0cb5406185c6cccdb13ced51ce/src/stan_math_backend/Mangle.ml#L30

nhuurre commented 2 years ago

The documentation page is outdated--the "backend language" keywords are no longer reserved. EDIT: Looking again, the page says "It is legal to name a variable any of the following names" so I guess it's not outdated, just irrelevant.

context should not be reserved at all. This is a mistake in the codegen backend resulting in ambiguous naming. I'm working on a fix right now.