-
-
Notifications
You must be signed in to change notification settings - Fork 25.8k
WIP: Refactor euclidean_distance, add optional argument for preallocated output array. #483
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Explicit is better than implicit!
---------- | ||
X : ndarray, float32, shape = [n_samples_a, n_features] | ||
|
||
Y : ndarray, float32, shape = [n_sample_b, n_features] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo: n_sampleS_b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, fixed.
It seems that under certain circumstances, using BLAS might be faster, but loses precision in certain cases. @jakevdp has done some benchmarking. In an ideal world, these Cython functions should try and guess whether the BLAS call is worth it and do it. |
Code to bench (create a '.ipy' file and run it in IPython): import timeit from sklearn.metrics.pairwise import euclidean_distances import numpy as np np.random.seed(0) for n_features in (10, 100, 1000): for n_samples in (10, 100, 1000): Y = np.random.random(size=(n_features, n_samples)) X = np.random.random(size=(n_features, n_samples)) print 'n_features: % 2i, n_samples % 2i' % (n_features, n_samples) %timeit euclidean_distances(X, Y) |
another implementation I found in milk: https://github.com/luispedro/milk/blob/master/milk/unsupervised/pdist.py |
This pull request adds
euclidean_distance
.euclidean_distance
is operating on floating point arrays (general consensus in the sprint room is that it doesn't usually make much sense in the integer case and if you really want that, you can cast).out
argument foreuclidean_distance
(only in the dense-dense case; raises an error otherwise).