[WIP] Common test for sample weight by eickenberg · Pull Request #5461 · scikit-learn/scikit-learn

eickenberg · 2015-10-19T15:25:36Z

Trying to address #5444

I wrote a common test that goes through all classifiers and regressors and checks whether sample weights correspond to data augmentation. If the coef_ attribute is available, then it is also compared.

The estimators that fail this test are the following

[('AdaBoostRegressor', 'pred'),
 ('BaggingClassifier', 'pred'),
 ('BaggingRegressor', 'pred'),
 ('DecisionTreeRegressor', 'pred'),
 ('ExtraTreeClassifier', 'pred'),
 ('ExtraTreeRegressor', 'pred'),
 ('ExtraTreesClassifier', 'pred'),
 ('ExtraTreesRegressor', 'pred'),
 ('GradientBoostingRegressor', 'pred'),
 ('LogisticRegressionCV', 'coef_'),
 ('Perceptron', 'coef_'),
 ('Perceptron', 'pred'),
 ('RandomForestRegressor', 'pred'),
 ('RidgeCV', 'coef_'),
 ('RidgeCV', 'pred'),
 ('RidgeClassifierCV', 'coef_'),
 ('SGDClassifier', 'coef_'),
 ('SGDClassifier', 'pred'),
 ('SGDRegressor', 'coef_'),
 ('SGDRegressor', 'pred')]

…ck whether test should apply to all of the estimators

MechCoder · 2015-10-19T19:36:25Z

sklearn/tests/test_common.py

you can use in utils.validation.has_fit_parameter

MechCoder · 2015-10-19T19:37:37Z

Oops just realized this is a WIP. Sorry for commenting @eickenberg

amueller · 2015-10-21T09:37:09Z

for SGD it could be a convergence issue.
for LogisticRegressionCV, that has issues in master, see #5008.
I'm surprised by RidgeCV. It's odd that Ridge works but RidgeCV doesn't. Maybe sample_weights are not passed to the loss? Actually, I don't think we have a proper mechanism of passing sample_weights to scores yet :-/ RidgeCV doesn't use a score, though. So this smells fishy.

amueller · 2015-10-21T09:37:46Z

I have no explanation for the tree-based models. Maybe @arjoly or @glouppe or @jmschrei have?

amueller · 2015-10-21T09:40:22Z

Actually, sample weight support in scorers is fine.

glouppe · 2015-10-21T11:16:45Z

For random forest with bootstrap=True, it is very unlikely that will get identical results, even with the same random_state, because of the subsampling.

For methods using randomization, you should make sure to use the same random_state, which is not the case as far as I can tell.

Finally, because of numerical issues, it may happen that trees differ slightly, though the top parts should be consistent.

glouppe · 2015-10-21T11:21:10Z

sklearn/tests/test_common.py

random_state should be enforced here, if it is a parameter of Estimator.

there is the set_random_state helper for that.

amueller · 2015-10-21T11:33:34Z

whoops, I was confused about the bootstrapping. Obviously it's different.

amueller · 2015-10-21T11:34:14Z

Interestingly RandomForestClassifier passes but ExtraTreesClassifier fails

amueller · 2015-10-21T11:35:28Z

Oh, I had assumed the random state was fixed...

eickenberg · 2015-10-21T11:42:34Z

I'm surprised by RidgeCV

This is because sample_weight is broken for _RidgeGCV, see #4490

Discussing with @arjoly and @agramfort yesterday, the random nature of the subsampling causes the augmented data to be split in ways that are impossible with sample_weight set. If sample_weight is set to e.g. 3 for one sample, then in a random subsampling it will either be there with weight 3 or not be there, whereas the data augmented version can have e.g. representations of 2.

eickenberg · 2015-10-21T11:43:46Z

For random forest with bootstrap=True, it is very unlikely that will get identical results, even with the same random_state, because of the subsampling.

Oh, sorry, @glouppe, you had already mentioned that.

eickenberg · 2015-10-21T11:46:43Z

So the question becomes:

Is there a test that should invariably pass for everybody? Again, discussing with @arjoly and @agramfort, one thing that should always work is that a sample_weight of 0 should lead to neglect of the sample and thus be equivalent to omission of the sample. (I guess up to random_state again with ensembles ... :/)
If this cannot be applied globally, which subsets can we work with? E.g. the test that I wrote should definitely work with the linear_models. So I can remove it from test_common and make it more specific.

Any opinions on this?

amueller · 2015-10-21T12:01:09Z

Well you definitely have to fix the random state. Otherwise even the same parameters won't produce the same results ;)
The sample weight of 0 test sounds good. We can add tests that are only for linear models to the common test. I think there are some for class_weight. I would try to test as much as possible, though. Maybe just skip the RandomForestClassifier?

eickenberg · 2015-10-21T12:09:45Z

OK. @ainafp is taking over this PR.

eickenberg · 2015-10-21T15:04:26Z

superseded by #5515

eickenberg added 2 commits October 19, 2015 17:23

WIP adding common test for sample weights

23109f0

WIP testing structure in place, many tests failing. Proceeding to che…

7a0b752

…ck whether test should apply to all of the estimators

MechCoder reviewed Oct 19, 2015
View reviewed changes

sklearn/tests/test_common.py

Copy link
Copy Markdown

Member

MechCoder Oct 19, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can use in utils.validation.has_fit_parameter

MechCoder changed the title ~~Common test for sample weight~~ [WIP] Common test for sample weight Oct 19, 2015

glouppe reviewed Oct 21, 2015
View reviewed changes

ainafp mentioned this pull request Oct 21, 2015

[WIP] Sample weight consistency #5515

Closed

eickenberg closed this Oct 21, 2015

Uh oh!

Conversation

eickenberg commented Oct 19, 2015

Uh oh!

MechCoder Oct 19, 2015

Choose a reason for hiding this comment

Uh oh!

MechCoder commented Oct 19, 2015

Uh oh!

amueller commented Oct 21, 2015

Uh oh!

amueller commented Oct 21, 2015

Uh oh!

amueller commented Oct 21, 2015

Uh oh!

glouppe commented Oct 21, 2015

Uh oh!

glouppe Oct 21, 2015

Choose a reason for hiding this comment

Uh oh!

amueller Oct 21, 2015

Choose a reason for hiding this comment

Uh oh!

amueller commented Oct 21, 2015

Uh oh!

amueller commented Oct 21, 2015

Uh oh!

amueller commented Oct 21, 2015

Uh oh!

eickenberg commented Oct 21, 2015

Uh oh!

eickenberg commented Oct 21, 2015

Uh oh!

eickenberg commented Oct 21, 2015

Uh oh!

amueller commented Oct 21, 2015

Uh oh!

eickenberg commented Oct 21, 2015

Uh oh!

eickenberg commented Oct 21, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants