Note that for CV estimators (ElasticNetCV, ...), when no cv parameter is given, then the random state is not passed to the cross-validation split, which may lead to non-deterministic behaviors.

Note that for CV estimators (ElasticNetCV, ...), when no cv parameter is given, then the random state is not passed to the cross-validation split, which may lead to non-deterministic behaviors.

Interesting, it seems except for LogisticRegressionCV the CV estimators are passing this test. Should I consider that a bug of the test?

Do we have good reason to believe there are multiple estimators that should fail such a test in scikit-learn?

In terms of the specifics, equivalence of predict is going to be a very weak test. Classifiers, for instance, may have subtly different boundaries, but produce the same predictions. The assert_model_equal helper proposed at #4841 might be more robust as a test.

You might also consider whether some datasets are more likely to elicit variation than others. For example, I'd consider using a dataset with a random target so that a meaningful classifier is unlikely learnable.

I should add: without something like assert_model_equal, decision_function remains a more powerful test for classifiers than predict

Do we have good reason to believe there are multiple estimators that should fail such a test in scikit-learn?

Given @TomDLT's comment about the *CV estimators not passing on random_state I would expect all of them to fail this test. Without a fixed CV split they should give (statistical) equivalent but not identical estimators. IMHO this test should test for identity not (statistically) equivalence.

I like the idea on the random dataset.

The other thing that comes to mind here is that we may have false negatives
if, as is often the case, a parameter controls the use of randomness. This
is a limitation of check_estimator altogether that assumes an estimator
class is homogeneous in its behaviour regardless of parameters.

On 29 August 2016 at 22:05, Tim Head notifications@github.com wrote:

Do we have good reason to believe there are multiple estimators that
should fail such a test in scikit-learn?

Given @TomDLT https://github.com/TomDLT's comment about the *CV
estimators not passing on random_state I would expect all of them to fail
this test. Without a fixed CV split they should give (statistical)
equivalent but not identical estimators. IMHO this test should test for
identity not (statistically) equivalence.

I like the idea on the random dataset.

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

The other thing that comes to mind here is that we may have false negatives
if, as is often the case, a parameter controls the use of randomness.

Starting with the different solvers that may exists in one estimator.

On 08/29/2016 06:05 AM, Tom Dupré la Tour wrote:

Note that for CV estimators (|ElasticNetCV|, ...), when no |cv|
parameter is given, then the random state is not passed to the
cross-validation split, which may lead to non-deterministic behaviors.

All the default ones use deterministic splitting.

On 08/29/2016 08:08 AM, Joel Nothman wrote:

The other thing that comes to mind here is that we may have false
negatives
if, as is often the case, a parameter controls the use of randomness. This
is a limitation of check_estimator altogether that assumes an estimator
class is homogeneous in its behaviour regardless of parameters.
yeah, we could remedy that with stronger annotations.
If an estimator has several "modes" (like solvers) we could provide
dictionaries
that enumerate them. That's sort of heavy-handed, though.
The definition would basically be "the set of parameters so that trying all
explores all paths". Though there could still be data-dependent paths
in the code that are harder to explore.

If we would "just" describe the legal space of all parameters in a more
formal way
(like the python3 type annotation) and we assume that only categorical
variables
change the semantics of a model, then we could also try all combinations.
That would probably not be a unit test, though.

IMHO having this test is useful, even if it only covers the default argument case (perfection being the enemy of good or some such).

LinearRegressionCV uses StratifiedKFold without passing on random_state. ElasticNetCV (and friends) use KFold without shuffling (-> deterministic). Does someone remember the reasoning behind this choice? Can we change either check_cv to pass through a random_state or LinearRegressionCV to use KFold (without shuffling)?

It's pretty weird that Regression uses stratified CV...? And certainly random_state should be passed on. I'd be interested to see if checking pickle equality gives the same results though.

LinearRegressionCV uses stratified? That's a bug.
Not using shuffling by default was as choice made to show users if they have correlations in their data.

I think several comments here are red herrings.

Note that for CV estimators (ElasticNetCV, ...), when no cv parameter is given, then the random state is not passed to the cross-validation split, which may lead to non-deterministic behaviors.

Our default cv (KFold, StratifiedKFold) are not randomised by default.

Do we want to continue on with this? Marking as Stalled.

Do we want to continue on with this?

@jnothman: I am interested to carry on if this is relevant.