Closed jonathandroth closed 1 month ago
@jonathandroth Is this urgent?
If not, I want to take a beat to make sure I'm doing it right. (I got rid of some of the loops so I'd prefer to make sure I'm not messing something up inadvertently.) However, it's probably a straightforward change to L454 and L478 from
i_sel = this.use_last_treated_only? selectindex((g_all :> t_list[i]) :& (g_all :== g_max)): selectindex(g_all :> t_list[i])
to
i_sel = this.use_last_treated_only? selectindex((g_all :> t_list[i]) :& (g_all :== g_max)): selectindex(g_all :> max((g_list[i], t_list[i])))
I would say medium-priority; would ideally like to get it fixed relatively quickly, but fine if it takes a few days or a week.
The fix you propose looks right to me.
As a check, an event-study produced using this dataset using both positive/negative event-times should show essentially a straight line rather than a kink at -1. https://github.com/jonathandroth/HetEventStudies/blob/master/output/df.dta
And the change should not affect the estimates for the positive event times.
On Mon, Sep 23, 2024 at 4:56 PM Mauricio Caceres Bravo < @.***> wrote:
@jonathandroth https://github.com/jonathandroth Is this urgent?
If not, I want to take a beat to make sure I'm doing it right. (I got rid of some of the loops so I'd prefer to make sure I'm not messing something up inadvertently.) However, it's probably a straightforward change to L454 and L478 from
i_sel = this.use_last_treated_only? selectindex((g_all :> t_list[i]) :& (g_all :== g_max)): selectindex(g_all :> t_list[i])
to
i_sel = this.use_last_treated_only? selectindex((g_all :> t_list[i]) :& (g_all :== g_max)): selectindex(g_all :> max((g_list[i], t_list[i])))
— Reply to this email directly, view it on GitHub https://github.com/mcaceresb/stata-staggered/issues/3#issuecomment-2369366285, or unsubscribe https://github.com/notifications/unsubscribe-auth/AE6EXFBKH4EZ47XKZVLZBHLZYB57XAVCNFSM6AAAAABOW3CGCCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNRZGM3DMMRYGU . You are receiving this because you were mentioned.Message ID: @.***>
That example worked, but it's now failing some unit tests. I'll check it out tomorrow.
Ok, thanks! If there are further issues, let me know, as they possibly apply to the R package too
On Tue, Sep 24, 2024 at 1:14 AM Mauricio Caceres Bravo < @.***> wrote:
That example worked, but it's now failing some unit tests. I'll check it out tomorrow.
— Reply to this email directly, view it on GitHub https://github.com/mcaceresb/stata-staggered/issues/3#issuecomment-2370186361, or unsubscribe https://github.com/notifications/unsubscribe-auth/AE6EXFCAMI4NGSUOYFZC6BDZYDYJ7AVCNFSM6AAAAABOW3CGCCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNZQGE4DMMZWGE . You are receiving this because you were mentioned.Message ID: @.***>
@jonathandroth So sorry for the delay but this matches R now exactly.
Perfect! Can you update on ssc too? Thanks!
On Fri, Oct 4, 2024 at 11:29 PM Mauricio Caceres Bravo < @.***> wrote:
@jonathandroth https://github.com/jonathandroth So sorry for the delay but this matches R now exactly.
— Reply to this email directly, view it on GitHub https://github.com/mcaceresb/stata-staggered/issues/3#issuecomment-2394886865, or unsubscribe https://github.com/notifications/unsubscribe-auth/AE6EXFC4Z6AH4PAHKALBZVTZZ5MLJAVCNFSM6AAAAABOW3CGCCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGOJUHA4DMOBWGU . You are receiving this because you were mentioned.Message ID: @.***>
@jonathandroth Up in ssc
Thanks!
On Thu, Oct 10, 2024 at 2:06 PM Mauricio Caceres Bravo < @.***> wrote:
@jonathandroth https://github.com/jonathandroth Up in ssc
— Reply to this email directly, view it on GitHub https://github.com/mcaceresb/stata-staggered/issues/3#issuecomment-2405740868, or unsubscribe https://github.com/notifications/unsubscribe-auth/AE6EXFDCPWMHVQYN4QF6VZDZ2262ZAVCNFSM6AAAAABOW3CGCCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMBVG42DAOBWHA . You are receiving this because you were mentioned.Message ID: @.***>
Thanks!!!
Pedro H. C. Sant'Anna https://psantanna.com https://psantanna.com
On Thu, Oct 10, 2024 at 14:08 Jonathan Roth @.***> wrote:
Thanks!
On Thu, Oct 10, 2024 at 2:06 PM Mauricio Caceres Bravo < @.***> wrote:
@jonathandroth https://github.com/jonathandroth Up in ssc
— Reply to this email directly, view it on GitHub < https://github.com/mcaceresb/stata-staggered/issues/3#issuecomment-2405740868>,
or unsubscribe < https://github.com/notifications/unsubscribe-auth/AE6EXFDCPWMHVQYN4QF6VZDZ2262ZAVCNFSM6AAAAABOW3CGCCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMBVG42DAOBWHA>
. You are receiving this because you were mentioned.Message ID: @.***>
— Reply to this email directly, view it on GitHub https://github.com/mcaceresb/stata-staggered/issues/3#issuecomment-2405743822, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABE734Z3XODCVRT33EZKJWLZ227A7AVCNFSM6AAAAABOW3CGCCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMBVG42DGOBSGI . You are receiving this because you are subscribed to this thread.Message ID: @.***>
@mcaceresb I just pushed an update to the staggered R package that fixes a bug with the way placebos were constructed. In particular, placebos in the event-study were including a cohort g in its own control group. The fix was a one-line change in the R package, so hopefully it's straightforward to change here when you have the time? LMK if for some reason this would be more complicated. Thanks!