sergiocorreia / ivreghdfe

Run IV/2SLS with many levels of fixed effects (i.e. ivreg2+reghdfe)
MIT License
77 stars 27 forks source link

Incorrect results with `absorb`? #13

Closed mcaceresb closed 5 years ago

mcaceresb commented 5 years ago

I'd assume this is an issue stemming from ftools or reghdfe, since this repo hasn't changed in some time. But I am not sure what part of it is causing this issue. In any case, the results form an IV with absorb are wrong:

local sergio https://raw.githubusercontent.com/sergiocorreia

cap ado uninstall reghdfe
cap ado uninstall ftools
cap ado uninstall ivreghdfe

net install reghdfe, from(`sergio'/reghdfe/master/src/) replace
net install ftools,  from(`sergio'/ftools/master/src/)  replace

reghdfe, compile
ftools,  compile

ssc install ivreg2
net install ivreghdfe, from(`sergio'/ivreghdfe/master/src/) replace

sysuse auto, clear
xi: ivreghdfe price (mpg = foreign) i.rep78
xi: ivreg     price (mpg = foreign) i.rep78
xi: ivreghdfe price (mpg = foreign), absorb(rep78)

The first two give the exact same coefficient, 10.335, but the third gives 0.0208.

sergiocorreia commented 5 years ago

I think I know the problem. Yesterday I changed the reghdfe code to keep by default the variables standardized after reghdfe (it improve numerical accuracy).

The fix should be relatively simple (changing an argument in the call to partial_out()) but we are swamped with recruiting this week so I'm only going to be able to update it tomorrow evening,. at the earliest.

(on the meantime, You can use reghdfe with the old option to run the IV though)

sergiocorreia commented 5 years ago

Actually, a better soln osnto change the reghdfe code so it does not returned standardized values by default. There might be other programs out there using the internals of reghdfe that might get affected otherwise

mcaceresb commented 5 years ago

i just tested the release from the 27th and it works fine, but the release from the 28th introduces the problem (reghdfe).

What is the old option to run the IV? No idea this was a thing.

sergiocorreia commented 5 years ago

Makes sense; I'll work on a patch asap tomorrow evening

About old, you can add -old- to the options, and it will run reghdfe version 3, which supported IV regressions directly.

On Tue, Jan 29, 2019, 2:51 AM Mauricio Caceres Bravo < notifications@github.com wrote:

i just tested the release from the 27th and it works fine, but the release from the 28th introduces the problem (reghdfe).

What is the old option to run the IV? No idea this was a thing.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/sergiocorreia/ivreghdfe/issues/13#issuecomment-458440563, or mute the thread https://github.com/notifications/unsubscribe-auth/AANEKBhg6YOEU9qG2gqwJT3Evt7ZiiSFks5vH_1wgaJpZM4aXdap .

sergiocorreia commented 5 years ago

Just pushed a commit to reghdfe that should fix this issue. Let me know if it works on your side.

(BTW, you don't need to use the xi: prefix anymore, as both ivreg2 and reghdfe support factor variables! see help xi for more details)

mcaceresb commented 5 years ago

Yup, looking good now!

PS: xi was there bc ivreg didn't support factor variables and my mind really disliked the code alignment otherwise. I mean, the third regression didn't even use factor variables and I still put a xi in front.