Skip to content
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

float numbers can't be set to RFECV's parameter "step" #7467

Closed
lijierr opened this issue Sep 22, 2016 · 7 comments
Closed

float numbers can't be set to RFECV's parameter "step" #7467

lijierr opened this issue Sep 22, 2016 · 7 comments
Labels
Milestone

Comments

@lijierr
Copy link

lijierr commented Sep 22, 2016

Description

When I use RFECV with parameter 'step' as a float number will cause warnings/errors "rfe.py:203: VisibleDeprecationWarning: using a non-integer number instead of an integer will result in an error in the future". And the analysis can't be finished until integer or 1/2.

I read description of RFECV and learned that parameter 'step' can accept float. (introduction online: If greater than or equal to 1, then step corresponds to the (integer) number of features to remove at each iteration. If within (0.0, 1.0), then step corresponds to the percentage (rounded down) of features to remove at each iteration.)

And I didn't read any bugs from source script. Please tell.

@lesteve
Copy link
Member

lesteve commented Sep 22, 2016

What would be best is if you could provide a standalone snippet reproducing the problem.

@jnothman
Copy link
Member

It's actually pretty easy to see in the code where this comes about. The
float semantics are handled for RFE, but not properly for RFECV. This is a
bug. PR welcome.

On 22 September 2016 at 22:16, Loïc Estève notifications@github.com wrote:

What would be best is if you could provide a standalone snippet
reproducing the problem.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#7467 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAEz69klwjPadKdidokVOuzcInf3oHwEks5qsnGxgaJpZM4KDpAQ
.

@jnothman jnothman added the Bug label Sep 22, 2016
@naoyak
Copy link
Contributor

naoyak commented Sep 23, 2016

Isn't it OK for RFECV.fit() to omit the step validation, since the underlying RFE.fit() call checks this for each fold iteration?

It sounds like the warning is caused due to indexing by float, these lines in 0.17.x might be causing CFE to be initialized with n_features_to_select as float. @joanna-li what version are you on?

@jnothman
Copy link
Member

Step is used multiple times. Not all are through RFE

@amueller amueller modified the milestone: 0.19 Sep 29, 2016
@PedroGFonseca
Copy link

Can reproduce on sklearn '0.18.dev0'.

Code sample (works fine with RFE and fails with RFECV):

from sklearn.feature_selection import RFECV

selector = RFECV(model_classifier, step=0.1, cv=3, 
                 scoring='roc_auc', verbose=10, n_jobs=-1)

selector.fit(X_train, y_train)

Symptoms:

  • Process goes on forever without ending.
  • No cores seem to activate.
  • Stdout prints: ... lib/python3.4/site-packages/sklearn/feature_selection/rfe.py:198: DeprecationWarning: using a non-integer number instead of an integer will result in an error in the future support_[features[ranks][:threshold]] = False
  • Stdout also prints:
    Fitting estimator with 177 features. Fitting estimator with 177 features. Fitting estimator with 177 features. Fitting estimator with 124 features. Fitting estimator with 124 features. Fitting estimator with 71 features. Fitting estimator with 71 features. Fitting estimator with 124 features. Fitting estimator with 18 features. Fitting estimator with 18 features. Fitting estimator with 71 features. Fitting estimator with 18 features.
    (but nothing actually happens)
  • Works fine with integer step

@jnothman
Copy link
Member

That's a lot of symptoms! Yes, we know it's not fixed.

NelleV pushed a commit that referenced this issue Sep 30, 2016
* Fixing error with step parameter #7467. Basically converting the step parameter the same way RFE does.

* Adding in an explanation to the test case.

* Moving the location of where we set the data.

* Adding in an assertion against the data.
@NelleV
Copy link
Member

NelleV commented Sep 30, 2016

Fixed in #7469

@NelleV NelleV closed this as completed Sep 30, 2016
TomDLT pushed a commit to TomDLT/scikit-learn that referenced this issue Oct 3, 2016
…arn#7469)

* Fixing error with step parameter scikit-learn#7467. Basically converting the step parameter the same way RFE does.

* Adding in an explanation to the test case.

* Moving the location of where we set the data.

* Adding in an assertion against the data.
amueller pushed a commit to amueller/scikit-learn that referenced this issue Oct 14, 2016
…arn#7469)

* Fixing error with step parameter scikit-learn#7467. Basically converting the step parameter the same way RFE does.

* Adding in an explanation to the test case.

* Moving the location of where we set the data.

* Adding in an assertion against the data.
Sundrique pushed a commit to Sundrique/scikit-learn that referenced this issue Jun 14, 2017
…arn#7469)

* Fixing error with step parameter scikit-learn#7467. Basically converting the step parameter the same way RFE does.

* Adding in an explanation to the test case.

* Moving the location of where we set the data.

* Adding in an assertion against the data.
paulha pushed a commit to paulha/scikit-learn that referenced this issue Aug 19, 2017
…arn#7469)

* Fixing error with step parameter scikit-learn#7467. Basically converting the step parameter the same way RFE does.

* Adding in an explanation to the test case.

* Moving the location of where we set the data.

* Adding in an assertion against the data.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants