[MRG] Arbitrary SVC kernels by georgipeev · Pull Request #11296 · scikit-learn/scikit-learn

georgipeev · 2018-06-15T22:17:39Z

Reference Issues/PRs

What does this implement/fix? Explain your changes.

Removed non-applicable input validation when a custom kernel is used. Allowed specifying X as a list when using custom kernels.

Any other comments?

WIP until I add a meaningful unit test aside from the current one that merely checks that no errors arise when using custom kernels.

jnothman · 2018-06-17T22:20:11Z

And yet it seems you already have test failures :( Let us know if you need help

agramfort · 2020-01-29T10:03:57Z

sklearn/svm/tests/test_svm.py

    kfunc = lambda x, y: np.dot(x, y.T)
    clf = svm.SVC(kernel=kfunc)
-    clf.fit(X, Y)
+    clf.fit(np.array(X), Y)


basically here it's an API change. Before for a custom kernel we would convert X to array. I propose to delegate the input checking to kernel function.

agramfort · 2020-01-29T10:04:11Z

sklearn/svm/tests/test_svm.py

    a = svm.SVC(kernel=lambda x, y: np.dot(x, y.T), probability=True,
                random_state=0, max_iter=1)
-    assert_warns(ConvergenceWarning, a.fit, X, Y)
+    assert_warns(ConvergenceWarning, a.fit, np.array(X), Y)


same here. Due to API change.

glemaitre · 2020-01-30T09:43:49Z

sklearn/svm/tests/test_svm.py

+    assert svc1.score(data, y) == svc3.score(K, y)
+    assert svc1.score(data, y) == svc2.score(X, y)
+    if hasattr(svc1, 'decision_function'):  # classifier
+        assert_array_almost_equal(svc1.decision_function(data),


We should use assert_allclose instead of assert_array_almost_equal

glemaitre · 2020-01-30T09:46:00Z

sklearn/svm/_base.py

                                 "the number of samples at training time" %
                                 (X.shape[1], self.shape_fit_[0]))
-        elif n_features != self.shape_fit_[1]:
+        elif not callable(self.kernel) and n_features != self.shape_fit_[1]:


If n_features is only used here, let's be explicit:

Suggested change

elif not callable(self.kernel) and n_features != self.shape_fit_[1]:

elif not callable(self.kernel) and X.shape[1] != self.shape_fit_[1]:

glemaitre · 2020-01-30T09:46:11Z

sklearn/svm/_base.py

+        elif not callable(self.kernel) and n_features != self.shape_fit_[1]:
            raise ValueError("X.shape[1] = %d should be equal to %d, "
                             "the number of features at training time" %
                             (n_features, self.shape_fit_[1]))


Suggested change

(n_features, self.shape_fit_[1]))

(X.shape[1], self.shape_fit_[1]))

glemaitre · 2020-01-30T09:46:27Z

sklearn/svm/_base.py

                "cannot use sparse input in %r trained on dense data"
                % type(self).__name__)
-        n_samples, n_features = X.shape
+        if not callable(self.kernel):


I would remove this assignment then

glemaitre · 2020-01-30T09:48:23Z

sklearn/svm/_base.py

                             % (sample_weight.shape, X.shape))

-        if isinstance(self.gamma, str):
+        kernel = self.kernel


Suggested change

kernel = self.kernel

kernel = 'precomputed' if callable(self.kernel) else self.kernel

sklearn/svm/_base.py

glemaitre · 2020-01-30T09:53:15Z

doc/whats_new/v0.23.rst

+  kernels in :class:`svm.SVC` and :class:`svm.SVR`.
+  :pr:`11296` by `Alexandre Gramfort`_ and  :user:`Georgi Peev <georgipeev>`.
+
+- |API| Do not enforce float entries in X when using custom kernel


We could add this change in Change of behaviour. I think this is fine since it could be considered as bug fix but better to document it as well.

glemaitre

LGTM

jnothman

Sorry I thought I posted this yesterday. Hopefully not out of date

jnothman · 2020-01-29T21:55:08Z

doc/whats_new/v0.23.rst

+  kernels in :class:`svm.SVC` and :class:`svm.SVR`.
+  :pr:`11296` by `Alexandre Gramfort`_ and  :user:`Georgi Peev <georgipeev>`.
+
+- |API| Do not enforce float entries in X when using custom kernel


I understand "API" to mean "there's a new way to do something I could do before". I'm not sure it quite fits here

jnothman · 2020-01-29T21:55:20Z

doc/whats_new/v0.23.rst

  `probB_`, are now deprecated as they were not useful. :pr:`15558` by
  `Thomas Fan`_.

+- |Fix| Fix use of custom kernel not taking float entries such as string


This could be regarded as an enhancement

well it was support to work but it was not tested. I prefer to see this as a bug fix.

jnothman · 2020-01-29T21:56:32Z

sklearn/svm/_base.py

-                         order='C', accept_sparse='csr',
-                         accept_large_sparse=False)
+        if callable(self.kernel):
+            check_consistent_length(X, y)


Now do we have a backward incompatibility since before a kernel would have been passed a validated float array?

I'm okay with that level of incompatibility though perhaps it deserves a note in the change log

this is mentioned in section Changed models

agramfort · 2020-02-01T11:29:48Z

@jnothman feel free to merge. I tried to address your comments.

jnothman · 2020-02-01T21:36:43Z

Thanks @georgipeev

amueller added the Stalled label Aug 5, 2019

agramfort force-pushed the arbitrary-SVC-kernels branch from 8e5bf21 to f324b1f Compare January 29, 2020 10:02

agramfort reviewed Jan 29, 2020

View reviewed changes

agramfort changed the title ~~[WIP] Arbitrary SVC kernels~~ [MRG] Arbitrary SVC kernels Jan 29, 2020

rth added the module:svm label Jan 29, 2020

glemaitre reviewed Jan 30, 2020

View reviewed changes

glemaitre approved these changes Jan 30, 2020

View reviewed changes

jnothman reviewed Jan 30, 2020

View reviewed changes

Georgi Peev and others added 8 commits January 31, 2020 20:50

Removed non-applicable checks on X when using user-defined kernels

3e2fefe

fix/improve tests

4012362

fix long lines

3234adc

more test

021de81

pep8

f1f018f

update what's new + more test

adaff25

review glemaitre

20b02bf

cleanup what's new

638b989

agramfort force-pushed the arbitrary-SVC-kernels branch from e649ab3 to 638b989 Compare January 31, 2020 19:50

Update v0.23.rst

1e9d494

jnothman approved these changes Feb 1, 2020

View reviewed changes

jnothman merged commit 84bc8d3 into scikit-learn:master Feb 1, 2020

thomasjpfan pushed a commit to thomasjpfan/scikit-learn that referenced this pull request Feb 22, 2020

FIX Arbitrary SVC kernels (scikit-learn#11296)

d5e1032

panpiort8 pushed a commit to panpiort8/scikit-learn that referenced this pull request Mar 3, 2020

FIX Arbitrary SVC kernels (scikit-learn#11296)

dae8f3d

	elif not callable(self.kernel) and n_features != self.shape_fit_[1]:
	elif not callable(self.kernel) and X.shape[1] != self.shape_fit_[1]:

	(n_features, self.shape_fit_[1]))
	(X.shape[1], self.shape_fit_[1]))

	kernel = self.kernel
	kernel = 'precomputed' if callable(self.kernel) else self.kernel

Uh oh!

Conversation

georgipeev commented Jun 15, 2018

Reference Issues/PRs

What does this implement/fix? Explain your changes.

Any other comments?

Uh oh!

jnothman commented Jun 17, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

glemaitre left a comment

Choose a reason for hiding this comment

Uh oh!

jnothman left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

agramfort commented Feb 1, 2020

Uh oh!

jnothman commented Feb 1, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants