search

asheshrambachan / HonestDiD

Robust inference in difference-in-differences and event study designs
Other
175 stars 45 forks source link

Pull Request for #45: Error when running sensitivity analysis (custom reference period) #46

Closed mcaceresb closed 1 year ago

mcaceresb commented 1 year ago

45 seems to be the result of a time vector that is not coded with -1 as the reference period. This PR allows for a referencePeriod option in honest_did. Should be a straightforward edit. @jonathandroth

jonathandroth commented 1 year ago

Hmm, I'm a little unsure about this one. The core HonestDiD functions assume that in the event-study, the coefficient for relative time referencePeriod has been normalized to zero. But I think the did package constructs event-studies in a way such that the coefficient normalized to zero (if there is one) is always -1. So I'm not sure what use-case there would be such that you would want to use give HonestDiD a did event-study and incorrectly tell it that the reference period is something other than -1?

Maybe we should:

What do you think?

mcaceresb commented 1 year ago

@jonathandroth

But I think the did package constructs event-studies in a way such that the coefficient normalized to zero (if there is one) is always -1.

This is not true, right? As shown by this comment in #45 the did package maintains the gaps in your original time vector, which is normalized so 0 is the time of treatment (i.e. t - t* with t* first treated). This means -1 doesn't have to be the reference period; rather, it's max{t: t < t*} - t*. You can also try it in their README example by multiplying year and first.treat by come constant.

I haven't thought about this, but conceptually it makes sense to me that you can run an event study without consecutive time; more generally, you should be able to run an event study with a gap year, no? If you have -5 to -3 and -1 to 5 then you shouldn't throw out -5 to -3 just because you're missing -2, right? Mechanically did can run with both scenarios.

jonathandroth commented 1 year ago

It seems to me a rather knife-edge scenario that a user has an event-time -2 but not an event-time -1, no?

I'm also not entirely sure what a user wants HonestDiD to do if they input an event-study vector like -4,-2,0,...? Should M refer to the maximum change between consecutive periods or should it be between consecutive event-study coefficients? I.e. are we allowing for changes of M between -4 and -2, or changes of 2M?

My inclination is just to rule this out, but I could be convinced to allow it with warnings.

J

On Sat, Sep 30, 2023 at 9:56 AM Mauricio Caceres Bravo < @.***> wrote:

@jonathandroth https://github.com/jonathandroth

But I think the did package constructs event-studies in a way such that the coefficient normalized to zero (if there is one) is always -1.

This is not true, right? As shown by this comment https://github.com/asheshrambachan/HonestDiD/issues/45#issuecomment-1732612221 in #45 https://github.com/asheshrambachan/HonestDiD/issues/45 the did package maintains the gaps in your original time vector, which is normalized so 0 is the time of treatment (i.e. t - t with t first treated). This means -1 doesn't have to be the reference period; rather, it's max{t: t < t} - t. You can also try it in their README example by multiplying year and first.treat by come constant.

I haven't thought about this, but conceptually it makes sense to me that you can run an event study without consecutive time; more generally, you should be able to run an event study with a gap year, no? If you have -5 to -3 and -1 to 5 then you shouldn't throw out -5 to -3 just because you're missing -2, right? Mechanically did can run with both scenarios.

— Reply to this email directly, view it on GitHub https://github.com/asheshrambachan/HonestDiD/pull/46#issuecomment-1741772078, or unsubscribe https://github.com/notifications/unsubscribe-auth/AE6EXFCSTPXB757DWJ23UALX5AQHBANCNFSM6AAAAAA5FCBAKM . You are receiving this because you were mentioned.Message ID: @.***>

mcaceresb commented 1 year ago

@jonathandroth

I've made it spit out an error instead. LMK how it looks. This is the message:

honest_did expects a time vector with consecutive time periods;
please re-code your event study and interpret the results accordingly.

And if -1 is not the reference,

not enough post-periods (is your time vector is coded to have -1 as the reference?)
jonathandroth commented 1 year ago

The first error message looks good. Although maybe let's add "(e.g. using e=-5:5 as option for the att_gt command)" after "please re-code your event-study"?

I was a little confused by the second error message. Here you're checking if referencePeriod == -1 and then throwing that error if it evaluated to FALSE? Maybe let's just be more direct and say "The honest_did command only supports referencePeriod = -1"?

On Mon, Oct 2, 2023 at 12:07 PM Mauricio Caceres Bravo < @.***> wrote:

@jonathandroth https://github.com/jonathandroth

-

I agree an event study with a gap is certainly a corner case. You're right that it's not conceptually clear what the user wants if the event study vector is spaced by more than 1 (though I think that's more common). However, the only way HonestDiD can run following what you said here https://github.com/asheshrambachan/HonestDiD/pull/46#issuecomment-1739539303 is for M to refer to the maximum change between consecutive coefficients, right? (Assuming the user re-codes the event study.)

I think it's fine to rule out either case, but mechanically did can run in both and conceptually I thought that was fine (so it's not necessarily normalizing the vector to be consecutive numbers with -1 as the reference).

