Make simulate_data() for internal use and apply in the testthat files

[gthopkins]

To folks wishing to recreate the figures in the appendix using the old simulate_data() code, take a visit to the radEmu supplementary repository. This is where we now keep the version that was used to generate the figures in the corresponding manuscript.

[svteichman]

Forgive me if you've already figured this out @gthopkins, but now that you made the simulate_data() function not exported, you'll have to update lines 90-91 of the clustered data vignette to call radEmu:::simulate_data() instead of just simulate_data(), and may want to choose an argument for mean_z to replicate the old version of the call which includes the old argument on the exponential scale, mean_count_before_ZI.

[gthopkins]

Thank you for the help @svteichman! I realized quickly that I need to call the function as radEmu:::simulate_data() in the vignette and testing section, but I figured I would finish updating the testing before correcting this issue in the PR. I will consider if I need to add more functionality besides the mean_z argument as I go through updating the vignette, thank you for the great tip!

[gthopkins]

@adw96 I have now changed the vast majority of the testthat code to instead use the simulate_data() function. With some minor adjustments to accomodate hard-coded thresholds, all but two tests still pass. However, there are two tests I truly do not understand, which I have temporarily commented out. I did not have enough documentation in the test-micro_wald.R to figure out what two tests are meant to confirm.

The two tests in question are:

expect_equal(wald_result$coefficients$pval, 0.61, tolerance = 0.02)

and, identically, in a seperate simulation

expect_equal(wald_result$coefficients$pval, 0.11, tolerance = 0.03)

Do you have any sense why we would want to confirm the p-values match these two arbitrary numbers? If not, could you connect me with someone who can clarify? Any p-value will be highly sensitive to the means of data generation, so I am not sure of the best way to proceed.

[gthopkins]

@adw96 I have added a "partially_verbose" argument to emuFit, which allows users to track progress of the algorithm (mostly score tests) without the annotations given by emuFit_micro_penalized(), emuFit_micro(), micro_wald(), or score_test(). This means we omit the lengthy technical messages in likeness of:

Max absolute difference in B since last augmented Lagrangian outer step: ___
Estimate deviates from feasibility by ___
Parameter u set to ___
Parameter rho set to ___
Iteration limit reached; exiting optimization.
Computing data augmentations for Firth penalty. For larger models, this may take some time.
Scaled norm of derivative ___

but we will still get insightful updates like:

Centering rows of B with pseudo-Huber smoothed median with smoothing parameter 0.1.
Performing Wald tests and constructing CIs.
[1] "Running score test 1 of 46 (row of B k = 2; column of B j = 1)."
...
[1] "Running score test 46 of 46 (row of B k = 2; column of B j = 1)."

This amounts to quite a simple change. Please note: this is now a "compound" pull request, in that it contains all of my commits pertaining to 1) changing the testing files and 2) the partially_verbose argument.

[svteichman]

@gthopkins overall this looks great! I see there is a merge conflict with "test-emuFit.R", could you please pull the recent updates to the package and resolve this merge conflict?

I think that everything from the simulate_data() update looks great.

For the partially verbose option, could you make partially verbose an option to add to the verbose argument instead of adding a new argument? I'm imagining that a user could input TRUE FALSE or "partial" (or something along those lines) for the verbose argument. Then you could add a check that verbose evaluates to one of these (and if not throw an error), and then internally if the user inputs verbose = "partial" then you could make a new variable partially_verbose, set verbose = TRUE, and then use all of the code that you currently have. I just recommend this in order to reduce the number of arguments (emuFit() already has so many) and hopefully streamline the process a tiny bit for the user.

[gthopkins]

@svteichman yes, I would be happy to make that change! Personally, I find very little value in the current "verbose = TRUE" argument. For example, the technical messages in likeness of

Max absolute difference in B since last augmented Lagrangian outer step: ___
Estimate deviates from feasibility by ___
Parameter u set to ___
Parameter rho set to ___
Iteration limit reached; exiting optimization.
Computing data augmentations for Firth penalty. For larger models, this may take some time.
Scaled norm of derivative ___

are out of context and do not clarify much for me. I propose the following three settings:

1. TRUE will correspond to big-picture messages, what we have been calling partially verbose
2. FALSE will correspond to no messages at all
3. "development" will display the all technical messages above in addition big-picture messages

What are your thoughts on this? I just think people are more likely to set "verbose = TRUE" as the immediate alternative to FALSE, but I do not think anyone would actually want the development messages.

[svteichman]

I think this is a great idea! Thanks for thinking through this. I use the verbose = T messages but I agree that a user will rarely need them.