-
Notifications
You must be signed in to change notification settings - Fork 763
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
Allow parallel prediction for UpliftRandomForestClassifier #477
Allow parallel prediction for UpliftRandomForestClassifier #477
Conversation
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.
Thanks @heiderich! Looks good to me. I left one minor change request.
computation for UpliftRandomForestClassifier
of UpliftRandomForestClassifier
of UpliftRandomForestClassifier. It is passed as `prefer` to joblib.Parallel in the `fit` and `predict` methods.
e1ba553
to
b84f114
Compare
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.
LGTM. Thanks!
@@ -1256,7 +1260,8 @@ class UpliftRandomForestClassifier: | |||
n_reg=10, | |||
evaluationFunction='KL', | |||
normalization=True, | |||
n_jobs=-1): | |||
n_jobs=-1, | |||
joblib_prefer: str = "threads"): |
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.
nit: We're going to use black
as a formatter as discussed in #474. You can use it integrated in your IDE or run its command line tool to ensure that the code style follows PEP-8. e.g. no white space around =
when used in input arguments.
Since we already have a separate PR, you don't need to update the style here.
Proposed changes
Previous to this PR
UpliftRandomForestClassifier
was usingjoblib
to fit theUpliftTreeClassifier
s in parallel ifn_jobs
was not equal to 1. With this PR predictions are also computed in parallel (one job per tree).Types of changes
What types of changes does your code introduce to CausalML?
Put an
x
in the boxes that applyChecklist
Put an
x
in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.