@StefanieSenger I get what you meant when you said Ridge is confusing. cv can no longer be 'auto' (I assume thats been deprecated) so removing that helps.

Essentially when using cv=None, we use the default efficient LOO cv, in which case negative mean squared error is used. R2 is used for all other values of cv. Now that I've figured out what this means, it makes sense as it is and I can't think of a better way to put it without making it too long. Suggestions welcome.

That reads much clearer. :)

I am wondering, since there is no deprecation notification on cv for 'auto', if the gcv_mode is meant with it. In this case it would have to be mentioned here.

Edit: just checked: the reference to 'auto' first appeared in the 0.21 release (link), but cv didn't have the option 'auto' then.

Yeah no idea where 'auto' came from.

if the gcv_mode is meant with it.

I don't think so because I think mean squared error is the default no matter what gcv mode is:

also 'auto' could be either 'svd' or 'eigen' so it doesn't make sense to say score is only a specific thing when gcv_mode is auto.

There is a selector for each cv fold, so I think plural is right here

Thinking again about the use of the word 'mean' again, maybe it is used to indicate it is not the count (which you get with normalize=False) but the fraction/'mean' accuracy across the test set?

Following on from our conversation in #30316 (comment)

I do still agree that 'mean' is confusing. Not sure what a better solution would be though, 'fraction accuracy' ?

Yes, it seems so to me as well.

But accuracy is defined (acc = (TP+TN)/(all)) as a normalized fraction anyways and I think we don't need to provide the implementation details to the users here.

I've removed 'mean', but happy to change

Yes, it seems so to me as well.

But accuracy is defined (acc = (TP+TN)/(all)) as a normalized fraction anyways and I think we don't need to provide the implementation details to the users here.

Maybe like this?

Hmm maybe I don't understand this parameter. I thought this was the metric to use to see if we should stop early, it's not a score to return..? In which case 'compute score' seems misleading?

I'm not sure, but what I was thinking was that the scoring to be used for the predictions is computed from the loss somehow during fit time, for efficiency.

But true: the code is early stopping based on the scores archived and it doesn't seem to have anything to do with predict(). I am more confused about this than before. Please, feel free to disregard my suggestion.

I am more confused about this than before.

The meat of the early stopping code seems to be here:

AFAICT, we want to check if there has been improvement in the score of the predictions, and if not we stop early (before we reach max leaves or something). For 'loss' we just use the loss, otherwise a scorer function is used. Both calculate on predictions, but scorers have the signature (est, X, y), so we do it on a subset of train to avoid calling predict constantly.

Yes, after looking at it again I would agree.

The scoring param here is used differently than in the CV models or in cross validation, which is only for determining how we calculate early stopping (if it is enabled). It's the parameter name, that is confusing. But this cannot be changed so easily, so we need to live with it.

Going back to the documentation I see that it doesn't promise anything more than the scoring is used in early stopping: Scoring parameter to use for early stopping. Still, I wonder if this misunderstanding could be prevented for other people by rewording into Scoring to use for calculating early stopping. Or alternatively add as a last sentence, that this scoring param differs from the way scoring is defined in the Glossary. But it's also good as it is.

I see your point. I've altered it slightly to "Scoring method to use ..." because the 'parameter' part seemed redundant and possibly confusing.

I think it is clear enough in the parameter docstring that this is used only for early stopping. What we could do though is add an extra sentence in the glossary about how scoring is sometimes used to specify scoring method to use for early stopping as well?

What we could do though is add an extra sentence in the glossary about how scoring is sometimes used to specify scoring method to use for early stopping as well?

Oh yes, that's good. 👍

is sounds wrong, what about just:

That's perfect! :)

This constraint could be integrated with the description of callable a few lines above:

"callable with [signature ...], that returns a single value"

Should we also mention the default type here, similar as in the other parts of the docs?

There is no default for this one (private func), but I have looked at the rest and made them consistent, thanks!

The term "test set" here makes me feel as if the user had to provided the test set coming from a train-test split. I guess "validation set" or just "left out data" would be more appropriate, but maybe it's only me. Alternatively, we could use a wording similar to

Strategy to evaluate the performance of the model across cross-validation splits.

If you agree, this comment also applies to cross_val_score and maybe slightly adapt it for permutation_test_score.

I like:

Strategy to evaluate the performance of the model across cross-validation splits.

I've altered to use estimator, to reference the estimator parameter:

Strategy to evaluate the performance of the estimator across cross-validation splits.

Am happy to change though.

For permutation_test_score I have used "validation set", as we use CV to generate the null hypothesis distribution as well, so I thought "across cross-validation splits" was not as fitting. Happy to change.