-
-
Notifications
You must be signed in to change notification settings - Fork 25.9k
[MRG] Matern kernel #3885
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] Matern kernel #3885
Conversation
sklearn/metrics/pairwise.py
Outdated
|
||
K = euclidean_distances(X, Y, squared=False) | ||
if coef0 == 0.5: | ||
K = np.exp(-K * gamma) |
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.
This can be done more efficiently, see e.g. the rbf kernel:
K *= -gamma
np.exp(K, K) # exponentiate K in-place
2e643ce
to
3f156bb
Compare
Support for arbitrary values of coef0 is added; it is not very efficient, though, because it requires the evaluation of the modified Bessel function of the second kind (scipy.special.kv).According to Rasmussen & Williams (page 85), there are rarely use-cases where coef0 not in [0.5, 1.5, 2.5, inf] would make sense. Maybe we should warn? |
f35506d
to
f967e95
Compare
@afabisch the issues have now been adressed. what is your opinion? |
is compared on a (discontinuous) step-function: | ||
* The Matern kernel for coef0==1.5, learning a once differentiable function | ||
* The Matern kernel for coef0==2.5, learning a twice differentiable function | ||
* The Matern kernel for coef0==3.5, learning a three-rimes differentiable |
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.
three-rimes?
Thanks for your feedback @afabisch; the issues have been adressed. |
You should rebase on master. I don't know what we should do in case of the expensive evaluation (coef0 != 0.5, 1.5, 2.5, inf). We should at least warn the user in the docstring. We could also add a warning in the code. btw. we don't check for nu <= 0.0. In this case we should raise a |
f0bb42d
to
86680c5
Compare
I rebased on master, updated the doc of coef0, and added a ValueError for coef <= 0 |
@@ -415,6 +416,22 @@ def test_rbf_kernel(): | |||
assert_array_almost_equal(K.flat[::6], np.ones(5)) | |||
|
|||
|
|||
def test_matern_kernel(): |
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.
maybe add an absolute exponential kernel test?
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.
Actually at least all if branches need testing.
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.
All if branches are now tested (line 437ff). The absolute exponential test (line 430ff) is a bit self-referential; an alternative would be to test against the absolute_exponential in gaussian_process.correlation_models.
Maybe a stupid question, but what it this meant to be used for? SVMs? Is that practical? |
I believe the main use-case of the Matern kernel is in combination with Gaussian processes. Combined with GPs, the Matern kernel (nu=1.5 or 2.5) can often improve considerably over the RBF-kernel and absolute exponential kernel (see example in PR #3388), since it has weaker assumptions on the smoothness of the function than RBF and is less jumpy than the absolute exponential kernel. That being said, it can also be beneficial with other kernel-methods. I've used it together with kernel_approximation.Nystroem where it also often outperformed the RBF-kernel. Once this PR is merged, I would like to work on refactoring correlation_models.py such that it uses as much from pairwise.py as possible (in particular the Matern kernel). I'm not doing that directly in this PR in order to keep this PR small and focused. |
2568459
to
6e25a90
Compare
Ok, fair enough. It does seem to me that the matern kernel is mostly used in GPs. |
Although it is usually used in GPs, I think we should integrate it in the library. It seems to be useful in combination with e.g. kernel ridge regression and seems to be a basic kernel even though it might not be used as often as the RBF kernel. I would +1 for merge. |
Yes, would be nice if we could merge this PR. @amueller What do you think? |
Could you squash and rebase please? |
And yeah, we can merge it (after review). |
Have you checked that the docs are rendered properly? |
We want to release pretty soon so we are focusing on bug-fixes a bit currently, and I'm a bit busy the next two weeks. I'll try to review but I can't promise anything, sorry. After the release, I'll certainly have the time. |
REFACTOR Performance improvements in matern_kernel based on @afabisch's suggestions EXAMPLE Comparing Matern kernels of different smoothness on a step-function. ENH Support for arbitrary values of coef0 ("nu") in the Matern kernel EXAMPLE Added illustration of Matern kernel for different values of coef0
DOC Extended documentation of Matern kernel DOC Documentation of parameter coef0 of matern_kernel
MISC Using np.finfo(float).eps instead of 1e-10 in Matern kernel Furthmore, fixed typo and increased readability of test_matern_kernel. TEST Extended tests of Matern kernel
6e25a90
to
02711c0
Compare
Done. The documentation is rendered properly for me. |
|
||
from sklearn.metrics.pairwise import matern_kernel | ||
|
||
import matplotlib.pyplot as plt |
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.
matplotlib import should be before sklearn
that's it for me. |
02711c0
to
fa582b4
Compare
Thanks for the review @agramfort . Your comments should be addressed. I changed the format to "X: array, shape (n_samples_X, n_features)" etc. in the whole pairwise module. |
sklearn/metrics/pairwise.py
Outdated
else: # general case; expensive to evaluate | ||
K[K == 0.0] += np.finfo(float).eps # strict zeros would result in nan | ||
tmp = (math.sqrt(2 * coef0) * gamma * K) | ||
K[:] = (2 ** (1. - coef0)) / scipy.special.gamma(coef0) |
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.
I would have used K.fill
besides LGTM
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.
done
DOC Add "matern" as valid kernel name in various places in the doc MISC Revising Matern kernel PR based on @agramfort's review REFACTOR Using K.fill(X) instead of K[:] = X
fa582b4
to
340b402
Compare
was the intention here mostly to use it with the GP and this is replaced by #4270 or do we still want this in pairwise? |
Since we already have the Matern kernel in GPR and I did not find any issue requesting the kernel for another usage, I am closing this PR. |
Add Matern kernel to the kernels in pairwise.py.
The Matern kernel (https://en.wikipedia.org/wiki/Mat%C3%A9rn_covariance_function and Rasmussen and Williams 2006, pp84) is a generalization of the RBF and the absolute exponential kernel with an additional hyperparemeter nu, which allows interpolating between these two (RBF: nu=inf, absolute exponential: nu=0.5). In contrast to the RBF kernel, it makes less strict assumptions on the smoothness of the function to be learned. This is shown in an example for a step-function for different values of nu:
TODOs: