Create `estfun.DirectAdjusted` and `bread.DirectAdjusted` methods and deprecate "B" matrix calculations

[jwasserman2]

This PR largely addresses #79 but also address parts of #55 and #83.

Major Changes Addressing #79

• estfun.DirectAdjusted and bread.DirectAdjusted methods have been created. Their outputs align with the desiderata in estfun_DA.tex
  • Tests of their basic functionality have been added to test.DirectAdjusted.R
  • Tests of their outputs have been added to vignettes/not-for-cran/estfun_dot_DirectAdjusted_testing.R and test.SandwichLayerVariance.R
• Higher-level variance estimation functions such as the user-facing vcovDA and the HC0 model-based variance estimator .vcov_MB_CR0 have been updated in SandwichLayerVariance.R to account for the new DirectAdjusted methods
• An internal function .make_uoa_ids() has been created that generates a vector the same length as the number of rows of the output of estfun.DirectAdjusted. These ID's indicate the unit of assignment each row pertains to and are passed on to the cluster argument of internal vcovCL calls. Testing for this function have been added to test.SandwichLayerVariance.R
• Functions that had previously been written to calculate B matrices, as we had called them, and their corresponding tests in the testing suite have been deprecated and written to vignettes/not-for-cran/deprecated_vcovDA_functions.R and vignettes/not-for-cran/test.deprecated_vcovDA_functions.R. @josherrickson how do you feel about this? Should I leave these functions in the R folder and just make them internal functions, also keeping the tests in the testing suite?

Changes Addressing #83

• estfun.lmrob and bread.lmrob have been moved to lmrob_methods.R within the R folder. I've added a few tests to improve our testing coverage of them, but I still haven't implemented the tests/organized the repo in the way we've discussed on that thread (I'll get to those tasks after I get to a few other priorities)

Changes Addressing #55

• Another test has been added that I'd been interested in including. We've done pretty solid testing on vcovDA, and the only situation I would like to add is the HC1 adjustment. That would ask for a new model-based variance estimator function to be written, which I'll get to in another PR

[josherrickson]

Re: b functions. Are they completely defunct in that we don't even use them internally? Or are they still used internally but there's a different external function?

[jwasserman2]

Re: b functions. Are they completely defunct in that we don't even use them internally? Or are they still used internally but there's a different external function?

They aren't even used internally anymore

[josherrickson]

I see no reason to keep them around then?

[benthestatistician]

Thanks for this very nice PR, @jwasserman2 . I look forward to reviewing it. In advance of that review I'll chime in on the issue of the b functions, which I'm guessing may be left there for us to use to cross-check results from the newer architecture. @josherrickson, I had recommended to Josh W that we keep them available to us for that purpose. Of course I may be mistaken about all of this, not yet having made my review; please don't hesitate to correct me @jwasserman2 .

[jwasserman2]

I had recommended to Josh W that we keep them available to us for that purpose

I don't remember if we've discussed this before, but I stashed them for that reason, especially since we don't have a versioning system set up for the package yet. Ideally we would snapshot the current repo, then once this has been reviewed we can merge it into main and make a major dev increment for the package version

[josherrickson]

I have no objection to keeping them around for testing purposes - I think we should move them out of the actual package (e.g. move to them to ~/btesting and .Rbuildignore them) if they're not used anymore just to avoid issues.

[benthestatistician]

I worry that stashing the functions in an odd place could introduce its own issues. I also worry about .Rbuildignore-ing them, as it makes them available to us in some circumstances but not others. In addition, as we move toward broader distribution, there may be scenarios were exchange detailed notes with users outside of the development team; I would want that functionality to be on the table to us then. So I propose that we keep them in the usual places, but refrain from exporting them.

[benthestatistician]

@jwasserman2 and I had an offline discussion about deprecated functions. One issue is to avoid having them clutter the documentation. Neither of us knew precisely how to circumvent having roxygen2 create man pages for a function that we didn't want to be advertising to the public, but that we wanted to continue to have and continue to document within the source code.

Here I'm piping up to note that one can avoid having the man page get built by omitting a title and description. Here's an example from something else I'm doing. Note that the initial comment block has lines starting with ## not ##' .

##  Positions of 0 are ignored. The function doesn't forbid
##  both of +/- i appearing in `positions`, corresponding to a
##  unit available as both treatment and control.
##' @keywords internal
##' @param positions integer. Positive ~ treatment; negative ~ control
indices_bipartite_pairs  <- function(positions) {<...>}

Using roxygen2 version 7.1.2.

[josherrickson]

That'll produce warnings on creation of documentation - shouldn't be an issue for CRAN but not ideal.

Looks like @noRd will suppress creation of the man file. https://roxygen2.r-lib.org/articles/rd.html

[benthestatistician]

Good point @josherrickson . & thanks for the @noRd tip; that'll be useful elsewhere as well.

[jwasserman2]

I've just added the absorbed_intercepts and absorbed_moderators slots as logical and character vectors, respectively to DirectAdjusted objects @benthestatistician @xinhew0708 @josherrickson. Model-based standard error computations error out when the absorbed_intercepts slot is TRUE (only possible for DirectAdjusted objects created using lmitt.formula with absorb=TRUE). When Xinhe creates her design-based standard error function, she can allow those standard error computations to go through. Should we also error when moderated treatment effects are present (since there's absorption going on in the background)?

[jwasserman2]

I just wrote some tests to make sure that vcovDA returns the correct SE's when moderators and intercepts either are or aren't absorbed when using the code existing on the absorb branch. I'm going to merge this branch into main, then after @josherrickson looks to merge absorb into main (which I think we'll likely need to do together), I will add these tests to the testing suite (I have them saved locally for now).

[benthestatistician]

Does the existing estfun_DA() code incorporate the term reflecting absorption of blocks intercepts, in that scenario? I.e., the final term in this expression from the spec:

I understand that this may have evolved since the merge of the estfun branch, but this seemed the best place for the question. @jwasserman2 .

[jwasserman2]

estfun.DirectAdjusted doesn't incorporate that term in either case of absorb = TRUE or absorb = FALSE

[benthestatistician]

That's good b/c I now think I was mistaken to suggest that it should go into what estful.DirectAdjusted produces. I think we'll still want it in some circumstances affecting design-based estimation, but then we'll want a separate function to produce it. 9a47d5f updates estfun_DA.tex accordingly.