Skip to content
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

[MRG+2] Add parameter to linear_kernel for dense_output #10999

Merged
merged 6 commits into from Apr 25, 2018
Copy path View file
@@ -722,7 +722,7 @@ def paired_distances(X, Y, metric="euclidean", **kwds):


# Kernels
def linear_kernel(X, Y=None):
def linear_kernel(X, Y=None, dense_output=True):
"""
Compute the linear kernel between X and Y.
@@ -734,12 +734,19 @@ def linear_kernel(X, Y=None):
Y : array of shape (n_samples_2, n_features)
dense_output : boolean (optional), default True
Whether to return dense output even when the input is sparse. If
``False``, the output is sparse if both input arrays are sparse.
.. versionadded:: 0.20
parameter ``dense_output`` for dense output.

This comment has been minimized.

Copy link
@rth

rth Apr 21, 2018

Member

Please remove this line; versionadded + the version alone is sufficient.

Returns
-------
Gram matrix : array of shape (n_samples_1, n_samples_2)
"""
X, Y = check_pairwise_arrays(X, Y)
return safe_sparse_dot(X, Y.T, dense_output=True)
return safe_sparse_dot(X, Y.T, dense_output=dense_output)


def polynomial_kernel(X, Y=None, degree=3, gamma=None, coef0=1):
@@ -540,6 +540,18 @@ def test_linear_kernel():
assert_array_almost_equal(K.flat[::6], [linalg.norm(x) ** 2 for x in X])


def test_linear_kernel_sparse_output():

This comment has been minimized.

Copy link
@rth

rth Apr 21, 2018

Member

This is very redundant with test_cosine_similarity_sparse_output.
Let's just make this latter test a parametric test with pytest, that takes as input the similarity function (and the metric name), to check both for cosine similarity and the linear kernel. Also possibly rename that test to something more generic.

This comment has been minimized.

Copy link
@tgsmith61591

tgsmith61591 Apr 23, 2018

Author Contributor

No problem. I fixed this in the latest push.

This comment has been minimized.

Copy link
@rth

rth Apr 23, 2018

Member

No what I meant is to define a single function and make it parametric with pytest,

import pytest

@pytest.mark.parametrized('name, pairwise_func',
                          [('linear_kernel', linear_kernel),
                           ('cosine_similarity', cosine_similarity)])
def test_pairwise_similarity_sparse_output(name, pairwise_func):
    # adapt the previous version of test_cosine_similarity_sparse_output here

see https://docs.pytest.org/en/latest/parametrize.html#pytest-mark-parametrize-parametrizing-test-functions

rng = np.random.RandomState(0)
X = rng.random_sample((5, 4))
X_sparse = csr_matrix(X)
K = linear_kernel(X_sparse, X_sparse, dense_output=False)
assert_true(issparse(K)) # output will be sparse

# assert the sparse array is equal elementally to the dense array
K_dense = linear_kernel(X, X, dense_output=True)
assert_array_almost_equal(K.toarray(), K_dense)


def test_rbf_kernel():
rng = np.random.RandomState(0)
X = rng.random_sample((5, 4))
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.