I've made it spit out an error instead. LMK how it looks. This is the message:

honest_did expects a time vector with consecutive time periods; please re-code your event study and interpret the results accordingly.

And if -1 is not the reference,

not enough post-periods (is your time vector is coded to have -1 as the reference?)

— Reply to this email directly, view it on GitHub https://github.com/asheshrambachan/HonestDiD/pull/46#issuecomment-1743301329, or unsubscribe https://github.com/notifications/unsubscribe-auth/AE6EXFAKQJX2BLRKRHV7DS3X5LRCXAVCNFSM6AAAAAA5FCBAKOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONBTGMYDCMZSHE . You are receiving this because you were mentioned.Message ID: @.***>

mcaceresb commented 1 year ago

@jonathandroth Almost. I'm technically checking if there are enough pre and post periods, and inferring the reason might be because the reference period is not coded as -1. That's what the message is trying to convey.

If the reference period is missing then the previous check will not fail even if it's not coded as -1. The way to figure out whether it's correctly coded is to make sure that at least -2 and 0 exist, basically.

Does that make sense? I figured saying what the check actually does was correct (i.e. not enough pre or post periods, as applicable, and suggest it's due to an improperly coded reference).

mcaceresb commented 1 year ago

@jonathandroth e=-5:5 is not an option in did::att_gt; is there an internal option to recode time? I'd have thought manual input was required.

jonathandroth commented 1 year ago

On the first point: what use-case do you have in mind where the user using the did package would set referencePeriod != -1?

On the second point: sorry, I referenced the wrong command. You get an event-study from did using something like:

es <- did::aggte(cs_results, type = "dynamic", min_e = -5, max_e = 5)

But then I'm confused as to how you would ever exclude -1? I guess it could only happen if there is a gap in the event-times (e.g. unbalanced panel)?

On Mon, Oct 2, 2023 at 7:15 PM Mauricio Caceres Bravo < @.***> wrote:

@jonathandroth https://github.com/jonathandroth e=-5:5 is not an option in did::att_gt; is there an internal option to recode time? I'd have thought manual input was required.

— Reply to this email directly, view it on GitHub https://github.com/asheshrambachan/HonestDiD/pull/46#issuecomment-1743902608, or unsubscribe https://github.com/notifications/unsubscribe-auth/AE6EXFDSOPSUMU76SVTQO7DX5NDIRAVCNFSM6AAAAAA5FCBAKOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONBTHEYDENRQHA . You are receiving this because you were mentioned.Message ID: @.***>

mcaceresb commented 1 year ago

@jonathandroth

On the first point: what use-case do you have in mind where the user using the did package would set referencePeriod != -1?

Perhaps checking the consecutive time periods obviates this issue? But #45 is an example where the reference period was -2 and there was no -1 because time was spaced. Either way I think we should check that there is at least pre and at least one post period. The error message can instead note say HonestDiD assumes -1 is the reference period? E.g.

not enough post-periods (check your time vector; note honest_did takes -1 as the reference period)

On the second point: sorry, I referenced the wrong command. You get an event-study from did using something like: es <- did::aggte(cs_results, type = "dynamic", min_e = -5, max_e = 5) But then I'm confused as to how you would ever exclude -1? I guess it could only happen if there is a gap in the event-times (e.g. unbalanced panel)?

Can't you have a gap with a balanced panel? Take a balanced panel and put the treated in the same cohort, then drop the treated year -1 for everyone. aggte omits -1. (I don't think min_e and max_e help with this or the time vector gaps; note both are included in #45.)

mcaceresb commented 1 year ago

@jonathandroth I think here we're set on the behavior and we were just debating what error message and suggestion to give the user, right?

jonathandroth commented 1 year ago

I think that's right.

Your post made me wonder whether we should allow for evenly-space periods that are not consecutive (e.g. data is for 2000, 2004, 2008, etc), but I'm inclined to leave it as is for now

On Sun, Oct 22, 2023 at 3:51 PM Mauricio Caceres Bravo < @.***> wrote:

@jonathandroth https://github.com/jonathandroth I think here we're set on the behavior and we were just debating what error message and suggestion to give the user, right?

— Reply to this email directly, view it on GitHub https://github.com/asheshrambachan/HonestDiD/pull/46#issuecomment-1774183588, or unsubscribe https://github.com/notifications/unsubscribe-auth/AE6EXFC7DKCI2K3TZ5HQ4XDYAV2NFAVCNFSM6AAAAAA5FCBAKOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONZUGE4DGNJYHA . You are receiving this because you were mentioned.Message ID: @.***>