Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

updates to ARM model for normal_id_glm - variable name "cov" should be "x_as_mat" or something like that #216

Open
mitzimorris opened this issue Jul 30, 2022 · 6 comments
Labels

Comments

@mitzimorris
Copy link
Member

mitzimorris commented Jul 30, 2022

the ARM models which have been updated for normal_id_glm have introduced a transformed data variable named "cov" - not sure what this name is supposed to be, but it's definitely not a covariance matrix. perhaps @andrjohns can explain?

would it be OK to change to "x_mat" or something like that?

./ARM/Ch.10/ideo_interactions.stan
15-}
16-model {
17:  score1 ~ normal_id_glm(cov, alpha, beta, sigma);
18-}

./ARM/Ch.10/ideo_two_pred.stan
14-}
15-model {
16:  score1 ~ normal_id_glm(cov, alpha, beta, sigma);
17-}

./ARM/Ch.12/radon_no_pool.stan
23-  
24-  a ~ normal(mu_a, sigma_a);
25:  y ~ normal_id_glm(cov, a[county], beta, sigma_y);
26-}

./ARM/Ch.12/radon_complete_pool.stan
14-model {
15-  sigma ~ cauchy(0, 2.5);
16:  y ~ normal_id_glm(cov, alpha, beta, sigma);
17-}

./ARM/Ch.12/radon_no_pool_chr.stan
23-  sigma_y ~ cauchy(0, 2.5);
24-  
25:  y ~ normal_id_glm(cov, alpha[county], beta, sigma_y);
26-}

./ARM/Ch.12/radon_group.stan
28-  sigma_beta ~ cauchy(0, 2.5);
29-  
30:  y ~ normal_id_glm(cov, alpha[county], beta, sigma);
31-}

./ARM/Ch.12/radon_group_chr.stan
24-  alpha ~ normal(mu_a, sigma_a);
25-  
26:  y ~ normal_id_glm(cov, alpha[county], beta, sigma);
27-}

./jupyter/radon/stan/radon_complete_pool.stan
14-model {
15-  sigma ~ cauchy(0, 2.5);
16:  y ~ normal_id_glm(cov, alpha, beta, sigma);
17-}
@mitzimorris
Copy link
Member Author

mitzimorris commented Jul 30, 2022

apologies for blaming the wrong person - @WardBrian - why the name "cov"?

possible names: design_matrix, predictors, xs? xs seems best.

@mitzimorris
Copy link
Member Author

mitzimorris commented Jul 30, 2022

why this:

data {
  int<lower=1> N;
  vector[N] x;
  vector[N] y;
}
transformed data {
  matrix[N, 1] xs = [x']';
}

instead of:

matrix[N, 1] xs = to_matrix(x);

is to_matrix less efficient? is it so inefficient that we don't want it in the example models?

@WardBrian
Copy link
Member

I didn’t rename them, my name might show up in the blame because I ran the canonicalizer on the models a few months ago

As for why the coding was done that way, I don’t believe that will be more efficient than to_matrix. The implementation of to_matrix for vector types is just a forwarding constructor

@mitzimorris
Copy link
Member Author

@WardBrian apologies - I was right the first time - #200

I've spent the past 30 minutes trying to find announcements or guidance on when normal_id_glm was introduced. it looks to me like a bunch of devs added stuff and assumed that what they added was just obvious. for our users, myself included, it's not. this is an ongoing problem - we get fabulous contributions to the code from sophisticated developers but the users need it explained to them like they're 7 - not that there's anything wrong with being 7.

@mitzimorris
Copy link
Member Author

As for why the coding was done that way, I don’t believe that will be more efficient than to_matrix. The implementation of to_matrix for vector types is just a forwarding constructor

I think I see why this was done - in a few models, you need to past together several inputs, and you end up writing things like this:

data {
  int<lower=1> N;
  int<lower=1> J; // number of counties
  array[N] int<lower=1, upper=J> county;
  vector[N] u;
  vector[N] x;
  vector[N] y;
}
transformed data {
  matrix[N, 2] xs = [x', u']';
}

to_matrix requires something like this:

matrix[N, 2] xs = to_matrix({x', u'})';

clear? does it blend? will test.

@mitzimorris
Copy link
Member Author

the names of the chapter 12 ARM models make no sense, and the description in the README is wrong.
these were developed 9 years ago by a Columbia undergrad and no one had the time to check them.

"no-pooling" and "complete-pooling" should be non-hierarchical models.

any model with "chr" - which perhaps stands for "centered hierarchical regression" - just a guess -
is a "partial pooling" model.

I doubt any of this is worth fixing, OTOH, keeping this around is an equally bad option. we should archive this altogether.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants