TST: Added test to check ConvergenceWarning

MechCoder · ogrisel · commit ad5773cfca6b · 2014-08-01T12:15:57.000+02:00
diff --git a/sklearn/linear_model/logistic.py b/sklearn/linear_model/logistic.py
@@ -639,9 +639,9 @@ class LogisticRegression(BaseLibLinear, LinearClassifierMixin,
         Intercept (a.k.a. bias) added to the decision function.
         If `fit_intercept` is set to False, the intercept is set to zero.
 
-    `n_iter_` : int | array, shape (n_classes,)
-        Number of iterations run per class. Valid only for the liblinear
-        solver.
+    `n_iter_` : int
+        Maximum of the actual number of iterations across all classes.
+        Valid only for the liblinear solver.
 
     See also
     --------
diff --git a/sklearn/linear_model/tests/test_logistic.py b/sklearn/linear_model/tests/test_logistic.py
@@ -9,8 +9,10 @@
 from sklearn.utils.testing import assert_greater
 from sklearn.utils.testing import assert_raises
 from sklearn.utils.testing import assert_true
+from sklearn.utils.testing import assert_warns
 from sklearn.utils.testing import raises
 from sklearn.utils.testing import ignore_warnings
+from sklearn.utils import ConvergenceWarning
 
 from sklearn.linear_model.logistic import (
     LogisticRegression,
@@ -422,3 +424,12 @@ def test_logistic_regressioncv_class_weights():
                                    class_weight='auto')
     clf_lib.fit(X, y)
     assert_array_almost_equal(clf_lib.coef_, clf_lbf.coef_, decimal=4)
+
+
+def test_logistic_regression_convergence_warnings():
+    """Test that warnings are raised if model does not converge"""
+
+    X, y = make_classification(n_samples=20, n_features=20)
+    clf_lib = LogisticRegression(solver='liblinear', max_iter=2)
+    assert_warns(ConvergenceWarning, clf_lib.fit, X, y)
+    assert_equal(clf_lib.n_iter_, 2)
diff --git a/sklearn/svm/base.py b/sklearn/svm/base.py
@@ -716,7 +716,7 @@ def fit(self, X, y):
 
             # LibLinear wants targets as doubles, even for classification
             y_ind = np.asarray(y_ind, dtype=np.float64).ravel()
-            raw_coef_, self.n_iter_  = liblinear.train_wrap(
+            raw_coef_, n_iter_  = liblinear.train_wrap(
                 X, y_ind, sp.isspmatrix(X), self._get_solver_type(),
                 self.tol, self._get_bias(), self.C, self.class_weight_,
                 self.max_iter, rnd.randint(np.iinfo('i').max)
@@ -725,12 +725,10 @@ def fit(self, X, y):
             # seed for srand in range [0..INT_MAX); due to limitations in Numpy
             # on 32-bit platforms, we can't get to the UINT_MAX limit that
             # srand supports
-            for n_iter in self.n_iter_:
-                if n_iter >= self.max_iter:
-                    warnings.warn("Liblinear failed to converge, increase "
-                                  "the number of iterations.", ConvergenceWarning)
-            if len(self.classes_) == 2:
-                self.n_iter_ = self.n_iter_[0]
+            self.n_iter_ = max(n_iter_)
+            if self.n_iter_ >= self.max_iter:
+                warnings.warn("Liblinear failed to converge, increase "
+                              "the number of iterations.", ConvergenceWarning)
 
             if self.fit_intercept:
                 self.coef_ = raw_coef_[:, :-1]
diff --git a/sklearn/svm/liblinear.c b/sklearn/svm/liblinear.c
diff --git a/sklearn/svm/liblinear.pyx b/sklearn/svm/liblinear.pyx
@@ -37,7 +37,7 @@ def train_wrap(X, np.ndarray[np.float64_t,   ndim=1, mode='c'] Y,
                 bias)
 
     cdef np.ndarray[np.int32_t, ndim=1, mode='c'] \
-        class_weight_label = np.arange(class_weight.shape[0], dtype=np.int32)
+        class_weight_label = np.arange(class_weight.shape[0], dtype=np.intc)
     param = set_parameter(solver_type, eps, C, class_weight.shape[0],
                           class_weight_label.data, class_weight.data,
                           max_iter, random_seed)
@@ -59,7 +59,7 @@ def train_wrap(X, np.ndarray[np.float64_t,   ndim=1, mode='c'] Y,
     cdef int labels_ = nr_class
     if nr_class == 2:
         labels_ = 1
-    cdef np.ndarray[np.int32_t, ndim=1, mode='c'] n_iter = np.zeros(labels_, dtype=np.int32)
+    cdef np.ndarray[np.int32_t, ndim=1, mode='c'] n_iter = np.zeros(labels_, dtype=np.intc)
     get_n_iter(model, <int *>n_iter.data)
 
     cdef int nr_feature = get_nr_feature(model)
diff --git a/sklearn/svm/src/liblinear/linear.cpp b/sklearn/svm/src/liblinear/linear.cpp
@@ -15,6 +15,7 @@
    - Remove the hard-coded value of max_iter (1000), that allows max_iter
      to be passed as a parameter from the classes LogisticRegression and
      LinearSVC, Manoj Kumar
+   - Added function get_n_iter that exposes the number of iterations.
         See issue 3499: https://github.com/scikit-learn/scikit-learn/issues/3499
         See pull 3501: https://github.com/scikit-learn/scikit-learn/pull/3501
    
@@ -2809,11 +2810,12 @@ void get_labels(const model *model_, int* label)
 
 void get_n_iter(const model *model_, int* n_iter)
 {
-	int labels;
-	labels = model_->nr_class;
-	if (labels == 2)
-		labels = 1;
-	if (model_->n_iter != NULL)
+    int labels;
+    labels = model_->nr_class;
+    if (labels == 2)
+        labels = 1;
+
+    if (model_->n_iter != NULL)
         for(int i=0;i<labels;i++)
             n_iter[i] = model_->n_iter[i];
 }
diff --git a/sklearn/svm/tests/test_svm.py b/sklearn/svm/tests/test_svm.py
@@ -674,6 +674,14 @@ def test_consistent_proba():
     assert_array_almost_equal(proba_1, proba_2)
 
 
+def test_linear_svc_convergence_warnings():
+    """Test that warnings are raised if model does not converge"""
+
+    lsvc = svm.LinearSVC(max_iter=2)
+    assert_warns(ConvergenceWarning, lsvc.fit, X, Y)
+    assert_equal(lsvc.n_iter_, 2)
+
+
 if __name__ == '__main__':
     import nose
     nose.runmodule()
diff --git a/sklearn/tests/test_common.py b/sklearn/tests/test_common.py
@@ -343,6 +343,6 @@ def test_transformer_n_iter():
         # Dependent on external solvers and hence accessing the iter
         # param is non-trivial.
         external_solver = ['Isomap', 'KernelPCA', 'LocallyLinearEmbedding',
-                           'RandomizedLasso','LogisticRegressionCV']
+                           'RandomizedLasso', 'LogisticRegressionCV']
         if hasattr(estimator, "max_iter") and name not in external_solver:
             yield check_transformer_n_iter, name, estimator
