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] Allow usage of nu=inf in Matern kernel #15503

Closed
wants to merge 5 commits into from

Conversation

sam-dixon
Copy link
Contributor

Reference Issues/PRs

Fixes #12031

What does this implement/fix? Explain your changes.

If the nu parameter is set to inf, the kernel is equivalent to the RBF kernel, as specified in the documentation.

Tests

import numpy as np 
from sklearn.gaussian_process.kernels import Matern, RBF 
a = np.random.randn(3)[:, np.newaxis]
 
mat_kernel = Matern(nu=np.inf) 
mat_k, mat_grad = mat_kernel(a, eval_gradient=True)

rbf_kernel = RBF() 
rbf_k, rbf_grad = rbf_kernel(a, eval_gradient=True)

print(np.all(rbf_k == mat_k), np.all(rbf_grad == mat_grad))

Returns True, True

Any other comments?

@eickenberg
Copy link
Contributor

eickenberg commented Nov 2, 2019

This contribution looks good to me.

Looking at the referenced issue, it doesn't seem like a decision was reached on whether this should be implemented, so it might be a good idea to ping them to finish the discussion.
(I think this is a great solution.)

@jnothman
Copy link
Member

jnothman commented Nov 3, 2019 via email

Copy link
Member

@rth rth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @sam-dixon !

Please add unit tests for this feature under sklearn/gaussian_process/tests/test_kernels.py in particular,

  • that with nu=inf the output is equal to the RBF kernel
  • that for large nu asymptotically this kernel yields approximately the same result as nu=inf

Also please add an entry to the change log at doc/whats_new/v0.22.rst. Like the other entries there, please reference this pull request with :pr: and credit yourself (and other contributors if applicable) with :user:.

@@ -259,6 +263,11 @@ def test_matern_kernel():
K1 = Matern(nu=nu, length_scale=1.0)(X)
K2 = Matern(nu=nu + tiny, length_scale=1.0)(X)
assert_array_almost_equal(K1, K2)
# test that coef0==large is close to RBF
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If so you might also want to check that they are not too close!

Copy link
Member

@jnothman jnothman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise lgtm

@cmarmo
Copy link
Contributor

cmarmo commented Nov 22, 2019

@sam-dixon do you think you could find some time to sync to upstream and address the comments? Thanks!

@sam-dixon sam-dixon requested a review from rth November 23, 2019 22:11
@rth rth closed this in #15972 Dec 26, 2019
@sam-dixon sam-dixon deleted the matern-inf-nu branch January 6, 2020 00:58
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.

Usage of inf in 'nu' param of Matern kernel
5 participants