@jnothman Is this PR in the right direction? I am not sure if this is what you meant by using 'auto'.

@jnothman Really Sorry, I forget that I had an open PR here, all because of my semester examinations. I somehow feel that the search option in github's open PR does not give full list :P

https://github.com/scikit-learn/scikit-learn/pulls/thechargedneutron should help.

You're getting an error in CI:

/home/travis/build/scikit-learn/scikit-learn/sklearn/linear_model/tests/test_logistic.py:245: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
/home/travis/build/scikit-learn/scikit-learn/sklearn/linear_model/logistic.py:1231: in fit
    self.dual)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
solver = 'liblinear', multi_class = 'auto', penalty = 'l2', dual = False
    def _check_solver_option(solver, multi_class, penalty, dual):
        if solver not in ['liblinear', 'newton-cg', 'lbfgs', 'sag', 'saga']:
            raise ValueError("Logistic Regression supports only liblinear, "
                             "newton-cg, lbfgs, sag and saga solvers, got %s"
                             % solver)
    
        if multi_class not in ['multinomial', 'ovr']:
            raise ValueError("multi_class should be either multinomial or "
>                            "ovr, got %s" % multi_class)
E           ValueError: multi_class should be either multinomial or ovr, got auto

@jnothman Do you think we need to convert auto into the original parameter in the fit? If that's the case, _check_solver_option fails otherwise. So, do I need to copy paste the same thing there also or there's some better way to do it?

@jnothman I guess this is a better implementation and indeed passes tests (except probably a doctest failure).

Pulled the latest version to resolve conflicts.

test_logistic_regression_warnings and test_logistic_regression_cv_warnings could also be merged into one test to avoid duplication, using a loop or with pytest:

@pytest.mark.parametrize('model', [LogisticRegression, LogisticRegressionCV])
def test_logistic_regression_warnings(model):
    ...

Same for test_logistic_regression_auto and test_logistic_regression_cv_auto

@TomDLT Parametrize added.

Can you refactor it, for instance putting this part in _check_solver_option?

That would require me to have a return parameter in _check_solver_option which returns the solver according to the penalty. I think the current implementation is better than returning all those from the check function. I would change if you think returning is a better option.

I think the current implementation is better than returning all those from the check function.

Why?

But if you prefer you can also merge them into a new function. I just find it redundant and prone to future mistakes.

I'm marking this as stalled / help wanted. But maybe we need a core dev to take it on, as it really is bad that we have ovr and intercept regularisation as default.

Do we really want L-BFGS as default or SAG or depend on the data to decide? I think if we change the default we should change to a solver that's automatic, as we did for PCA, and that is picked according to the penalty.

So can someone write down the decision tree of settings we're working towards? SAG and L-BFGS should at least reach the same coefficients given reasonable amounts of data?

I think this should be added as a priority for 0.20.

Another question related to this: should LogisticRegressionCV(solver='liblinear') be optimising intercept_scaling as well as C?

@TomDLT, given your previous work in this space, would you mind working on defining appropriate defaults? If fit_intercept=False and (the data is binary or multiclass='ovr'), should we still be defaulting to liblinear??

I also wonder if we can say that for something named 'auto' we give no strong assurances of API stability across versions: we make a weak assurance that with sufficient (i.e. infinite) max_iter, 'auto' should result in convergence to the same solution (but not necessarily the same n_iter_) across versions.

I'll do it

Hm SAG is pretty unstable if you don't scale the data. We could always pre-scale the data but that seems a bit un-scikit-learn.

@TomDLT are you still on this? Otherwise @NicolasHug will take it up. Also: are you at scipy?

So do we just want to change the default of solver as a first step or do we want to do an automagic "auto"? It's really not clear to me how auto would really work, since good values for tol and max_iter and dual depend on the solver, right? If someone sets "dual=True" does that mean they'll always get liblinear? Or do we ignore that unless they set the solver to liblinear?

On the one hand it would be annoying to make a backward-incompatible change twice, on the other hand, getting "auto" right seems very hairy and probably not doable for 0.20, even without benchmarks.

If we want to run benchmarks, we would need to benchmark at least these cases, right?
Tall data (small n_features, large n_samples)
wide data (small n_samples, large n_features)
sparse wide data
multiclass = ovr or multinomial or binary
fit_intercept = True/False

That would be 18 possible regimes already. Well maybe the decision tree for the optimum solver doesn't involve all combinations.

SAG and L-BFGS should at least reach the same coefficients given reasonable amounts of data?

If the data is scaled badly that's probably not gonna happen, I think?