Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixest vcov catches #357

Merged
merged 10 commits into from Aug 16, 2021

Conversation

grantmcdermott
Copy link
Contributor

@grantmcdermott grantmcdermott commented Aug 15, 2021

I know you (we!) largely came out against hardcoding specific model classes in #280. But been I've running into this whilst teaching, so thought I might as well have a go at it. I won't be offended if you decide not to merge given the aforementioned stance, but I do think the current behaviour is a little sub-optimal given the popularity of fixest.

(Note: If you're game, I can probably do something similar for lfe::felm, which also requires a couple of control flow catches. estimatr is harder b/c --- unlike lfe::felm --- it doesn't include the iid vcov errors as part of the return object).

cc @elbersb

Examples:.

devtools::load_all('~/Documents/Projects/modelsummary/')
#> ℹ Loading modelsummary
library(fixest)

mod_lm =         lm(hp ~ mpg + drat, mtcars)
mod_feols =   feols(hp ~ mpg + drat, mtcars, vcov = ~vs)

mods = list('lm' = mod_lm, 'feols' = mod_feols)

msummary(mods, gof_omit = 'R2|IC|Log|F', output = 'markdown')
lm feols
(Intercept) 278.515 278.515
(55.415) (20.573)
mpg -9.985 -9.985
(1.792) (4.110)
drat 19.126 19.126
(20.198) (32.644)
Num.Obs. 32 32
Std.Errors Clustered (vs)
msummary(mods, vcov = 'iid', gof_omit = 'R2|IC|Log|F', output = 'markdown') 
lm feols
(Intercept) 278.515 278.515
(55.415) (55.415)
mpg -9.985 -9.985
(1.792) (1.792)
drat 19.126 19.126
(20.198) (20.198)
Num.Obs. 32 32
Std.Errors IID IID
## Aside: this "doesn't" work...
msummary(mods, vcov = c('iid', NULL), gof_omit = 'R2|IC|Log|F', output = 'markdown')
lm feols
(Intercept) 278.515 278.515
(55.415) (55.415)
mpg -9.985 -9.985
(1.792) (1.792)
drat 19.126 19.126
(20.198) (20.198)
Num.Obs. 32 32
Std.Errors IID IID
## ... but only b/c the vcov list drops the NULL element (as it should)
c('iid', NULL)
#> [1] "iid"

msummary(mods, vcov = 'robust', gof_omit = 'R2|IC|Log|F', output = 'markdown') 
lm feols
(Intercept) 278.515 278.515
(46.992) (46.992)
mpg -9.985 -9.985
(2.471) (2.471)
drat 19.126 19.126
(23.133) (23.133)
Num.Obs. 32 32
Std.Errors Robust Robust
msummary(mods, vcov = 'HC1', gof_omit = 'R2|IC|Log|F', output = 'markdown')
lm feols
(Intercept) 278.515 278.515
(42.037) (42.037)
mpg -9.985 -9.985
(2.209) (2.209)
drat 19.126 19.126
(20.572) (20.572)
Num.Obs. 32 32
Std.Errors HC1 HC1
msummary(mods, vcov = ~ vs, gof_omit = 'R2|IC|Log|F', output = 'markdown')
lm feols
(Intercept) 278.515 278.515
(20.573) (20.573)
mpg -9.985 -9.985
(4.110) (4.110)
drat 19.126 19.126
(32.644) (32.644)
Num.Obs. 32 32
Std.Errors C: vs C: vs
msummary(mods, vcov = ~ cyl, gof_omit = 'R2|IC|Log|F', output = 'markdown')
lm feols
(Intercept) 278.515 278.515
(40.571) (40.571)
mpg -9.985 -9.985
(3.571) (3.571)
drat 19.126 19.126
(34.092) (34.092)
Num.Obs. 32 32
Std.Errors C: cyl C: cyl

Created on 2021-08-14 by the reprex package (v2.0.0)

R/get_vcov.R Outdated
@@ -15,6 +15,42 @@ get_vcov <- function(model, vcov = NULL, conf_level = NULL, ...) {
"The `vcov` argument accepts a variance-covariance matrix, a vector of standard errors, or a ",
"function that returns one of these, such as `stats::vcov`.")

## fixest models
if (class(model) %in% c("fixest", "fixest_multi")) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My guess is the test failures are due to this line, because class(model) is a vector with two elements for these classes. So I think you need class(model)[1] %in% ... or maybe inherits(model, "fixest") || inherits(model, "fixest_multi")

@vincentarelbundock
Copy link
Owner

This looks fantastic, thanks! I'd be happy to merge when tests pass. I'm pretty sure I identified the culprit in my comment above, and it should be a very easy fix.

@grantmcdermott
Copy link
Contributor Author

I updated with your hotfix and also added some better checking + flow, depending on the fixest version. Looks like everything is passing except for an annoying warning about an undeclared dependency (Matrix?) for lfe, which I added to the "hardcoded" test suite. Happy to remove for now if that's easiest.

@vincentarelbundock vincentarelbundock merged commit d6492f6 into vincentarelbundock:main Aug 16, 2021
@vincentarelbundock
Copy link
Owner

Fantastic!

@grantmcdermott grantmcdermott deleted the fixest_vcov branch August 16, 2021 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants