Feature request: Implement periodic covariance function

[olspert]

Summary:

Implement a periodic covariance function.
Periodic covariance function is defined as:
k(x, x')=sigma^2*exp(-2*sin(pi*|x-x'|/p)^2/l^2)),

where sigma^2 is the signal variance,p is the period and l is the lengthscale.

Description:

Create a new header file stan/math/prim/mat/fun/cov_per.hpp containing two functions named cov_per. Use code from stan/math/prim/mat/fun/cov_exp_quad.hpp as a template.

Create a new header file stan/math/rev/mat/fun/cov_per.hpp containing class named cov_per_vari and two functions named cov_per. Use code from stan/math/rev/mat/fun/cov_exp_quad.hpp as a template.

Write unit tests.

Current Version:

v2.17.0

[betanalpha]

Please see https://github.com/stan-dev/stan/wiki/Adding-a-Gaussian-Process-Covariance-Function <https://github.com/stan-dev/stan/wiki/Adding-a-Gaussian-Process-Covariance-Function> for what we are requesting for the addition of new covariance functions. cov_exp_quad was a prototypical implementation and hence is still technically incomplete.

…

On Oct 26, 2017, at 10:09 AM, olspert ***@***.***> wrote: Summary: Implement a periodic covariance function. Periodic covariance function is defined as: k(x, x')=sigma^2*exp(-2*sin(pi*|x-x'|/p)^2/l^2)), where sigma^2 is the signal variance,p is the period and l is the lengthscale. Description: Create a new header file stan/math/prim/mat/fun/cov_per.hpp containing two functions named cov_per. Use code from stan/math/prim/mat/fun/cov_exp_quad.hpp as a template. Create a new header file stan/math/rev/mat/fun/cov_per.hpp containing class named cov_per_vari and two functions named cov_per. Use code from stan/math/rev/mat/fun/cov_exp_quad.hpp as a template. Write unit tests. Current Version: v2.17.0 — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#660>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABdNlhk30kOeA5XhJYr56QT_iDZPL7SGks5swJKtgaJpZM4QHott>.

[bob-carpenter]

Is that meant to be a minimal set of requirements for a pull request? Is cov_exp_quad not useful because it's incomplete?

[olspert]

Maybe this issue should be put on hold for a while until one example of a covariance function meeting the given requirements has been written by somebody. Periodic covariance function probably wouldn't be the best candidate for this task. I have though implemented all the functionality on the same level as it was for cov_exp_quad, but I guess there is no need to check this implementation in to the repository at this moment.

[betanalpha]

Yes, minimal. The lpdf and Cholesky don’t have to be well implemented — for example the latter can just be a lightweight function that calls the covariance matrix function and then returns its Cholesky. If the covariance matrix is structured then these implementations can be much more sophisticated to take advantage of that structure, but they don’t have to especially on the first iteration of implementation. Right now using even cov_exp_quad is a bit ungainly because you have to repeat the steps of creating the matrix and then plugging it into a multivariate normal (for the centered parameterization) or taking its Cholesky (for the non-centered parameterization). Having all three functions makes it much more streamlined to implement the corresponding GP and requiring that all covariance functions support all three functions makes it uniform. Again, adding the two additional functions is very straightforward once the covariance matrix has been implemented.

…

On Oct 26, 2017, at 10:23 PM, Bob Carpenter ***@***.***> wrote: Is that meant to be a minimal set of requirements for a pull request? Is cov_exp_quad not useful because it's incomplete? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#660 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABdNlranHAPRZQxIjAHTolGBanWBzsTqks5swT6ggaJpZM4QHott>.

[bob-carpenter]

Is this the issue for fixing our existing not-up-to-spec covariance function?

#501

If so, it seems sensible to me to fix that first so we have at least one example to show people.

Personally, I'd rather get partial features in if they're useful and then backfill when necessary. It doesn't seem the full implementation for covariance functions is necessary for some of the functions to be useful.

My main worry is that he higher the hurdles are for submission, the fewer volunteer developers we'll be able to attract.

[olspert]

As I've been informed by Aki, it is ok if I make the pull request for the current issue without including yet the gp_name_chol and gp_name_lpdf to the implementation.
But later I guess together with issue #501 the corresponding functions for periodic covariance functions should be also added.

[bob-carpenter]

@betanalpha and @syclik: Are you OK with @olspert's plan:

#660 (comment)

[syclik]

I'm ok with the plan! Thanks!

On a side note, perhaps we should change the language of the wiki: I think "must" should be replaced with "should." It's written in a style of a requirements document, but they weren't requirements before implementing cov_exp_quad and we don't actually know the effort level required to finish out what's listed there. (I implemented most of the cov_exp_quad functions the first time around and it was already a ton of effort to implement that much -- so much that it really took @rtrangucci's help to finish; maybe the other signatures would actually not take that much time, but not sure.)