-
Notifications
You must be signed in to change notification settings - Fork 88
Add Policy Tree class for learning policies based on IRM #212
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
Conversation
doubleml/double_ml_irm.py
Outdated
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.
- remove the sentence "However, they ..." from the
depth
documentation. This should be part of the docs. - "learnt" to "learned" ? Maybe easier with US English
- Line 488.
groups
should bex_vars
. Maybe renamex_vars
tocovariates
orfeatures
. - is
min_samples_leaf=5
sufficient? I think, we should increase it slightly. - Line 486. Change "Default is 2" to "Default is
2
."
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.
- See documentation comments in
double_ml_irm.py
doubleml/tests/test.ipynb
Outdated
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.
Pls remove
doubleml/tests/test_irm.py
Outdated
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.
What are you testing specifically. Would this not be better at test_doubleml_return_types
?
Thank you for your review @SvenKlaassen
|
With this PR I would like to add a Policy Tree implemenation.
Description
DoubleMLPolicyTree
class that fits a weighted classification of arbitrary covariates on an orthogonal basis.policy_tree()
method to the IRM model, that fits and returns a Policy Tree based on the ATE score.Reference to Issues or PRs
Development of new features.
PR Checklist