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

LogisticRegressionCV.score doesn't respect scoring, inconsistent with GridSearchCV #10998

Closed
amueller opened this Issue Apr 19, 2018 · 11 comments

Comments

Projects
None yet
5 participants
@amueller
Member

amueller commented Apr 19, 2018

From what I can see, LogisticRegressionCV.score always computes accuracy, not the metric given by scoring. That's inconsistent with the behavior of GridSearchCV. How do other CV estimators do that? I assume the same?
That's pretty unfortunate and I think we should change it. We did the same backward incompatible change in GridSearchCV before with a warning.

@amueller amueller added this to the 0.20 milestone Apr 19, 2018

@amueller

This comment has been minimized.

Show comment
Hide comment
@amueller

amueller Apr 19, 2018

Member

I feel this is a duplicate but can't find any other issues. #2709 is somewhat related.
Also related to #4668 though I think the issue here is more clear as the user provided a metric.
Looks like I complained about that in GridSearchCV in 2013 #1831

Member

amueller commented Apr 19, 2018

I feel this is a duplicate but can't find any other issues. #2709 is somewhat related.
Also related to #4668 though I think the issue here is more clear as the user provided a metric.
Looks like I complained about that in GridSearchCV in 2013 #1831

@kris-singh

This comment has been minimized.

Show comment
Hide comment
@kris-singh

kris-singh Apr 25, 2018

Just looked at the code for LogisticRegressionCV.score. Can you give an example of what you are saying it is not very clear from the code? LogisticRegressionCv calls the _log_reg_scoring_path function which does compute the scores based on the given metric.

kris-singh commented Apr 25, 2018

Just looked at the code for LogisticRegressionCV.score. Can you give an example of what you are saying it is not very clear from the code? LogisticRegressionCv calls the _log_reg_scoring_path function which does compute the scores based on the given metric.

@jnothman

This comment has been minimized.

Show comment
Hide comment
@jnothman

jnothman Apr 25, 2018

Member
Member

jnothman commented Apr 25, 2018

@amueller

This comment has been minimized.

Show comment
Hide comment
@amueller

amueller Apr 30, 2018

Member

@jnothman hm that might be a good idea, though maybe slightly magic? It might change derived classes in user code. Though it might change it to the expected result...

Member

amueller commented Apr 30, 2018

@jnothman hm that might be a good idea, though maybe slightly magic? It might change derived classes in user code. Though it might change it to the expected result...

@jnothman

This comment has been minimized.

Show comment
Hide comment
@jnothman

jnothman Apr 30, 2018

Member
Member

jnothman commented Apr 30, 2018

@amueller

This comment has been minimized.

Show comment
Hide comment
@amueller

amueller Apr 30, 2018

Member

@agramfort and @GaelVaroquaux might have opinions?

We could add a "common" check for everything having a scoring parameter though.

Member

amueller commented Apr 30, 2018

@agramfort and @GaelVaroquaux might have opinions?

We could add a "common" check for everything having a scoring parameter though.

@agramfort

This comment has been minimized.

Show comment
Hide comment
@agramfort

agramfort May 1, 2018

Member

indeed it's a bug. It's because the score method from LogisticRegressionCV is derived from LogisticRegression. We need to add a score method in LogisticRegressionCV that is using self.scoring

Member

agramfort commented May 1, 2018

indeed it's a bug. It's because the score method from LogisticRegressionCV is derived from LogisticRegression. We need to add a score method in LogisticRegressionCV that is using self.scoring

@kris-singh

This comment has been minimized.

Show comment
Hide comment
@kris-singh

kris-singh May 1, 2018

I would like to take this up. If the method suggested @agramfort is the way to go I feel I can contribute to this.

kris-singh commented May 1, 2018

I would like to take this up. If the method suggested @agramfort is the way to go I feel I can contribute to this.

@amueller

This comment has been minimized.

Show comment
Hide comment
@amueller

amueller May 1, 2018

Member

Yes, and adding tests. But we also need to check that this is the same for other *CV estimators.

Member

amueller commented May 1, 2018

Yes, and adding tests. But we also need to check that this is the same for other *CV estimators.

@reijerh

This comment has been minimized.

Show comment
Hide comment
@reijerh

reijerh May 28, 2018

I'm not sure I'm posting this in the right place, but I came here via https://datascience.stackexchange.com/questions/17620/scoring-argument-in-scikit-learn-lassocv-lassolarscv-elasticnetcv because I expected to be able to pass a custom scoring function to ElasticNetCV, but couldn't find a way to do it. This issue seemed relevant.

Just wanted to give you a headsup, I'm not even sure it's at all possible to implement custom scoring for ElasticNetCV.

reijerh commented May 28, 2018

I'm not sure I'm posting this in the right place, but I came here via https://datascience.stackexchange.com/questions/17620/scoring-argument-in-scikit-learn-lassocv-lassolarscv-elasticnetcv because I expected to be able to pass a custom scoring function to ElasticNetCV, but couldn't find a way to do it. This issue seemed relevant.

Just wanted to give you a headsup, I'm not even sure it's at all possible to implement custom scoring for ElasticNetCV.

@agramfort

This comment has been minimized.

Show comment
Hide comment
@agramfort

agramfort May 28, 2018

Member
Member

agramfort commented May 28, 2018

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