Placebo tests and extended plotting functionality #88
Conversation
Rmd file and data for replication of Abadie et al. (2010)
-updates functions in augsynth.R -adds permutation.R (also contains some plotting functions) -updates package documentation
Updates the github directory in tobacco replication vignette
Replaces convention of inf = F with inf_type = 'none'.
- Separates out inference from estimation. Sets augsynth(inf = "none") by default, which should be backwards compatible. - Adds plot_type as an argument for plot.augsynth (allows user to specify the "name" of the desired outputted plot). - Adds the option to generate a donor table (synthetic control weights and RMSPEs for any inference type) - Ensures that all existing tests execute successfully.
Updates to abadie et al 2010 replication notebook
Adds warnings to multi-outcome and multisynth models if a user attempts to pass in `inf_type` at the time of model fitting.
Minor edits to tobacco replication notebook
…synth class methods for accessing data characteristics.
- converts the tobacco replication notebook to a vignette - removes cache folders and plot folder from the Abadie et al. (2010) tobacco replication notebook
…ome of the MDES calculation stuff from the permutation package to clarify inference side.
There was a problem hiding this comment.
Overall I think there needs to be a structural change to keep inference calculations in summary.augsynth. The new add_inference function seems to be necessary only to add inference output to the augsynth function, but that should be restricted to the summary.augsynth function. I've called out parts of the code where this can be changed, but mostly it means that all of the new plotting work should happen in the plot.summary.augsynth function, and then the augsynth object never needs any inference added to it.
| pull(quo_name(unit)) %>% unique() | ||
| augsynth$time_var <- quo_name(time) | ||
| augsynth$unit_var <- quo_name(unit) | ||
| augsynth$raw_data <- data |
There was a problem hiding this comment.
It's not clear to me why the output object needs to carry the raw data around. Can this be removed?
There was a problem hiding this comment.
We need the raw_data attribute for the update_augsynth() function (which drops items based on RMSPE, etc.).
|
|
||
|
|
||
|
|
||
| test_that( "MDES_table corresponds to default treatment table", { |
There was a problem hiding this comment.
Can this test be broken up into subtests, since it looks like there are a few distinct things.
There was a problem hiding this comment.
Another good test would be that the placebo gaps agree with manually changing the treatment variable in the data and re-running augsynth.
…nor tests make sense.
… into tobacco example more.
- adds a test for update_augsynth() ensuring that non-binding update criteria (ex. RMPSE multiple of infinity) return the same donor weights as the original model. - removes superfluous datasets froom the tobacco_replication.Rmd file - updates a test in test-donor_control.R to resolve an error from a variable that is called but not defined (or used as part of the test, as far as I can tell)
The proposed changes:
All of the changes to the augsynth package itself take place in augsynth.R and a new file, permutation.R.
The
tobacco_replicationdirectory contains a vignette demonstrating the new functionality and replicating the results of Abadie et al. (2010).