JMSLab / xtevent

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

Inclusion of endpoints when executing xteventtest after adjusting with a linear trend #166

Closed Constantino-Carreto-Romero closed 10 months ago

Constantino-Carreto-Romero commented 10 months ago

After running xtevent with the trend option to adjust for a linear trend, when executing xteventtest the endpoints should not be considered in the tests.

For instance, when executing:

*Setup
webuse nlswork, clear
*year variable has many missing observations.
*Create a time variable that ignores these gaps.
by idcode (year): gen time=_n
xtset idcode time
*Generate a policy variable that follows staggered adoption.
by idcode (time): gen union2 = sum(union)
replace union2 = 1 if union2 > 1
*Adjust the pre-trend by estimating a linear trend by OLS
xtevent ln_w age c.age#c.age ttl_exp c.ttl_exp#c.ttl_exp tenure , pol(union2) w(-5 9) cluster(idcode) impute(stag) trend(-4, method(ols) saveoverlay) 

If we then run:

*leveling off
xteventtest, overidpost(2)

we get:

1

but the coefficients considered for the test should be 8 and 9.

Constantino-Carreto-Romero commented 10 months ago

@jorpppp

jorpppp commented 10 months ago

Thanks @Constantino-Carreto-Romero . We should be able to adjust the value of overidpre and overidpost that xtevent will use if the xtevent call used the trend option. Why don't you give it a shot?

@jmshapir @SimonFreyaldenhoven FYI

Constantino-Carreto-Romero commented 10 months ago

@jorpppp sure thing, I'll try changing xteventtest's overid option. For instance, if first calling xtevent with overidpre(5) (and pre(0)) and trend(-3, method(ols)), then the available pre-event coefficients for xteventtest's are only -5 and -4. xteventtest will try to test pre-trends using the first 5 pre-event coefficients, since now it will not find 5 pre-event coefficients available, but a smaller number of coefficients, it should try using all available pre-event coefficients. I think it should also show some message saying the change in the number of coefficients used for the test and what these coefficients are.

jorpppp commented 10 months ago

@Constantino-Carreto-Romero Yes, this sounds like a good plan. On some instances there won't be any coefficients available to test, in which case we may also want to have an informative error message.

Constantino-Carreto-Romero commented 10 months ago

update: I've not made progress, but I'm planning to retake today's afternoon.

Constantino-Carreto-Romero commented 10 months ago

@jorpppp

Constantino-Carreto-Romero commented 10 months ago

Summary: In this issue, we made changes to xteventtest, so it doesn't use the endpoints in any of its tests if previously xtevent used linear trend adjustment. We also modified the number of coefficients it uses to test pre-trends in some cases if previously xtevent used linear trend adjustment with the ols method.

Changes brought to main in https://github.com/JMSLab/xtevent/commit/4198174fab4a012e3ab69eca0b76f2ecc9199e40

Thread continues in https://github.com/JMSLab/xtevent/pull/167