dmphillippo / multinma

Network meta-analysis of individual and aggregate data in Stan
https://dmphillippo.github.io/multinma
35 stars 17 forks source link

Fix centering a single variable #7

Closed ndunnewind closed 3 years ago

ndunnewind commented 3 years ago

Currently, if length(xbar) == 1, the entire column is overwritten with the first centered observation.

Another option would be to use base R sweep(), as used in the relative_effects() function:

https://github.com/dmphillippo/multinma/blob/227fb6e2272baa689308228134a18a97af2b18c8/R/relative_effects.R#L300

dmphillippo commented 3 years ago

Hi @ndunnewind, there doesn't seem to be an actual bug here - since dat_all is a tibble we have drop = FALSE by default. Regression and centering with a single variable are working just fine for me, see e.g. the BCG vaccine example.

Are you seeing this breaking for you?

I am happy to merge the change anyway, since having drop = FALSE explicit is more defensive code.

ndunnewind commented 3 years ago

Thanks for looking into this! The BCG vaccine example works fine. However, when trying to predict absolute effects a warning is given, as shown below. dat_all is not a tibble in this case.

> predict(bcg_fit_lat, newdata = data.frame(latitude = seq(10, 50, 10)), baseline = distr(qnorm))
------------------------------------------------------------------ Study: New 1 ---- 

                           mean   sd  2.5%   25%  50%  75% 97.5% Bulk_ESS Tail_ESS Rhat
pred[New 1: Unvaccinated]  0.01 1.02 -1.99 -0.69 0.02 0.69  2.01     4053     4014    1
pred[New 1: Vaccinated]   -0.01 1.04 -2.09 -0.73 0.00 0.68  2.03     3983     4102    1

Warning message:
In `[<-.data.frame`(`*tmp*`, , names(xbar), value = list(-23.4615384615385,  :
  provided 10 variables to replace 1 variables

Apparently, this is because I'm passing a data.frame as newdata. This is what I get when using a tibble instead.

> predict(bcg_fit_lat, newdata = tibble(latitude = seq(10, 50, 10)), baseline = distr(qnorm))
------------------------------------------------------------------ Study: New 1 ---- 

                           mean   sd  2.5%   25%   50%  75% 97.5% Bulk_ESS Tail_ESS Rhat
pred[New 1: Unvaccinated]  0.00 1.01 -2.04 -0.68  0.02 0.68  1.99     3915     3320    1
pred[New 1: Vaccinated]   -0.66 1.02 -2.69 -1.35 -0.63 0.03  1.39     3926     3615    1

I think my proposed fix would be the easiest and safest way to solve this issue.

dmphillippo commented 3 years ago

Good spot, thank you! I agree, this is the best fix. Merging now for the next release.