Improved handling of vcov. in summary(), anova(), confint(), etc. with enhanced documentation

[zeileis]

John, following the discussions with Diogo I have:

• improved the handling of vcov. in the summary() method, throwing a warning if it is a matrix but diagnostics = TRUE
• allowed to pass additional arguments to vcov. through ...
• ensured the same handling in anova() and confint()
• split all these functions into a separate R code file and corresponding Rd documentation
• added quite a few examples for the different handling of the vcov. argument
• updated DESCRIPTION and NEWS correspondingly

It would be great if you could have a look whether you see any problems or inconcsistencies. Thanks!

[john-d-fox]

In the explanation of the ... argument for summary.ivreg(), I'd explicitly mention that the supplied arguments can be passed to a function given to vcov.. Also, I don't think that the examples include this case (i.e. using ... to pass arguments to a vcov. function).

Otherwise, the changes to the code, documentation, etc., seem fine to me.

[john-d-fox]

Actually, I see that one of the examples does pass an argument to the vcov. function. Also, the use of ... to do this is documented in the vcov. argument. So, I think that the changes, including to the documentation and examples, are fine. Apologies for the false alarms.

[zeileis]

Thanks for the feedback John!
I thought it would be best to always use the same alternative covariance (and picked HC1 for that) and illustrate it in the different functions (summary, confint, anova).

https://github.com/zeileis/ivreg/blob/vcov/man/summary.ivreg.Rd#L117-L138

Do you think it would be better to use more than one type of covariance? Or should I bring out more clearly that vcov = vcovHC, type = "HC1" (function with ...) and vcov = hc1 (just function) and vcov = vc1 (matrix) are the three flavors and they yield identical output?

[john-d-fox]

I think that the current examples are adequate, though it might help to clarify the equivalence of different approaches; e.g.:

summary(m, vcov = vcovHC, type = "HC1") # type argument passed to vcovHC()
summary(m, vcov = hc1) # equivalent using user-defined hc1()

That said, it wouldn't hurt to introduce another example or examples with different vcov. functions.

[zeileis]

Thanks, John. I made the examples more explicit but I couldn't think of a good second vcov specification (HC1 is also what we use in the examples for the data set).

Moreover, I've added a testthat script to assure that the different vcov specifications yield the same result.

I'm running tests on the Win builder and will then send the new version to CRAN.

[john-d-fox]

I took a look at the latest changes and they seem fine to me. Thanks for doing all this.

[DiogoFerrari]

I tested the commit, and it is working well. The documentation is also clear. I would suggest enforcing the matrix rule, though, to avoid confusion and improve consistency. For instance,

This work as expected

summary(res, vcov. = sandwich::vcovCL,  diagnostics = T)$diag

And this returns NULL

summary(res, vcov. = sandwich::vcovCL(res), diagnostics = F)$diag

However, these two return the same diagnostics, but the vcov. the argument is ignored in the second line because it is a matrix (compare to the results of summary(res, vcov. = sandwich::vcovHC, diagnostics = T)$diag, for instance).

summary(res, diagnostics = T)$diag
summary(res, vcov. = sandwich::vcovHC(res), diagnostics = T)$diag

So, people can easily make mistakes when they write the second line, thinking they are getting the diagnostics with robust (or clustered; the same applies in that case) std. errors when they are not. The code won't tell them that they are using the incorrect result. If you ignore vcov. when a matrix is used, then it would be better to return NULL or through an error instead of silently ignoring that argument and returning the results as if that argument wasn't used.

[zeileis]

I'm not sure what you are saying here. Why do you find this surprising?

summary(res, vcov. = sandwich::vcovCL(res), diagnostics = F)$diag
## NULL

You have disabled the diagnostics (diagnostics = F) so you don't get any, no matter how vcov. is specified.

And in this case you get a warning:

summary(res, vcov. = sandwich::vcovCL(res), diagnostics = T)$diag
##                  df1 df2 statistic      p-value
## Weak instruments   1  45 45.157769 2.654508e-08
## Wu-Hausman         1  44  1.102002 2.995590e-01
## Sargan             0  NA        NA           NA
## Warning message:
## In summary.ivreg(res, vcov. = sandwich::vcovCL(res), diagnostics = T) :
##   'vcov.' must be a function for extracting covariance matrix estimates when
##   using 'diagnostics = TRUE', using 'vcov. = NULL' instead

The alternative would have been to throw an error which I found a bit extreme given we haven't even warned about this so far.

Note also that "enforcing the matrix rule" is not possible in this case because the variance-covariance matrix needs to be extracted from an auxiliary model and not just from res.

[zeileis]

In any case the package is out on CRAN now: https://CRAN.R-project.org/package=ivreg

The NEWS https://zeileis.github.io/ivreg/news/ explains the update and links to the ?summary.ivreg documentation.

[DiogoFerrari]

Ah, ok. I disable warnings by default bc I think they are annoying, so I didn't see you added that message when I ran it. In any case, I would throw an error or return Null if people tried to pass a matrix to vcov. to avoid mistakes. This is a design choice, but can induce mistake.

Thanks for adding the changes.