Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Hyperparameter tuning for CausalForestDML #390
Hyperparameter tuning for CausalForestDML #390
Changes from 16 commits
6ff48e7
28776cd
2840eb9
052f727
7a3fc5f
6b0e34a
8fa571a
e9fc204
d38d87a
5066c4a
eaa6dc2
6e6e4bb
4c047ba
cd1f617
5ee83bc
bcf4940
e22723f
817d9e3
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Maybe explicitly mention that although the parameters will have been updated, this estimator will not have been fit with this data.
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.
now only final stage params are tunable
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.
I still think it's worth mentioning that this estimator has not been fit with this data, so that the user knows that the they still have to call fit afterward.
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.
Or alternatively, in your notebooks I think you always use:
Are there times a user would not want to do that, or should calling fit just be folded into tune so that it saves the user the trouble?
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.
That's what I'm doing in the notebooks I think.
I think it's good to have separate. Maybe someone wants to tune on a subset of the data or some small chunk. Also the tune does not need to take other keyword args like inference, cache_values etc.
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.
I added the extra docstring comment that the returned self is not yet fitted.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.