[MRG+1] Fix optics metric issues (DOC and precomputed) by adrinjalali · Pull Request #12028 · scikit-learn/scikit-learn

adrinjalali · 2018-09-06T15:16:55Z

Fixes #12009
See #11982

Still doesn't support the output of radius_neighbors_graph as input though.

Question: what's the criterion to include one's name in the authors section in the files? Are those only the original authors or do I count as an author in the optics_.py and test_optics.py now?

adrinjalali · 2018-09-06T15:50:59Z

I'm not sure about this travis error, is it because of something I did?

/home/travis/build/scikit-learn/scikit-learn/testvenv/local/lib/python2.7/site-packages/py/_path/common.py:377: in visit
    for x in Visitor(fil, rec, ignore, bf, sort).gen(self):
/home/travis/build/scikit-learn/scikit-learn/testvenv/local/lib/python2.7/site-packages/py/_path/common.py:419: in gen
    if p.check(dir=1) and (rec is None or rec(p))])
/home/travis/build/scikit-learn/scikit-learn/testvenv/local/lib/python2.7/site-packages/_pytest/main.py:561: in _recurse
    ihook = self.gethookproxy(path)
/home/travis/build/scikit-learn/scikit-learn/testvenv/local/lib/python2.7/site-packages/_pytest/main.py:416: in gethookproxy
    my_conftestmodules = pm._getconftestmodules(fspath)
/home/travis/build/scikit-learn/scikit-learn/testvenv/local/lib/python2.7/site-packages/_pytest/config/__init__.py:395: in _getconftestmodules
    mod = self._importconftest(conftestpath)
/home/travis/build/scikit-learn/scikit-learn/testvenv/local/lib/python2.7/site-packages/_pytest/config/__init__.py:431: in _importconftest
    raise ConftestImportFailure(conftestpath, sys.exc_info())
E   ConftestImportFailure: ModuleDeprecationWarning('The oldnumeric module will be dropped in Numpy 1.9',)
E     File "/home/travis/build/scikit-learn/scikit-learn/sklearn/__init__.py", line 64, in <module>
E       from .base import clone
E     File "/home/travis/build/scikit-learn/scikit-learn/sklearn/base.py", line 11, in <module>
E       from scipy import sparse
E     File "/usr/lib/python2.7/dist-packages/scipy/__init__.py", line 77, in <module>
E       from numpy import oldnumeric
E     File "/usr/lib/python2.7/dist-packages/numpy/oldnumeric/__init__.py", line 11, in <module>
E       warnings.warn(_msg, ModuleDeprecationWarning)

jnothman · 2018-09-06T21:37:53Z

We are pretty informal about what should deserve author credit. Maybe we should have some written policy. I've certainly touched a lot more files than credit me as an author, but when a contributor adds their name to the list it's not like I'm going to argue; and when we split modules, we are not meticulous about splitting the author list.

jnothman

We should accept a sparse X as well, but that may be a separate pr.

Can you test metrics supported by neighbors and by metrics.pairwise and all their combinations. I'm a bit worried about us mixing them

adrinjalali · 2018-09-07T13:52:22Z

I just checked the metrics supported by NearestNeighbors and pairwise_distances, and they're the same. They're just documented differently. There's just the matching distance which is an alias for hamming in scipy, which is not included in the docstring of NearestNeighbors, but is supported (it is included in pairwise_distances's docstring).

I also tried running OPTICS with all those metrics, and as long as the NearestNeighbors algorithm is set to auto, they all work.

adrinjalali · 2018-09-07T15:41:38Z

@jnothman, regarding the sparse X, it seems it's the NearestNeighbors.kneighbors(...) which is the one not supporting a sparse input. The following code throws an exception:

from sklearn.neighbors import (radius_neighbors_graph,
                               NearestNeighbors)
import numpy as np

rng = np.random.RandomState(0)
n_points_per_cluster = 50

C1 = [-5, -2] + .8 * rng.randn(n_points_per_cluster, 2)
C2 = [4, -1] + .1 * rng.randn(n_points_per_cluster, 2)
C3 = [1, -2] + .2 * rng.randn(n_points_per_cluster, 2)
C4 = [-2, 3] + .3 * rng.randn(n_points_per_cluster, 2)
C5 = [3, -2] + 1.6 * rng.randn(n_points_per_cluster, 2)
C6 = [5, 6] + 2 * rng.randn(n_points_per_cluster, 2)
X = np.vstack((C1, C2, C3, C4, C5, C6))

dists = radius_neighbors_graph(X, radius=10)

nn = NearestNeighbors(metric='precomputed', algorithm='auto').fit(dists)
res = nn.kneighbors(dists, 10)

Which throws:

AxisError: axis 1 is out of bounds for array of dimension 1

jnothman · 2018-09-08T10:23:32Z

Not sparse precomputed, just sparse feature matrix with, say, Euclidean distance

adrinjalali · 2018-09-15T11:43:44Z

I guess we can handle the sparse input in a separate PR, should we merge this?

qinhanmin2014

LGTM, thanks @adrinjalali

huntzhan · 2019-08-22T08:03:59Z

Hi team, thanks for your great work! I was wondering if there's any plan to support sparse input for OPTICS recently?

jnothman · 2019-08-22T08:29:04Z

Sparse input meaning sparse precomputed distance graph?

huntzhan · 2019-08-22T08:35:51Z

Meaning the csr_matrix of X support in OPTICS.fit(self, X, y=None)

scikit-learn/sklearn/cluster/dbscan_.py

Lines 330 to 355 in 68044b0

    
               def fit(self, X, y=None, sample_weight=None): 
        
                   """Perform DBSCAN clustering from features, or distance matrix. 
        
                   Parameters 
        
                   ---------- 
        
                   X : array-like or sparse matrix, shape (n_samples, n_features), or \ 
        
                       (n_samples, n_samples) 
        
                       Training instances to cluster, or distances between instances if 
        
                       ``metric='precomputed'``. If a sparse matrix is provided, it will 
        
                       be converted into a sparse ``csr_matrix``. 
        
                   sample_weight : array, shape (n_samples,), optional 
        
                       Weight of each sample, such that a sample with a weight of at least 
        
                       ``min_samples`` is by itself a core sample; a sample with a 
        
                       negative weight may inhibit its eps-neighbor from being core. 
        
                       Note that weights are absolute, and default to 1. 
        
                   y : Ignored 
        
                       Not used, present here for API consistency by convention. 
        
                   Returns 
        
                   ------- 
        
                   self 
        
                   """ 
        
                   X = check_array(X, accept_sparse='csr')

jnothman · 2019-08-22T08:50:26Z

It might be very easy to support. Try adding accept_sparse and test it out.

adrinjalali added 3 commits September 6, 2018 16:52

Support precomputed distance matrix

b32d70f

sync docstrings

414be06

remove sparse

7df42ff

adrinjalali changed the title ~~Fix optics metic issues (DOC and precomputed)~~ Fix optics metric issues (DOC and precomputed) Sep 6, 2018

jnothman reviewed Sep 6, 2018

View reviewed changes

jnothman approved these changes Sep 9, 2018

View reviewed changes

jnothman added this to the 0.20 milestone Sep 9, 2018

adrinjalali added 4 commits September 9, 2018 12:22

Merge branch 'master' into optics/metric

23bc0ac

put back reading from precomputed matrix

b1437c0

simplify test

5b510ad

revert to float

56cba77

adrinjalali mentioned this pull request Sep 10, 2018

OPTICS detecting the wrong outlier #11677

Closed

adrinjalali changed the title ~~Fix optics metric issues (DOC and precomputed)~~ [MRG+1] Fix optics metric issues (DOC and precomputed) Sep 10, 2018

adrinjalali mentioned this pull request Sep 12, 2018

Remove OPTICS from 0.20 #12053

Merged

qinhanmin2014 approved these changes Sep 17, 2018

View reviewed changes

qinhanmin2014 merged commit efe7b8c into scikit-learn:master Sep 17, 2018

adrinjalali deleted the optics/metric branch September 17, 2018 07:37

adrinjalali mentioned this pull request Mar 18, 2019

[MRG+1] OPTICS fit uses the selected extract_method parameter #12087

Merged

huntzhan mentioned this pull request Aug 23, 2019

[MRG] Make OPTICS support sparse input #14736

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[MRG+1] Fix optics metric issues (DOC and precomputed)#12028

[MRG+1] Fix optics metric issues (DOC and precomputed)#12028
qinhanmin2014 merged 7 commits intoscikit-learn:masterfrom
adrinjalali:optics/metric

adrinjalali commented Sep 6, 2018

Uh oh!

adrinjalali commented Sep 6, 2018

Uh oh!

jnothman commented Sep 6, 2018 via email

Uh oh!

jnothman left a comment

Uh oh!

adrinjalali commented Sep 7, 2018

Uh oh!

adrinjalali commented Sep 7, 2018

Uh oh!

jnothman commented Sep 8, 2018 via email

Uh oh!

adrinjalali commented Sep 15, 2018

Uh oh!

qinhanmin2014 left a comment

Uh oh!

huntzhan commented Aug 22, 2019

Uh oh!

jnothman commented Aug 22, 2019 via email

Uh oh!

huntzhan commented Aug 22, 2019

Uh oh!

jnothman commented Aug 22, 2019 via email

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

adrinjalali commented Sep 6, 2018

Uh oh!

adrinjalali commented Sep 6, 2018

Uh oh!

jnothman commented Sep 6, 2018 via email

Uh oh!

jnothman left a comment

Choose a reason for hiding this comment

Uh oh!

adrinjalali commented Sep 7, 2018

Uh oh!

adrinjalali commented Sep 7, 2018

Uh oh!

jnothman commented Sep 8, 2018 via email

Uh oh!

adrinjalali commented Sep 15, 2018

Uh oh!

qinhanmin2014 left a comment

Choose a reason for hiding this comment

Uh oh!

huntzhan commented Aug 22, 2019

Uh oh!

jnothman commented Aug 22, 2019 via email

Uh oh!

huntzhan commented Aug 22, 2019

Uh oh!

jnothman commented Aug 22, 2019 via email

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants