Skip to content

[MRG+1] allow callable kernels in cross-validation #8005

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

Merged
merged 4 commits into from
Dec 20, 2016

Conversation

amueller
Copy link
Member

@amueller amueller commented Dec 7, 2016

This allows callable kernels in cross-validation.
This was forbidden in #649 but I don't understand why.
I kept it in #803 when I introduced _pairwise for some reason.
Discussion in #7930 where @jnothman and me got kinda confused I think.

Removing the condition in metaestimators.py does not break any tests.

I checked all other occurrences of _pairwise and it only returns True for "precomputed", never for callable.s

@jnothman
Copy link
Member

jnothman commented Dec 7, 2016

This seems more sensible... but I'd like to hear whether there are reasons other than computational cost that others can find for this restriction existing in the first place.

@jnothman
Copy link
Member

jnothman commented Dec 7, 2016

In short, LGTM

@amueller
Copy link
Member Author

amueller commented Dec 7, 2016

weird I didn't get the error locally... must have not run the test properly..

@amueller
Copy link
Member Author

amueller commented Dec 7, 2016

I have no idea why I put _safe_split into utils.meta_estimator, but I don't really have a better idea now.

@jnothman
Copy link
Member

jnothman commented Dec 8, 2016 via email

@jnothman
Copy link
Member

jnothman commented Dec 8, 2016 via email

@jnothman
Copy link
Member

(I wonder whether kallable kernels were outlawed at some point when joblib didn't check for picklability and things crashed??)

@jnothman jnothman changed the title [MRG] allow callable kernels in cross-validation [MRG+1] allow callable kernels in cross-validation Dec 14, 2016
@amueller
Copy link
Member Author

for reference,

from sklearn.model_selection import cross_val_score
from sklearn.svm import SVC
from sklearn.datasets import make_blobs
import numpy as np

X, y = make_blobs()
cross_val_score(SVC(kernel=lambda x, y: np.inner(x, y)), X, y)

works, n_jobs=2 crashes with a pickling error - unsurprisingly.

@tguillemot
Copy link
Contributor

Thanks for the exemple @amueller.
LGTM

@tguillemot
Copy link
Contributor

Maybe we can add something about that in what's new ?

@amueller
Copy link
Member Author

this is good for merge, right?

@jnothman jnothman merged commit 5d0c7f5 into scikit-learn:master Dec 20, 2016
sergeyf pushed a commit to sergeyf/scikit-learn that referenced this pull request Feb 28, 2017
@Przemo10 Przemo10 mentioned this pull request Mar 17, 2017
Sundrique pushed a commit to Sundrique/scikit-learn that referenced this pull request Jun 14, 2017
NelleV pushed a commit to NelleV/scikit-learn that referenced this pull request Aug 11, 2017
paulha pushed a commit to paulha/scikit-learn that referenced this pull request Aug 19, 2017
maskani-moh pushed a commit to maskani-moh/scikit-learn that referenced this pull request Nov 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants