Add 1st stage regression in Feiv class

[Jayhyung]

Hi Alex @s3alfisc! More details with this PR will follow soon.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Files	Coverage Δ
pyfixest/estimation/FixestMulti_.py	82.54% <ø> (ø)
pyfixest/estimation/feiv_.py	97.61% <100.00%> (ø)
tests/test_iv.py	100.00% <100.00%> (ø)

... and 27 files with indirect coverage changes

[s3alfisc]

Very cool! Looking forward to it =)

[Jayhyung]

There are a few concerns that I have working on this issue.

1. Does feiv class in the codebase allow multiple endogenous independent variables? I'm asking this question because the IV test cases in the test code(line 97 ~ 117 in test_vs_fixest.py) only allow the single case. The first stage in my code only allows the single case, but if you would like to generalize the code that allows the multiple cases, I'm happy to work more on this!
2. I defined several attributes within feiv class related with the first stage regression(_pi_hat : estimated coefficient, X_hat : predicted values, _v_hat : residuals). Please, let me know if you want me to define more/less attributes related with the first stage.
3. I would add a test code that evaluates whether the result(e.g. _pi_hat) of the first stage regression is the same with the result from feols. Do you think this is a reasonable way to test the 1st stage part of the get_fit method in feiv class? Also, where do i have to add the test code in test_vs_fixest.py exactly?

Thanks!

[s3alfisc]

Hi @Jayhyung - sorry for my delayed response, I have been swamped for work over tethe last days :/

Does feiv class in the codebase allow multiple endogenous independent variables? I'm asking this question because the IV test cases in the test code(line 97 ~ 117 in test_vs_fixest.py) only allow the single case. The first stage in my code only allows the single case, but if you would like to generalize the code that allows the multiple cases, I'm happy to work more on this!

Yes, only one endogenous variable is allowed. It would be quite cool to allow for more, but this would require to rework the FixestFormulaParser and it might not be a small endeavor. So for now, I would recommend to maybe keep this as a separate issue?

I defined several attributes within feiv class related with the first stage regression(_pi_hat : estimated coefficient, X_hat : predicted values, _v_hat : residuals). Please, let me know if you want me to define more/less attributes related with the first stage.

For now, I think that coefficient, predicted values and residuals is all that we need, right? So this sounds good to me =)

I would add a test code that evaluates whether the result(e.g. _pi_hat) of the first stage regression is the same with the result from feols. Do you think this is a reasonable way to test the 1st stage part of the get_fit method in feiv class? Also, where do i have to add the test code in test_vs_fixest.py exactly?

Sounds like a good strategy =) I think that you could just create a new test file, e.g. test_iv.py?

[Jayhyung]

Thanks for the feedback! I will work on my next iteration. Stay tuned. :)

[Jayhyung]

Just added the error testing file! Please let me know if anything extra should be done.

[s3alfisc]

I think it correct that the endogenous variable is always the first one in _X. But it would likely be safer to use the endogvar created via the model_matrix_fixest function?

This would require to add an endogvar argument to Feiv.__init__ and to pass it to Feiv.get_fit().

We would also have to demean endogvar in FixestMulti.

Alternatively, we could also keep this as is, as the unit tests you've implemented would certainly catch it if we broke this logic. =)

What do you think @Jayhyung ?

[Jayhyung]

I think your practice is more sensible as it encompasses more general cases. I will take this into account in the next iteration!

[Jayhyung]

I added endogvar as you suggested to the next iteration. Let me know if we need anything extra to be done!

[s3alfisc]

Small formatting issue here =)

[Jayhyung]

Thanks! Just fixed the formatting issue. Let me know if anything comes up!

[s3alfisc]

Perfect. It's merged =) thanks @Jayhyung!