# Implementig periodic covariance function

**URL:** <https://discourse.mc-stan.org/t/implementig-periodic-covariance-function/1787>\
**Category:** General\
**Created:** [September 5, 2017, 11:03am UTC](https://discourse.mc-stan.org/t/implementig-periodic-covariance-function/1787 "2017-09-05T11:03:43Z")\
**Posts on this page:** 7\
**Page:** 1

<div class="post-metadata">

**Author:** ![nigul](https://avatars.discourse-cdn.com/v4/letter/n/839c29/32.png) [@nigul](https://discourse.mc-stan.org/u/nigul)\
**Post date:** [September 5, 2017, 11:03am UTC](https://discourse.mc-stan.org/t/implementig-periodic-covariance-function/1787/1 "2017-09-05T11:03:43Z")

</div>

Hi all!

I’m new to this forum so please excuse me if I’ve selected the wrong category for this post. I would want to implement a periodic covariance function for Gaussian Processes in Stan, but before creating a development request I’d like to get opinions about what would be the suitable name and order of parameters for it. In the current implementation I’ve written so far the signature of the function looks either cov\_per(x, sigma, l, p) or cov\_per(x1, x2, sigma, l, p) depending on the number of data vectors. The other question would be whether to prefer period or frequency as a parameter (or both, but in this case the amount of code would probaby double). I guess, in some sense, the period as a parameter might seem more natural choice.

Looking forward to get some feedback on this matter.

best regards,  
Nigul

---

<div class="post-metadata">

**Author:** ![betanalpha](https://yyz2.discourse-cdn.com/flex030/user_avatar/discourse.mc-stan.org/betanalpha/32/4_2.png) [@betanalpha](https://discourse.mc-stan.org/u/betanalpha)\
**Post date:** [September 5, 2017, 12:47pm UTC](https://discourse.mc-stan.org/t/implementig-periodic-covariance-function/1787/2 "2017-09-05T12:47:53Z")

</div>

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 the desired specification of new covariance functions.

`cov_exp_quad`, which we know doesn’t follow the above naming convention yet, implements both `cov_exp_quad(y | x, alpha, rho, nugget)` and `cov_exp_quad(y | x1, x2, alpha, rho, nugget)`. Start with the former, add the ladder if you’d like.

Period will likely be more natural for specifying priors, but others should chime in.

---

<div class="post-metadata">

**Author:** ![nigul](https://avatars.discourse-cdn.com/v4/letter/n/839c29/32.png) [@nigul](https://discourse.mc-stan.org/u/nigul)\
**Post date:** [September 5, 2017, 1:35pm UTC](https://discourse.mc-stan.org/t/implementig-periodic-covariance-function/1787/3 "2017-09-05T13:35:51Z")

</div>

Thanks for the reply. I actually took the cov\_exp\_quad as an example while naming the new fuction, but if I now understand correctly then the former one is already not following the specification. Is it still ok to name it as cov\_per for time being then?

---

<div class="post-metadata">

**Author:** ![Bob\_Carpenter](https://yyz2.discourse-cdn.com/flex030/user_avatar/discourse.mc-stan.org/bob_carpenter/32/9230_2.png) [@Bob\_Carpenter](https://discourse.mc-stan.org/u/Bob_Carpenter)\
**Post date:** [September 10, 2017, 11:57am UTC](https://discourse.mc-stan.org/t/implementig-periodic-covariance-function/1787/4 "2017-09-10T11:57:31Z")

</div>

> [@nigul](#):
>
> Is it still ok to name it as cov\_per for time being then?

If it’s something for yourself only, then you’re in charge of naming.

If it’s meant to be a contribution to Stan (which would be great—we can help if you get stuck on some aspect of the process), then you should follow the suggested naming conventions.

---

<div class="post-metadata">

**Author:** ![nigul](https://avatars.discourse-cdn.com/v4/letter/n/839c29/32.png) [@nigul](https://discourse.mc-stan.org/u/nigul)\
**Post date:** [September 11, 2017, 12:54pm UTC](https://discourse.mc-stan.org/t/implementig-periodic-covariance-function/1787/5 "2017-09-11T12:54:20Z")

</div>

Initially I needed the periodic covariance function for my own purposes, but I’ve tried to implement it so that it would be suitable for merging it to the main branch of the Stan at some moment. I’ve been following the guidelines given by Rob Trangucci which referred to the implementations of the functions in stan/math/prim/mat/fun/cov\_exp\_quad.hpp and stan/math/rev/mat/fun/cov\_exp\_quad.hpp. These functions deal with returning the covariance matrix and calculating the gradients, but as I see the new specification involves also implementing functions returning cholesky factors and log probabilities. I’m not quite sure how to proceed from here, but I would gladly continue with this task if possible. Can you tell if there are any implementations which already comply with the new specification, so that I could use them as examples?

---

<div class="post-metadata">

**Author:** ![Bob\_Carpenter](https://yyz2.discourse-cdn.com/flex030/user_avatar/discourse.mc-stan.org/bob_carpenter/32/9230_2.png) [@Bob\_Carpenter](https://discourse.mc-stan.org/u/Bob_Carpenter)\
**Post date:** [September 13, 2017, 11:27am UTC](https://discourse.mc-stan.org/t/implementig-periodic-covariance-function/1787/6 "2017-09-13T11:27:38Z")

</div>

I’m not familiar with Rob’s guidelines. Not everything needs to be implemented for something to be useful.

@rtrangucci — any suggestions here?

The thing to do at this point is to create an issue on `stan-dev/math` to define the feature you want to add. That will give people a chance to give you feedback on the design before you spend a lot of time on implementation.

---

<div class="post-metadata">

**Author:** ![nigul](https://avatars.discourse-cdn.com/v4/letter/n/839c29/32.png) [@nigul](https://discourse.mc-stan.org/u/nigul)\
**Post date:** [October 26, 2017, 2:12pm UTC](https://discourse.mc-stan.org/t/implementig-periodic-covariance-function/1787/7 "2017-10-26T14:12:59Z")

</div>

I created a feature request in the github now:  
[https://github.com/stan-dev/math/issues/660](https://github.com/stan-dev/math/issues/660)