-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Multiply robust #35
Multiply robust #35
Conversation
src/benchmark_mediation.py
Outdated
direct effect on the unexposed, | ||
indirect effect on the exposed, | ||
indirect effect on the unexposed, | ||
number of clipped samples] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 for consistency
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More importantly, there should be 6 output arguments, not one list.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
keep clipping but not return the number of clipped examples. And potentially open an issue to implement trimming in this method. And another issue to implement clipping in all methods and return 6 results instead of 5 in that case
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
keep clipping but not return the number of clipped examples. And potentially open an issue to implement trimming in this method. And another issue to implement clipping in all methods and return 6 results instead of 5 in that case
Let's just implement trimming for now, and remove clipping
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thx for the great job !
@@ -66,20 +66,22 @@ def get_interactions(interaction, *args): | |||
[ 2., 3., 1., 2., 2., 3., 4., 6., 2.], | |||
[ 4., 5., 1., 2., 4., 5., 8., 10., 2.]]) | |||
""" | |||
variables = args | |||
variables = list(args) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there any reason to sue this patterns. Arguments should be passed explicitly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you mean to use args directly rather than renaming it variables?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually you can specify a list of variables between which you want to compute interaction terms. Do you think this should be done differently?
src/benchmark_mediation.py
Outdated
direct effect on the unexposed, | ||
indirect effect on the exposed, | ||
indirect effect on the unexposed, | ||
number of clipped samples] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 for consistency
src/benchmark_mediation.py
Outdated
direct effect on the unexposed, | ||
indirect effect on the exposed, | ||
indirect effect on the unexposed, | ||
number of clipped samples] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More importantly, there should be 6 output arguments, not one list.
Multiply_robust estimator is fully implemented according Tchetgen Tchetgen 2012 and Huber 2016.
For linear generated data (even with x multidimensional), relative error is around 5% even for indirect effects, which is a noteworthy performance.
Docstring was changed, code was made readable, and .ravel() input was rectified (solving #34 and making #14 to progress).
Test code :