Closed xinhew0708 closed 2 months ago
.cov_mat_est : <anonymous>: no visible global function definition for
‘cov’
(/Users/runner/work/propertee/propertee/check/propertee.Rcheck/00_pkg_src/propertee/R/SandwichLayerVariance.R:871)
.cov01_est : <anonymous>: no visible global function definition for
‘cov’
(/Users/runner/work/propertee/propertee/check/propertee.Rcheck/00_pkg_src/propertee/R/SandwichLayerVariance.R:912)
.cov01_est : <anonymous>: no visible global function definition for
‘cov’
(/Users/runner/work/propertee/propertee/check/propertee.Rcheck/00_pkg_src/propertee/R/SandwichLayerVariance.R:913)
.get_DB_wo_covadj_se: no visible global function definition for
‘aggregate’
(/Users/runner/work/propertee/propertee/check/propertee.Rcheck/00_pkg_src/propertee/R/SandwichLayerVariance.R:764)
.get_DB_wo_covadj_se: no visible binding for global variable ‘var’
(/Users/runner/work/propertee/propertee/check/propertee.Rcheck/00_pkg_src/propertee/R/SandwichLayerVariance.R:764)
.get_DB_wo_covadj_se: no visible global function definition for
‘aggregate’
(/Users/runner/work/propertee/propertee/check/propertee.Rcheck/00_pkg_src/propertee/R/SandwichLayerVariance.R:768)
.get_DB_wo_covadj_se: no visible binding for global variable ‘var’
(/Users/runner/work/propertee/propertee/check/propertee.Rcheck/00_pkg_src/propertee/R/SandwichLayerVariance.R:768)
.prepare_design_matrix: no visible global function definition for
‘aggregate’
(/Users/runner/work/propertee/propertee/check/propertee.Rcheck/00_pkg_src/propertee/R/SandwichLayerVariance.R:859)
For all of these, we need to import these functions. See e.g. the bread.lm function, where we have the @importFrom roxygen comment, and use stats::summary.lm
. You'll likely only need to import once (though it's no harm to do it in all the functions), but every place aggregate
, cov
, orvar
appears, they need to be prefaced by stats::
.
There's a test failing in the checks:
── Error ('test.SandwichLayerVariance.R:2314:3'): #177 vcov with by argument ───
Error in `.order_samples(x, by = by, verbose = FALSE)`: Contributions to covariance adjustment and/or effect estimation are not uniquely specified. Provide a `by` argument to `cov_adj()` or `vcov.teeMod()`
Can you look into that?
It looks like documentation is outdated - please run make document
or run Roxygen in RStudio to get those up-to-date.
Test coverage for .vcov_DB0
is a bit low.
Most of those are error conditions so not of utmost importance, but I think we could use a few more tests.
A test in the case of absorbed intercepts should be included.
Thanks, @josherrickson. I agree about more tests (in particular the absorption condition), but also propose that the call for additional tests not stand in the way of an initial merge, given our interest in having this functionality readily avaiable to people who may take an interests after my conference presentation on Friday. I'm hopeful that the adding of @import
directives, devtools::document()
update and dealing with the currently failing test can happen this week.
I agree, most tests can wait. The absorption test is a bit more critical; we don't necessarily need to check that its "correct", but I'd like to see the test suite hit it just to avoid embarrassing crashes in that code. @xinhew0708 can you create a placeholder test that will hit that code but not actually check the results other than ensure it doesn't crash?
I just cleaned up a few things. PS remember to update documentation every time you touch it. (Or, really, before any push.)
Thanks for all the constructive comments. Addressed most of them except for tests for absorption (placeholder test, https://github.com/benbhansen-stats/propertee/pull/185/commits/d1dbc0dc2c6ca6870fe8263fba49a73ab65da69b) and some remaining error conditions (added three tests for errors/warning, https://github.com/benbhansen-stats/propertee/pull/185/commits/7fbf8e2e103f0d7191d96ceb45987d08318f1aed).
Thanks so much everyone! @xinhew0708, please do complete the merge after making that last minor change that Josh W requested --- ideally by 3, when I'll be giving our talk. (I'm assuming that GitHub is presenting you with a "Merge pull request button" just below this comment. If your permissions are such that it isn't, just note here that it's ready to be merged so that one of the rest of us can do it.)
Design-based variance estimation
.vcov_DB0
, used for.estfun_DB_blockabsorb
calculates design-based estimating equation contributions following _estfunDA.texTo calculate a design-based variance estimate, use
lmitt()
orlm()
to fit ateeMod
object and callvcov_tee()
specifyingtype = "DB0"
.An example:
data(simdata)
des <- rct_design(z ~ cluster(uoa1, uoa2) + block(bid), simdata)
cmod <- lm(y ~ x, simdata)
teemod <- lmitt(y ~ 1, design = des, data = simdata, weights = ate(des), offset = cov_adj(cmod))
vcov_tee(teemod, type = "DB0")
Tests of the following
vcov_tee
returns the correct design-based SE for the main effect ofteeMod
objects with RCT designs without covariance adjustment and without absorption.estfun_DB_blockabsorb
calculates the correct contributions for estimating equationsTests for whether
vcov_tee
returns the correct design-based SE for the main effect ofteeMod
objects with RCT designs with prior covariance adjustment and without absorption still need to be developed.