[MRG +1] Support metric='precomputed' in nearest neighbors #4090

jnothman · 2015-01-12T23:21:39Z

This fixes up #2532 (sorry to not offer the changes back there, @robertlayton: the rebase was too messy with my changes to validation in sklearn.metrics.pairwise) to handle queries that differ from the index data. This case is a little awkward in that the index data doesn't matter, but really should be supported for KNNClassifier et al.)

This also provides stronger validation to precomputed metrics in `pairwise_{distances,kernels}

amueller · 2015-01-13T23:33:14Z

I think the KNeighborsClassifier then needs a _pairwise property to work with GridSearchCV and cross_val_score, like here

jnothman · 2015-01-14T00:17:42Z

Good point.

amueller · 2015-01-14T00:18:24Z

(Also the regressor and possible other classes I didn't look at the details yet)

jnothman · 2015-01-14T00:21:46Z

(Also the regressor and possible other classes I didn't look at the details yet)

Of course; anything that can fit over a precomputed metric.

jnothman · 2015-01-18T12:00:24Z

@amueller I've added _pairwise and a test

coveralls · 2015-01-18T12:14:40Z

Coverage increased (+0.01%) when pulling 3e22ac1 on jnothman:pr2532 into d9ecf46 on scikit-learn:master.

amueller · 2015-01-20T17:08:15Z

sklearn/metrics/pairwise.py

@@ -919,7 +931,9 @@ def chi2_kernel(X, Y=None, gamma=1.):
    'euclidean': euclidean_distances,
    'l2': euclidean_distances,
    'l1': manhattan_distances,
-    'manhattan': manhattan_distances, }
+    'manhattan': manhattan_distances,
+    'precomputed': None,  # HACK: precomputed is always allowed, never called


Why did this become necessary here?

Never mind, got it. For some reason github thinks this is part of chi2_kernel.

amueller · 2015-01-20T17:13:20Z

I wonder if we can have a good test that ensures whether we forgot to add _pairwise.
We could add a pipeline test on estimator basis, that would at least ensure that it is not broken in the future.

amueller · 2015-01-20T17:18:10Z

sklearn/metrics/pairwise.py

@@ -1092,6 +1106,7 @@ def pairwise_distances(X, Y=None, metric="euclidean", n_jobs=1, **kwds):
                         "callable" % (metric, _VALID_METRICS))

    if metric == "precomputed":


metric= "precomputed" is not documented in the function, right?

amueller · 2015-01-20T17:23:11Z

apart from my minor comments LGTM.

coveralls · 2015-01-21T08:32:27Z

Coverage increased (+0.01%) when pulling 178ccb6 on jnothman:pr2532 into d9ecf46 on scikit-learn:master.

jnothman · 2015-03-30T13:02:06Z

rebased and awaiting final review.

amueller · 2015-04-29T00:51:45Z

travis is unhappy. Btw in the initial comment you said something about X=None which we now support in master...

jnothman · 2015-06-03T09:42:19Z

Thanks for both those pointers (a while ago), @amueller. I hope this is fixed now.

jnothman · 2015-06-03T09:42:41Z

So again, someone else's review is welcome.

amueller · 2015-06-03T19:18:25Z

travis is still failing.

jnothman · 2015-06-03T22:56:44Z

boo!

Also, handle radius_neighbors case identically to kneighbors for precomputed

GaelVaroquaux · 2015-08-30T12:25:36Z

sklearn/neighbors/tests/test_neighbors.py

@@ -100,6 +102,75 @@ def test_unsupervised_inputs():
        assert_array_almost_equal(ind1, ind2)


+def test_precomputed():
+    """Tests unsupervised NearestNeighbors with a distance matrix."""
+    # Note: smaller samples may result in spurious test success


We need to use a random number generator created in this test, to avoid having side effects across tests.

Support metric='precomputed' in nearest neighbors [rebased version of #4090]

GaelVaroquaux · 2015-08-30T15:15:05Z

Merged via #5188

jnothman mentioned this pull request Jan 12, 2015

MRG: Support for distance matrices in NearestNeighbor #2532

Closed

jnothman force-pushed the pr2532 branch 2 times, most recently from c85c0f3 to 3e22ac1 Compare January 18, 2015 12:04

amueller reviewed Jan 20, 2015
View reviewed changes

amueller changed the title ~~[MRG] Support metric='precomputed' in nearest neighbors~~ [MRG +1] Support metric='precomputed' in nearest neighbors Jan 20, 2015

jnothman force-pushed the pr2532 branch from 178ccb6 to d26730c Compare March 30, 2015 13:02

jnothman mentioned this pull request Apr 28, 2015

ValueError in dbscan_inner in case of precomputed metric #4641

Closed

robertlayton and others added 3 commits June 4, 2015 18:54

ENH precomputed is now a valid metric for 'brute'

f871916

ENH precomputed is now a valid metric for 'auto' neighbors

7f67874

Also, handle radius_neighbors case identically to kneighbors for precomputed

ENH support precomputed neighbors where query != index

0b5ac95

jnothman added 5 commits June 4, 2015 18:54

TST precomputed matrix validation in correct place

8685564

FIX add _pairwise and test to NeighborsBase

ad53dd8

DOC fix up parameter descriptions

4807b10

FIX broken rebase

3354957

TST add test for precomputed metric and X=None at predict

79345d1

jnothman mentioned this pull request Jun 4, 2015

[MRG + 1] Fix DBSCAN fit with precomputed matrix in edge cases #4650

Merged

FIX test with invalid input; simplify dbscan precomputed

b1dea89

jnothman force-pushed the pr2532 branch from 202215e to b1dea89 Compare June 4, 2015 09:05

GaelVaroquaux reviewed Aug 30, 2015
View reviewed changes

GaelVaroquaux mentioned this pull request Aug 30, 2015

Support metric='precomputed' in nearest neighbors [rebased version of #4090] #5188

Merged

GaelVaroquaux added a commit that referenced this pull request Aug 30, 2015

Merge pull request #5188 from GaelVaroquaux/pr_4090

ab8f155

Support metric='precomputed' in nearest neighbors [rebased version of #4090]

GaelVaroquaux closed this Aug 30, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MRG +1] Support metric='precomputed' in nearest neighbors #4090

[MRG +1] Support metric='precomputed' in nearest neighbors #4090

jnothman commented Jan 12, 2015

amueller commented Jan 13, 2015

jnothman commented Jan 14, 2015

amueller commented Jan 14, 2015

jnothman commented Jan 14, 2015

jnothman commented Jan 18, 2015

coveralls commented Jan 18, 2015

amueller Jan 20, 2015

amueller Jan 20, 2015

amueller commented Jan 20, 2015

amueller Jan 20, 2015

amueller commented Jan 20, 2015

coveralls commented Jan 21, 2015

jnothman commented Mar 30, 2015

amueller commented Apr 29, 2015

jnothman commented Jun 3, 2015

jnothman commented Jun 3, 2015

amueller commented Jun 3, 2015

jnothman commented Jun 3, 2015

GaelVaroquaux Aug 30, 2015

GaelVaroquaux commented Aug 30, 2015

		@@ -1092,6 +1106,7 @@ def pairwise_distances(X, Y=None, metric="euclidean", n_jobs=1, **kwds):
		"callable" % (metric, _VALID_METRICS))

		if metric == "precomputed":

[MRG +1] Support metric='precomputed' in nearest neighbors #4090

[MRG +1] Support metric='precomputed' in nearest neighbors #4090

Conversation

jnothman commented Jan 12, 2015

amueller commented Jan 13, 2015

jnothman commented Jan 14, 2015

amueller commented Jan 14, 2015

jnothman commented Jan 14, 2015

jnothman commented Jan 18, 2015

coveralls commented Jan 18, 2015

amueller Jan 20, 2015

Choose a reason for hiding this comment

amueller Jan 20, 2015

Choose a reason for hiding this comment

amueller commented Jan 20, 2015

amueller Jan 20, 2015

Choose a reason for hiding this comment

amueller commented Jan 20, 2015

coveralls commented Jan 21, 2015

jnothman commented Mar 30, 2015

amueller commented Apr 29, 2015

jnothman commented Jun 3, 2015

jnothman commented Jun 3, 2015

amueller commented Jun 3, 2015

jnothman commented Jun 3, 2015

GaelVaroquaux Aug 30, 2015

Choose a reason for hiding this comment

GaelVaroquaux commented Aug 30, 2015