Repository: scikit-learn/scikit-learn
Pull request: [DOC] Comments clarifying the effect of the random_state parameter
Fixes #14587 

The use of the random_state parameter might be confusing since it applies to the solver but not ot the the cross-validation generator. Comments have been added to make it clearer.




Problem statement:
random_state in LogisticRegressionCV does not seem to work
Hi all,

While trying to do logistic regression with cross validation (LogisticRegressionCV), I noticed that changing the random_state does not seem to have any impact on the results. On the other hand, shuffling the data set using pandas sample function does lead to small differences as expected.

#### Code example
```python
import pandas as pd
from sklearn.datasets import load_iris
from sklearn.linear_model import LogisticRegressionCV

data = load_iris()
data_df = pd.DataFrame(data['data'], columns=data['feature_names'])
versicolor_series = pd.Series(data['target']).apply(lambda x: 1 if x==1 else 0)
data_df['versicolor'] = versicolor_series

results_df = pd.DataFrame()

for index in range(0,20):
    #data_df = data_df.sample(frac=1) <= uncomment this line
    
    X_df = data_df.copy().drop(['versicolor'], axis = 1)
    y = data_df['versicolor'].values
    
    clf = LogisticRegressionCV(
        cv=5, 
        random_state=index,
        max_iter=10000,
        tol=1e-7,
        Cs=np.logspace(-4.0, 4.0, num=21)
    ).fit(X_df.values, y)
    
    results_df.loc[index, 'score'] = clf.score(X_df.values, y)
    coefs = pd.DataFrame(clf.coef_[0], columns=['coef'])
    selected_coefs = coefs.loc[coefs['coef'] != 0]
    results_df.loc[index, 'var_count'] = len(selected_coefs.index)
    results_df.loc[index, 'weight'] = 1/clf.C_

results_df
```

#### Expected Results
Some variations as in the `sample(frac=1)` case.

#### Actual Results
No variation at all.

#### Versions
MacOS 10.14.6
Python 3.6.4
Jupyter 1.0.0
pandas 0.24.2
numpy 1.14.0
scikit-learn 0.21.2

Hints from issue discussion:
The default CV is StratifiedKFold (5) which is not shuffled
Hi Joel,

Ok, thanks for the clarification.

IMHO, it would be nice to state something in the documentation for the `random_state` parameter, something like:

> Note that this parameter has by default no impact on the results since the default cross validation method (StratifiedKFold) does not by default shuffle the data set.
I'm confused by @jnothman's comment. That ``random_state`` controls the ``random_state`` of the solver, not of ``CV``. I think the docs could clarify that, though?
Yes, I meant it has no impact on cv, and impact on the fit depends on
solver and data. No impact whatsoever with solver=lbfgs iirc

@amueller, thanks for your reply.

I agree, it's kind of confusing, at least I was confused about it, so few sentences clarifying it in the docs would be great.
Please submit a pull request making the documentation for random_state more
explicitly describe what it affects.

Commit message: [DOC] Comments clarifying the effect of the random_state parameter (#14587)

Patch to review:
diff --git a/sklearn/linear_model/logistic.py b/sklearn/linear_model/logistic.py
index 432a5a7db3c0d..8e9c3b4b0f832 100644
--- a/sklearn/linear_model/logistic.py
+++ b/sklearn/linear_model/logistic.py
@@ -1814,7 +1814,10 @@ class LogisticRegressionCV(LogisticRegression, BaseEstimator,
         If int, random_state is the seed used by the random number generator;
         If RandomState instance, random_state is the random number generator;
         If None, the random number generator is the RandomState instance used
-        by `np.random`.
+        by `np.random`. Note that this only applies to the solver and not the
+        cross-validation generator. Futhermore, if a deterministic solver is
+        chosen, changing this value will have no impact on the outcome.
+
 
     l1_ratios : list of float or None, optional (default=None)
         The list of Elastic-Net mixing parameter, with ``0 <= l1_ratio <= 1``.


Code review comments:
diff_hunk: @@ -1814,7 +1814,10 @@ class LogisticRegressionCV(LogisticRegression, BaseEstimator,
         If int, random_state is the seed used by the random number generator;
         If RandomState instance, random_state is the random number generator;
         If None, the random number generator is the RandomState instance used
-        by `np.random`.
+        by `np.random`. Note that this only applies to the solver and not the
+        cross-validation generator. Futhermore, if a deterministic solver is
path: sklearn/linear_model/logistic.py
text: @user1:
Remove the sentence after starting by `Furthermore`. It is better just to mentioned the solver for which the `random_state` is used.