search

JMSLab / xtevent

Stata package -xtevent-
MIT License
43 stars 12 forks source link

xtevent 2.2.0 #109

Closed jorpppp closed 1 year ago

jorpppp commented 1 year ago

In this issue we'll update documentation for a new release.

jorpppp commented 1 year ago

@Constantino-Carreto-Romero The two last examples from the home page are not great, because it turns out that the post-event coefficients are ommitted. I am unsure if this expected behavior. Many observations are dropped when compared to the OLS estimates. Can you check this, please?

jorpppp commented 1 year ago

@Constantino-Carreto-Romero After digging deeper, I think I caught a bug and a possible enhancement here.

When _eventiv selects the lead of the differenced policy to use as an instrument, it does not check if there are missing values of the instrument. In short windows, the lead of the differenced policy may be missing for higher order leads, so comparing F-stats across regressions will be misleading. This is something we should enhance.

Moreover, when _eventivchecks for collinearity between the IV and the event-time dummies, it does so by regressing the IV on the dummies and checking if r2 is 1. This is not a good way of doing this, because Stata will drop collinear variables. We should be doing this with _rmcoll. This is the bug.

@Constantino-Carreto-Romero: If you agree, please create issues for both of these problems, and please try to come up with an example without these collinearity issues for the homepage. I am thinking that at least the bug needs to be fixed before a release.

Constantino-Carreto-Romero commented 1 year ago

When _eventiv selects the lead of the differenced policy to use as an instrument, it does not check if there are missing values of the instrument. In short windows, the lead of the differenced policy may be missing for higher order leads, so comparing F-stats across regressions will be misleading. This is something we should enhance.

I got it, I think the regression loop should start with the greatest lead, then mark the sample, and then run the rest of the regressions in the loop with the same sample.

Moreover, when _eventivchecks for collinearity between the IV and the event-time dummies, it does so by regressing the IV on the dummies and checking if r2 is 1. This is not a good way of doing this, because Stata will drop collinear variables. We should be doing this with _rmcoll. This is the bug.

Ok, I will implement that check with _rmcoll.

@Constantino-Carreto-Romero: If you agree, please create issues for both of these problems, and please try to come up with an example without these collinearity issues for the homepage. I am thinking that at least the bug needs to be fixed before a release.

Ok, I'll start working on the examples and open the issues.

santiagohermo commented 1 year ago

Hi @jorpppp @Constantino-Carreto-Romero. Just wanted to let you know that, in the README, the link below is broken. It looks like the link points to the issue folder in #103, but since it's not a permanent link it stopped working after the branch was closed.

image

santiagohermo commented 1 year ago

One more minor thing to point out @jorpppp @Constantino-Carreto-Romero:

I think there is a typo in line 932 in xteventplot

https://github.com/JMSLab/xtevent/blob/f08d735ef43512f8c8c2c2a57387c268b57216e5/xtevent/xteventplot.ado#L931-L932

It should actually read 

            param = F*aresult
            WB = (dhat-param)'*Vhatinv*(dhat-param)
Constantino-Carreto-Romero commented 1 year ago

Thanks! @santiagohermo I will open the issues for https://github.com/JMSLab/xtevent/issues/109#issuecomment-1429733191 and https://github.com/JMSLab/xtevent/issues/109#issuecomment-1440266068.

SimonFreyaldenhoven commented 1 year ago

@jorpppp how do feel about releasing a new version soon, now that we fixed a couple more bugs (leveling-off p-vale, critical value in smoothest path)?

I think it would be great to get any bug fixes out there quickly once we've fixed them, we can always include further enhancements n the next version.

jorpppp commented 1 year ago

Yes, I think we should release a new version soon. Only #126 is unfinished, let me talk to @Constantino-Carreto-Romero to see how quickly we can take care of it.

On Fri, Mar 10, 2023 at 9:44 AM Simon Freyaldenhoven < @.***> wrote:

@jorpppp https://github.com/jorpppp how do feel about releasing a new version soon, now that we fixed a couple more bugs (leveling-off p-vale, critical value in smoothest path)?

I think it would be great to get any bug fixes out there quickly once we've fixed them, we can always include further enhancements n the next version.

— Reply to this email directly, view it on GitHub https://github.com/JMSLab/xtevent/issues/109#issuecomment-1463991271, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEBUYUHW3RHZRA4VZWHRLHLW3ND6RANCNFSM6AAAAAAQYHM2AY . You are receiving this because you were mentioned.Message ID: @.***>

jorpppp commented 1 year ago

@SimonFreyaldenhoven #126 is done so we'll start working on the release. Any additional comments on #136 ?

jorpppp commented 1 year ago

With #136 and #120 done we'll go back to the checklist above and work on the release.

jorpppp commented 1 year ago

All tasks done, starting a PR for the release!

jorpppp commented 1 year ago

Summary: In this issue we reviewed xtevent for a 2.2.0 release.

Thread continues in #146