-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
var_model test_causality vs test_inst_causality redundant code? #4463
Comments
@jbrockmendel Add the name of the module, so we don't have to guess. I never looked at test_inst_causality. |
I'll have to take a closer look then to be sure. Strong prior for code being duplicated based on a) its in var_model and b) test_inst_causality came in with vecm. |
check some references, it's not easy to see from the code what it is supposed to do. I'm pretty sure that it is a test for the covariance/correlation structure (like a off-diagonal block of zeros) |
You're right, that does sound like a problem in and of itself. Lines 1760-1770 look analogous to 1633-1653. Then:
vs
... aren't quite doing the same thing, but look related. I'll see if I can sort this out. |
Uh also it looks like VARResults.cov_params may having a wrapping problem
|
open a separate bug issue for VARResults.cov_params I'll let you figure out the two causality tests, but they look very different to me, the first quote is making a restriction matrix for params, the second is making a restriction matrix for vech(sigma_u) (naming cov_resid = sigma_u, IIRC) |
The construction of the restriction matrix is done differently in these two methods, but I suspect they end up equivalnet
The text was updated successfully, but these errors were encountered: