sample_weights array can't be used with GridSearchCV #2879

Closed
shenberg opened this Issue Feb 20, 2014 · 12 comments

Comments

Projects
None yet
7 participants
@shenberg

The internal cross-validation isn't aware of sample weights, so and exception is thrown if a sample_weights sequence is passed to the grid search, because fit_grid_point does not split the weights into training and test sets.

@ndawe

This comment has been minimized.

Show comment
Hide comment
@ndawe

ndawe Feb 20, 2014

Member

I have added support for sample_weight in GridSearchCV in #1574. Hopefully I can get that merged soon.

Member

ndawe commented Feb 20, 2014

I have added support for sample_weight in GridSearchCV in #1574. Hopefully I can get that merged soon.

@shenberg

This comment has been minimized.

Show comment
Hide comment
@shenberg

shenberg Feb 23, 2014

Thanks, that was extremely fast turn-around.

As a meta-question, is sampling weighted? e.g. Is it better for stratified folds to try maintain the ratio of total weight per class instead of the number of samples?

Thanks, that was extremely fast turn-around.

As a meta-question, is sampling weighted? e.g. Is it better for stratified folds to try maintain the ratio of total weight per class instead of the number of samples?

@amueller

This comment has been minimized.

Show comment
Hide comment
@amueller

amueller Oct 27, 2016

Member

sample_weight support was merged long ago

Member

amueller commented Oct 27, 2016

sample_weight support was merged long ago

@amueller amueller closed this Oct 27, 2016

@jnothman

This comment has been minimized.

Show comment
Hide comment
@jnothman

jnothman Oct 29, 2016

Member

@amueller, sample_weight is still not supported as an argument to fit in *SearchCV

Member

jnothman commented Oct 29, 2016

@amueller, sample_weight is still not supported as an argument to fit in *SearchCV

@jnothman jnothman reopened this Oct 29, 2016

@amueller

This comment has been minimized.

Show comment
Hide comment
@amueller

amueller Nov 1, 2016

Member

There is fit_params

Member

amueller commented Nov 1, 2016

There is fit_params

@stephen-hoover

This comment has been minimized.

Show comment
Hide comment
@stephen-hoover

stephen-hoover Jan 31, 2017

Contributor

@amueller , I'm running into a similar issue. It's possible to set a sample_weights array as an instance attribute via the GridSearchCV.fit_params dictionary, and then everything works fine. But this fails in nested cross-validation with model_selection.cross_val_predict, because the GridSearchCV.fit (and RandomizedSearchCV.fit) method doesn't accept fit parameters.

Is there a reason why the BaseSearchCV subclasses can't accept fit parameters? Having to set data-dependent fit parameters as instance attributes appears to contradict http://scikit-learn.org/stable/developers/contributing.html#fitting .

I would be willing to make that change (adding fit parameters) if you'd accept that PR.

Contributor

stephen-hoover commented Jan 31, 2017

@amueller , I'm running into a similar issue. It's possible to set a sample_weights array as an instance attribute via the GridSearchCV.fit_params dictionary, and then everything works fine. But this fails in nested cross-validation with model_selection.cross_val_predict, because the GridSearchCV.fit (and RandomizedSearchCV.fit) method doesn't accept fit parameters.

Is there a reason why the BaseSearchCV subclasses can't accept fit parameters? Having to set data-dependent fit parameters as instance attributes appears to contradict http://scikit-learn.org/stable/developers/contributing.html#fitting .

I would be willing to make that change (adding fit parameters) if you'd accept that PR.

@jnothman

This comment has been minimized.

Show comment
Hide comment
@jnothman

jnothman Feb 2, 2017

Member

Yes, it's broken. One issue is that we need to be clear whether sample_weight is being passed only to scoring, only to fit, or both. Feel free to propose and champion a solution.

Member

jnothman commented Feb 2, 2017

Yes, it's broken. One issue is that we need to be clear whether sample_weight is being passed only to scoring, only to fit, or both. Feel free to propose and champion a solution.

@ManasHardas

This comment has been minimized.

Show comment
Hide comment
@ManasHardas

ManasHardas Dec 13, 2017

fit_params is deprecated since 0.19 and will be removed in 0.21
How else to make "sample_weights" a part of cross-validation?

fit_params is deprecated since 0.19 and will be removed in 0.21
How else to make "sample_weights" a part of cross-validation?

@amueller

This comment has been minimized.

Show comment
Hide comment
@amueller

amueller Dec 13, 2017

Member

@ManasHardas pass them to fit.

Member

amueller commented Dec 13, 2017

@ManasHardas pass them to fit.

@farfan92

This comment has been minimized.

Show comment
Hide comment
@farfan92

farfan92 Feb 5, 2018

@amueller Trying your solution when using RandomizedSearchCV, with a RandomForestClassifier.

Attempting to pass: fit_params ={'sample_weight':s_weights}
to the .fit method of RandomizedSearchCV results in

TypeError: fit() got an unexpected keyword argument 'fit_params'

But will still manage to run with the deprecation warning when passed to the constructor instead.

farfan92 commented Feb 5, 2018

@amueller Trying your solution when using RandomizedSearchCV, with a RandomForestClassifier.

Attempting to pass: fit_params ={'sample_weight':s_weights}
to the .fit method of RandomizedSearchCV results in

TypeError: fit() got an unexpected keyword argument 'fit_params'

But will still manage to run with the deprecation warning when passed to the constructor instead.

@jnothman

This comment has been minimized.

Show comment
Hide comment
@jnothman

jnothman Feb 5, 2018

Member
Member

jnothman commented Feb 5, 2018

@farfan92

This comment has been minimized.

Show comment
Hide comment
@farfan92

farfan92 Feb 5, 2018

I see, should have more closely looked at doc

farfan92 commented Feb 5, 2018

I see, should have more closely looked at doc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment