-
Notifications
You must be signed in to change notification settings - Fork 1.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] FIX use a stump as base estimator in RUSBoostClassifier #545
[MRG] FIX use a stump as base estimator in RUSBoostClassifier #545
Conversation
@@ -151,7 +152,7 @@ def fit(self, X, y, sample_weight=None): | |||
super(RUSBoostClassifier, self).fit(X, y, sample_weight) | |||
return self | |||
|
|||
def _validate_estimator(self, default=DecisionTreeClassifier()): | |||
def _validate_estimator(self, default=DecisionTreeClassifier(max_depth=1)): |
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 think that you are right. We should actually call the parent class then:
https://github.com/scikit-learn/scikit-learn/blob/7389dba/sklearn/ensemble/weight_boosting.py#L414
@glemaitre do you have any clue why this test is failing? Isn't it weird? |
Codecov Report
@@ Coverage Diff @@
## master #545 +/- ##
==========================================
- Coverage 98.88% 98.83% -0.06%
==========================================
Files 84 82 -2
Lines 5184 5043 -141
==========================================
- Hits 5126 4984 -142
- Misses 58 59 +1
Continue to review full report at Codecov.
|
@glemaitre early stopping issue |
@glemaitre as you can see using a stump as the default estimator I believe that the performance ain't so good. So, what do you propose? To make a benchmark using stump and a full decision tree and decide then, make the stump default to be in line with scikit-learn or leave the full grown tree as default? |
What does this implement/fix? Explain your changes.
I don't think that the referenced paper mentions something about stumps but this change keep us in line with the original implementation of
AdaBoostClassifier