'Pedantic Mode' is ready for your testing and feedback

[rybern]

Thanks for you comments Bob!

@Bob_Carpenter

Not used in the model → Not used in the program

We want to be careful to talk about Stan programs, each of which implements a model. There’s more than one program for any model.

I see - how about ‘Not used in the density calculation’? It’s checking specifically whether its value affects target.

@Bob_Carpenter

“columns a–b” wold be easier to read than “column a to column b.”

Yeah. The location printing is actually some older code that could use some love.

@Bob_Carpenter

“alpha has 0 priors” → “alpha does not have a prior”

How likely are false positives on this if we transform alpha and give it a prior?

Updated the wording on the last version.
That should not give a false positive, it can find that a prior was given via a transformation.

@Bob_Carpenter

Argument 0.001 suggess there may be parameters that are not unit scale;
consider rescaling with a multiplier

That’s fair. I’d be in favor of leaving the manual section in though, since it’s not an obvious thing (to me).

@Bob_Carpenter

The correct term is “defined” (as opposed to “declared”). This may confuse users, though, who are going to think declaring a variable defines it.

Agreed. Maybe ‘assigned’?

@Bob_Carpenter

Also, is there a reason this says “may”? Is it trying to cover for conditionals not being executed?

Exactly, the analysis is conservative.

@Bob_Carpenter

I don’t think “hard constraint” is informative enough. I also don’t see where an infinite gradient comes from—we either log or inverse-logit transform.

Yeah, I agree it makes sense not to introduce the new term hard constraint, lower and upper would be probably be better. (By the way, a lot of the language is copied from this page: Home · stan-dev/stan Wiki · GitHub)

@Bob_Carpenter

How are false positives handled here? For example, correlation parameters are usually constrained in (-1, 1) and many parameters will be constrained with lower bounds of 0.

(-1, 1) and (0, 1) are ignored, as are one sided bound

@Bob_Carpenter

It we’re talking about things like spacing, indentation, etc., maybe we could write a pretty printer. That’s been super useful in our C++ development.

I’m remembering now that we have one in stanc3, it’s the --auto-format CLI option

[rybern]

I’m not clear yet on what an exact rule would look like, and which distributions to apply it to. Happy to add it if we can work that out.

[mitzimorris]

@rybern

I’m remembering now that we have one in stanc3, it’s the --auto-format CLI option

FWIW, Stanc3 command line options are documented in the CmdStan manual, section 4.3 - https://github.com/stan-dev/cmdstan/releases/download/v2.23.0/cmdstan-guide-2.23.pdf

[bbbales2]

@rybern

I’m not clear yet on what an exact rule would look like, and which distributions to apply it to.

It’s one thing on the left when there are multiple things on the right, if that makes sense.

I think all the distributions probably suffer from this.

[rybern]

So for each distribution, use the basest type signature to determine which arguments are broadcast, and warn if some argument is broadcast but the first argument isn’t?
I’m hesitant because I’m not sure if checking for broadcast arguments will generalize perfectly, and because there seem to be good uses for it. Honestly, I think that whenever implicit broadcasting is allowed there will always be subtle bugs like this.
But, if this rule would catch bugs that you see regularly with a low false-positive, I could add it in.

[bbbales2]

@rybern

I’m hesitant because I’m not sure if checking for broadcast arguments will generalize perfectly

Fair.

@rybern

and because there seem to be good uses for it

At least for this case, I don’t know if there are legitimate uses.

@rybern

But, if this rule would catch bugs that you see regularly with a low false-positive, I could add it in.

Eh it happened to me, so I’m just assuming it happens to people pretty regularly.

If this check only covered the scalar distributions it would be fine. “If left hand thing is scalar, and at least one right hand thing isn’t a scalar, spit a warning”.

[breckbaldwin]

Weird bug. I copied and pasted from p 294 (https://mc-stan.org/docs/2_23/stan-users-guide-2_23.pdf) the below code:

  int<lower=0> N;
  vector[N] y;
  vector[N] x;
}
transformed data {
  vector[N] x_std;
  vector[N] y_std;
  x_std = (x - mean(x)) / sd(x);
  y_std = (y - mean(y)) / sd(y);
}
parameters {
  real alpha_std;
  real beta_std;
  real<lower=0> sigma_std;
}
model {
  alpha_std ~ normal(0, 10);
  beta_std ~ normal(0, 10);
  sigma_std ~ cauchy(0, 5);
  for (n in 1:N)
    y_std[n] ~ normal(alpha_std + beta_std * x_std[n],
    sigma_std);
}

Running your latest release in Rstudio per the command:

install_cmdstan(cores=4, overwrite=TRUE, repo_clone=TRUE, repo_branch="pedantic_mode")

I get:

bin/stanc  --warn-pedantic --o=foo.hpp foo.stan
Warning:
  The parameter alpha_std has 2 priors.
Warning:
  The parameter beta_std has 2 priors.
Warning:
  The parameter sigma_std has 2 priors.

Removing the transformed data {} block and renaming x_std -> x and y_std -> y fixes the problem as below:

data {
  int<lower=0> N;
  vector[N] y;
  vector[N] x;
}
parameters {
  real alpha_std;
  real beta_std;
  real<lower=0> sigma_std;
}
model {
  alpha_std ~ normal(0, 10);
  beta_std ~ normal(0, 10);
  sigma_std ~ cauchy(0, 5);
  for (n in 1:N)
    y[n] ~ normal(alpha_std + beta_std * x[n], sigma_std);
}

Mac OS 10.13.6

Breck

[rybern]

Thanks Breck, that was indeed a bug. I’ve pushed a fixed version.

[rybern]

Thanks everyone for your helpful feedback! I believe I’ve addressed all of the raised issues.

Pedantic mode has been available for comment in CmdStan for about two months and in the Stanc3 pull request for about four months. Does anyone have objections to merging this, flagged as ‘experimental’?

Here is a draft of the documentation. Happy to hear comments on that as well. @mitzimorris, I think we had a discussion a while ago about where the documentation should end up, but I can’t quite remember the conclusion. Let me know what you think.

[mitzimorris]

@rybern

@mitzimorris, I think we had a discussion a while ago about where the documentation should end up, but I can’t quite remember the conclusion. Let me know what you think.

I can’t remember the conclusion either. I think we should aim for a chapter in the Stan User’s Guide in the “Programming Techniques” section. all we need to do is advertise the Stan release where it has been made available. that will explain what Pedantic mode is. for the interfaces, the question is how to invoke it.

if docs in the User’s Guide sounds good to everyone here, I’ll file issues on the docs and interface repos.