From a92de21f7ade1e0830a35b7767dcf0ed2b90cc8a Mon Sep 17 00:00:00 2001 From: Maximiliano Vargas <43217761+mvargas33@users.noreply.github.com> Date: Wed, 15 Sep 2021 19:15:42 +0200 Subject: [PATCH 1/7] Remove 3.9 from compatibility --- .github/workflows/main.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 46e5d2c9..c32755ff 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -41,7 +41,7 @@ jobs: strategy: matrix: os: [ubuntu-latest] - python-version: ['3.6', '3.7', '3.8', '3.9'] + python-version: ['3.6', '3.7', '3.8'] steps: - uses: actions/checkout@v2 - name: Set up Python From a02cbd5be778fc59fc25e7f6a2fc08962baedb53 Mon Sep 17 00:00:00 2001 From: mvargas33 Date: Fri, 1 Oct 2021 14:31:31 +0200 Subject: [PATCH 2/7] Fix Triplets predict function. Made a test to show the point. --- metric_learn/base_metric.py | 4 ++- test/test_triplets_classifiers.py | 48 ++++++++++++++++++++++++++++++- 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/metric_learn/base_metric.py b/metric_learn/base_metric.py index 721d7ba0..323c994a 100644 --- a/metric_learn/base_metric.py +++ b/metric_learn/base_metric.py @@ -602,7 +602,9 @@ def predict(self, triplets): prediction : `numpy.ndarray` of floats, shape=(n_constraints,) Predictions of the ordering of pairs, for each triplet. """ - return np.sign(self.decision_function(triplets)) + return np.array([-1 if (t <= 0) else 1 for t in + self.decision_function(triplets)]) + #return np.sign(self.decision_function(triplets)) def decision_function(self, triplets): """Predicts differences between sample distances in input triplets. diff --git a/test/test_triplets_classifiers.py b/test/test_triplets_classifiers.py index 0f0bf7df..f17f9ef0 100644 --- a/test/test_triplets_classifiers.py +++ b/test/test_triplets_classifiers.py @@ -6,7 +6,7 @@ from metric_learn.sklearn_shims import set_random_state from sklearn import clone import numpy as np - +from numpy.testing import assert_array_equal @pytest.mark.parametrize('with_preprocessor', [True, False]) @pytest.mark.parametrize('estimator, build_dataset', triplets_learners, @@ -63,3 +63,49 @@ def test_accuracy_toy_example(estimator, build_dataset): # we force the transformation to be identity so that we control what it does estimator.components_ = np.eye(X.shape[1]) assert estimator.score(triplets_test) == 0.25 + + +@pytest.mark.parametrize('estimator, build_dataset', triplets_learners, + ids=ids_triplets_learners) +def test_no_zero_prediction(estimator, build_dataset): + """ + Test that all predicted values are in {-1, 1}, even when the + distance d(x,y) and d(x,z) is the same for a triplet of the + form (x, y, z). + """ + # Dummy fit + triplets, _, _, X = build_dataset(with_preprocessor=False) + # Force 3 dimentions only, to use cross product and get easy orthogonal vectors. + triplets = np.array([ [t[0][:3], t[1][:3], t[2][:3]] for t in triplets]) + X = np.array([x[:3] for x in X]) + # Dummy fit + estimator = clone(estimator) + set_random_state(estimator) + estimator.fit(triplets) + # we force the transformation to be identity, to force euclidean distance + estimator.components_ = np.eye(X.shape[1]) + + # Get two orthogonal vectors in respect to X[1] + k = X[1]/np.linalg.norm(X[1]) # Normalize first vector + x = X[2] - X[2].dot(k) * k # Get random orthogonal vector + x /= np.linalg.norm(x) # Normalize + y = np.cross(k, x) # Get orthogonal vector to x + # Assert these orthogonal vectors are different + with pytest.raises(AssertionError): + assert_array_equal(X[1], x) + with pytest.raises(AssertionError): + assert_array_equal(X[1], y) + # Assert the distance is the same for both + assert estimator.get_metric()(X[1], x) == estimator.get_metric()(X[1], y) + + # Form the three scenarios where predict() gives 0 with numpy.sign + triplets_test = np.array( # Critical examples + [[X[0], X[2], X[2]], + [X[1], X[1], X[1]], + [X[1], x, y] + ]) + # Predict + predictions = estimator.predict(triplets_test) + # Count non -1 or 1 values + not_valid = [e for e in predictions if e not in [-1, 1]] + assert len(not_valid) == 0 \ No newline at end of file From efc4d5dc7bfba31d04dffb84300a4ca96e7a5a61 Mon Sep 17 00:00:00 2001 From: mvargas33 Date: Fri, 1 Oct 2021 14:39:41 +0200 Subject: [PATCH 3/7] Fix identation --- metric_learn/base_metric.py | 4 ++-- test/test_triplets_classifiers.py | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/metric_learn/base_metric.py b/metric_learn/base_metric.py index 323c994a..3fee41c5 100644 --- a/metric_learn/base_metric.py +++ b/metric_learn/base_metric.py @@ -603,8 +603,8 @@ def predict(self, triplets): Predictions of the ordering of pairs, for each triplet. """ return np.array([-1 if (t <= 0) else 1 for t in - self.decision_function(triplets)]) - #return np.sign(self.decision_function(triplets)) + self.decision_function(triplets)]) + # return np.sign(self.decision_function(triplets)) def decision_function(self, triplets): """Predicts differences between sample distances in input triplets. diff --git a/test/test_triplets_classifiers.py b/test/test_triplets_classifiers.py index f17f9ef0..9a215b52 100644 --- a/test/test_triplets_classifiers.py +++ b/test/test_triplets_classifiers.py @@ -8,6 +8,7 @@ import numpy as np from numpy.testing import assert_array_equal + @pytest.mark.parametrize('with_preprocessor', [True, False]) @pytest.mark.parametrize('estimator, build_dataset', triplets_learners, ids=ids_triplets_learners) @@ -75,8 +76,8 @@ def test_no_zero_prediction(estimator, build_dataset): """ # Dummy fit triplets, _, _, X = build_dataset(with_preprocessor=False) - # Force 3 dimentions only, to use cross product and get easy orthogonal vectors. - triplets = np.array([ [t[0][:3], t[1][:3], t[2][:3]] for t in triplets]) + # Force 3 dimentions only, to use cross product and get easy orthogonal vec. + triplets = np.array([[t[0][:3], t[1][:3], t[2][:3]] for t in triplets]) X = np.array([x[:3] for x in X]) # Dummy fit estimator = clone(estimator) @@ -102,10 +103,9 @@ def test_no_zero_prediction(estimator, build_dataset): triplets_test = np.array( # Critical examples [[X[0], X[2], X[2]], [X[1], X[1], X[1]], - [X[1], x, y] - ]) + [X[1], x, y]]) # Predict predictions = estimator.predict(triplets_test) # Count non -1 or 1 values not_valid = [e for e in predictions if e not in [-1, 1]] - assert len(not_valid) == 0 \ No newline at end of file + assert len(not_valid) == 0 From 15899aba92a92dcb9c1cd9992fc338bdbcfb3fab Mon Sep 17 00:00:00 2001 From: mvargas33 Date: Mon, 4 Oct 2021 10:38:24 +0200 Subject: [PATCH 4/7] Simplified prediction as suggested --- metric_learn/base_metric.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/metric_learn/base_metric.py b/metric_learn/base_metric.py index 3fee41c5..21506011 100644 --- a/metric_learn/base_metric.py +++ b/metric_learn/base_metric.py @@ -602,9 +602,7 @@ def predict(self, triplets): prediction : `numpy.ndarray` of floats, shape=(n_constraints,) Predictions of the ordering of pairs, for each triplet. """ - return np.array([-1 if (t <= 0) else 1 for t in - self.decision_function(triplets)]) - # return np.sign(self.decision_function(triplets)) + return 2 * (self.decision_function(triplets) > 0) - 1 def decision_function(self, triplets): """Predicts differences between sample distances in input triplets. From 5c59aca11e30d9a5e924cca2366b5d4df8e157a3 Mon Sep 17 00:00:00 2001 From: mvargas33 Date: Fri, 8 Oct 2021 10:54:35 +0200 Subject: [PATCH 5/7] Resolved code review comments --- test/test_triplets_classifiers.py | 89 +++++++++++++++---------------- 1 file changed, 44 insertions(+), 45 deletions(-) diff --git a/test/test_triplets_classifiers.py b/test/test_triplets_classifiers.py index 9a215b52..535e004e 100644 --- a/test/test_triplets_classifiers.py +++ b/test/test_triplets_classifiers.py @@ -27,6 +27,50 @@ def test_predict_only_one_or_minus_one(estimator, build_dataset, assert len(not_valid) == 0 +@pytest.mark.parametrize('estimator, build_dataset', triplets_learners, + ids=ids_triplets_learners) +def test_no_zero_prediction(estimator, build_dataset): + """ + Test that all predicted values are not zero, even when the + distance d(x,y) and d(x,z) is the same for a triplet of the + form (x, y, z). i.e border cases. + """ + triplets, _, _, X = build_dataset(with_preprocessor=False) + # Force 3 dimentions only, to use cross product and get easy orthogonal vec. + triplets = np.array([[t[0][:3], t[1][:3], t[2][:3]] for t in triplets]) + X = X[:, :3] + # Dummy fit + estimator = clone(estimator) + set_random_state(estimator) + estimator.fit(triplets) + # We force the transformation to be identity, to force euclidean distance + estimator.components_ = np.eye(X.shape[1]) + + # Get two orthogonal vectors in respect to X[1] + k = X[1] / np.linalg.norm(X[1]) # Normalize first vector + x = X[2] - X[2].dot(k) * k # Get random orthogonal vector + x /= np.linalg.norm(x) # Normalize + y = np.cross(k, x) # Get orthogonal vector to x + # Assert these orthogonal vectors are different + with pytest.raises(AssertionError): + assert_array_equal(X[1], x) + with pytest.raises(AssertionError): + assert_array_equal(X[1], y) + # Assert the distance is the same for both + assert estimator.get_metric()(X[1], x) == estimator.get_metric()(X[1], y) + + # Form the three scenarios where predict() gives 0 with numpy.sign + triplets_test = np.array( # Critical examples + [[X[0], X[2], X[2]], + [X[1], X[1], X[1]], + [X[1], x, y]]) + # Predict + predictions = estimator.predict(triplets_test) + # Check there are no zero values + not_valid = [e for e in predictions if e == 0] + assert len(not_valid) == 0 + + @pytest.mark.parametrize('with_preprocessor', [True, False]) @pytest.mark.parametrize('estimator, build_dataset', triplets_learners, ids=ids_triplets_learners) @@ -64,48 +108,3 @@ def test_accuracy_toy_example(estimator, build_dataset): # we force the transformation to be identity so that we control what it does estimator.components_ = np.eye(X.shape[1]) assert estimator.score(triplets_test) == 0.25 - - -@pytest.mark.parametrize('estimator, build_dataset', triplets_learners, - ids=ids_triplets_learners) -def test_no_zero_prediction(estimator, build_dataset): - """ - Test that all predicted values are in {-1, 1}, even when the - distance d(x,y) and d(x,z) is the same for a triplet of the - form (x, y, z). - """ - # Dummy fit - triplets, _, _, X = build_dataset(with_preprocessor=False) - # Force 3 dimentions only, to use cross product and get easy orthogonal vec. - triplets = np.array([[t[0][:3], t[1][:3], t[2][:3]] for t in triplets]) - X = np.array([x[:3] for x in X]) - # Dummy fit - estimator = clone(estimator) - set_random_state(estimator) - estimator.fit(triplets) - # we force the transformation to be identity, to force euclidean distance - estimator.components_ = np.eye(X.shape[1]) - - # Get two orthogonal vectors in respect to X[1] - k = X[1]/np.linalg.norm(X[1]) # Normalize first vector - x = X[2] - X[2].dot(k) * k # Get random orthogonal vector - x /= np.linalg.norm(x) # Normalize - y = np.cross(k, x) # Get orthogonal vector to x - # Assert these orthogonal vectors are different - with pytest.raises(AssertionError): - assert_array_equal(X[1], x) - with pytest.raises(AssertionError): - assert_array_equal(X[1], y) - # Assert the distance is the same for both - assert estimator.get_metric()(X[1], x) == estimator.get_metric()(X[1], y) - - # Form the three scenarios where predict() gives 0 with numpy.sign - triplets_test = np.array( # Critical examples - [[X[0], X[2], X[2]], - [X[1], X[1], X[1]], - [X[1], x, y]]) - # Predict - predictions = estimator.predict(triplets_test) - # Count non -1 or 1 values - not_valid = [e for e in predictions if e not in [-1, 1]] - assert len(not_valid) == 0 From 16b0302aa4d8d55691c1f75cf39f450f83531d29 Mon Sep 17 00:00:00 2001 From: mvargas33 Date: Fri, 8 Oct 2021 11:29:27 +0200 Subject: [PATCH 6/7] Fix weird commit --- .github/workflows/main.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index c32755ff..46e5d2c9 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -41,7 +41,7 @@ jobs: strategy: matrix: os: [ubuntu-latest] - python-version: ['3.6', '3.7', '3.8'] + python-version: ['3.6', '3.7', '3.8', '3.9'] steps: - uses: actions/checkout@v2 - name: Set up Python From 1d3ce9e5e9a01b822fe89d8190d2433930977d27 Mon Sep 17 00:00:00 2001 From: mvargas33 Date: Mon, 11 Oct 2021 11:36:30 +0200 Subject: [PATCH 7/7] Simplified assertion --- test/test_triplets_classifiers.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/test_triplets_classifiers.py b/test/test_triplets_classifiers.py index 535e004e..f2d5c015 100644 --- a/test/test_triplets_classifiers.py +++ b/test/test_triplets_classifiers.py @@ -67,8 +67,7 @@ def test_no_zero_prediction(estimator, build_dataset): # Predict predictions = estimator.predict(triplets_test) # Check there are no zero values - not_valid = [e for e in predictions if e == 0] - assert len(not_valid) == 0 + assert np.sum(predictions == 0) == 0 @pytest.mark.parametrize('with_preprocessor', [True, False])