[MRG] FIX Fix LMNN rollback

metric_learn/lmnn.py

@@ -95,12 +95,16 @@ def fit(self, X, y):
     L = self.L_
     objective = np.inf
 
+    # we initialize the roll back
+    L_old = L.copy()
+    G_old = G.copy()
+    df_old = df.copy()
+    a1_old = [a.copy() for a in a1]
+    a2_old = [a.copy() for a in a2]
+    objective_old = objective
+
     # main loop
     for it in xrange(1, self.max_iter):
-      df_old = df.copy()
-      a1_old = [a.copy() for a in a1]
-      a2_old = [a.copy() for a in a2]
-      objective_old = objective
       # Compute pairwise distances under current metric
       Lx = L.dot(self.X_.T).T
       g0 = _inplace_paired_L2(*Lx[impostors])
@@ -158,14 +162,25 @@ def fit(self, X, y):
       if delta_obj > 0:
         # we're getting worse... roll back!
         learn_rate /= 2.0
+        L = L_old
+        G = G_old
         df = df_old
         a1 = a1_old
         a2 = a2_old
         objective = objective_old
       else:
-        # update L
-        L -= learn_rate * 2 * L.dot(G)
-        learn_rate *= 1.01
+        # We did good. We store this point as reference in case we do
+        # worse next time.
+        objective_old = objective
+        L_old = L.copy()
+        G_old = G.copy()
+        df_old = df.copy()
+        a1_old = [a.copy() for a in a1]
+        a2_old = [a.copy() for a in a2]
+      # we update L and will see in the next iteration if it does indeed
+      # better
+      L -= learn_rate * 2 * L.dot(G)
+      learn_rate *= 1.01
 
       # check for convergence
       if it > self.min_iter and abs(delta_obj) < self.convergence_tol:
@@ -177,7 +192,7 @@ def fit(self, X, y):
         print("LMNN didn't converge in %d steps." % self.max_iter)
 
     # store the last L
-    self.L_ = L
+    self.L_ = L_old
     self.n_iter_ = it
     return self
 

test/metric_learn_test.py

@@ -1,8 +1,11 @@
 import unittest
+import re
+import sys
 import numpy as np
 from six.moves import xrange
+from sklearn.externals.six import StringIO
 from sklearn.metrics import pairwise_distances
-from sklearn.datasets import load_iris
+from sklearn.datasets import load_iris, make_classification
 from numpy.testing import assert_array_almost_equal
 
 from metric_learn import (
@@ -70,6 +73,40 @@ def test_iris(self):
       csep = class_separation(lmnn.transform(), self.iris_labels)
       self.assertLess(csep, 0.25)
 
+  def test_convergence_simple_example(self):
+    # LMNN should converge on this simple example, which it did not with
+    # this issue: https://github.com/metric-learn/metric-learn/issues/88
+    X, y = make_classification(random_state=0)
+    old_stdout = sys.stdout
+    sys.stdout = StringIO()
+    lmnn = LMNN(verbose=True)
+    try:
+        lmnn.fit(X, y)
+    finally:
+        out = sys.stdout.getvalue()
+        sys.stdout.close()
+        sys.stdout = old_stdout
+    assert ("LMNN converged with objective" in out)
+
+  def test_no_twice_same_objective(self):
+    # test that the objective function never has twice the same value
+    # see https://github.com/metric-learn/metric-learn/issues/88
+    X, y = make_classification(random_state=0)
+    old_stdout = sys.stdout
+    sys.stdout = StringIO()
+    lmnn = LMNN(verbose=True)
+    try:
+      lmnn.fit(X, y)
+    finally:
+      out = sys.stdout.getvalue()
+      sys.stdout.close()
+      sys.stdout = old_stdout
+    lines = re.split("\n+", out)
+    objectives = [re.search("\d* (?:(\d*.\d*))[ | -]\d*.\d*", s)
+                  for s in lines]
+    objectives = [match.group(1) for match in objectives if match is not None]
+    assert len(objectives) == len(set(objectives))
+
 
 class TestSDML(MetricTestCase):
   def test_iris(self):