-
-
Notifications
You must be signed in to change notification settings - Fork 25.3k
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
[MRG] accelerate stacking-classifier in test_common.py::test_ensemble_heterogeneous_estimators_behavior #21562
[MRG] accelerate stacking-classifier in test_common.py::test_ensemble_heterogeneous_estimators_behavior #21562
Conversation
@@ -32,7 +32,8 @@ | |||
("lr", LogisticRegression()), | |||
("svm", LinearSVC()), | |||
("rf", RandomForestClassifier()), |
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! I think it would be possible to make it run even faster with:
("rf", RandomForestClassifier()), | |
("rf", RandomForestClassifier(n_estimators=5, max_depth=3)), |
Could you also update the related parametrized config for the same test function below?
Please also report the timings you get when running this test on your local machine with --durations=10
.
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.
@ogrisel Thank you very much for your suggestions. Note that this PR is still WIP and I plan to finish the optimization with your advice and the appropriate measurements reported.
Speed improvements reported as test duration 5-times averaged:
Note: With final improvement LR fit: 27ms, RF fit: 17ms, , SVM fit: 1ms. |
Based on @ogrisel suggestion I have committed the speed optimization of
|
…nto test-speed-improvement-stacking-classifier
…nto test-speed-improvement-stacking-classifier
…nto test-speed-improvement-stacking-classifier
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.
Thank you for the PR @chritter !
LGTM!
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 very much! LGTM.
…e_heterogeneous_estimators_behavior (scikit-learn#21562)
…e_heterogeneous_estimators_behavior (scikit-learn#21562)
…e_heterogeneous_estimators_behavior (#21562)
Reference Issues/PRs
Towards #21407
What does this implement/fix? Explain your changes.
These changes are accelerating test case
test_common.py::test_ensemble_heterogeneous_estimators_behavior
Any other comments?
#DataUmbrella Sprint