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

Additional GP kernels #741

Merged
merged 9 commits into from
Aug 9, 2024
Merged

Additional GP kernels #741

merged 9 commits into from
Aug 9, 2024

Conversation

sbfnk
Copy link
Contributor

@sbfnk sbfnk commented Aug 8, 2024

Description

This PR introduces additional GP kernels: Ornstein-Uhlenbeck and Matérn 5 / 2, for more modelling choices especially for forecasting.

Someone would have to check the maths is correct before this is merged.

Initial submission checklist

  • My PR is based on a package issue and I have explicitly linked it. not fixing issue but an extension of functionality
  • I have tested my changes locally (using devtools::test() and devtools::check()).
  • I have added or updated unit tests where necessary.
  • I have updated the documentation if required and rebuilt docs if yes (using devtools::document()).
  • I have followed the established coding standards (and checked using lintr::lint_package()).
  • I have added a news item linked to this PR.

After the initial Pull Request

  • I have reviewed Checks for this PR and addressed any issues as far as I am able.

@seabbs
Copy link
Contributor

seabbs commented Aug 8, 2024

Nice! Very exciting

@seabbs seabbs self-requested a review August 8, 2024 09:32
Copy link
Contributor

github-actions bot commented Aug 8, 2024

This is how benchmark results would change (along with a 95% confidence interval in relative change) if b4c211c is merged into main:

  • ❗🐌default: 33.1s -> 41.9s [+12.88%, +40.5%]
  • ✔️no_delays: 41.9s -> 52s [-5.74%, +53.8%]
  • ✔️random_walk: 9.84s -> 10.4s [-7.09%, +19.12%]
  • ✔️stationary: 20.2s -> 21.2s [-8.88%, +19.11%]
  • ✔️uncertain: 59.7s -> 1.25m [-4.26%, +55.7%]
    Further explanation regarding interpretation and methodology can be found in the documentation.

@sbfnk
Copy link
Contributor Author

sbfnk commented Aug 8, 2024

(reference for the maths is the approximate hilbert space gp paper https://arxiv.org/abs/2004.11408 on p.4 - perhaps something we should mention somewhere)

R/create.R Show resolved Hide resolved
@seabbs
Copy link
Contributor

seabbs commented Aug 8, 2024

We should cite and discuss this tutorial: https://avehtari.github.io/casestudies/Motorcycle/motorcycle_gpcourse.html#4_Heteroskedastic_GP_with_Hilbert_basis_functions

It also looks to contain a better way to set L and a way to directly build the diagonal matrix

Copy link
Contributor

@seabbs seabbs left a comment

Choose a reason for hiding this comment

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

This looks good. I've just looked at the code side so far and will look at the maths side tomorrow.

I think as is is fine but the Aki tutorial suggests there are now faster/more direct ways to write this and potentially some better approaches to parameterise.

inst/stan/functions/gaussian_process.stan Show resolved Hide resolved
inst/stan/functions/gaussian_process.stan Show resolved Hide resolved
Copy link
Contributor

@seabbs seabbs left a comment

Choose a reason for hiding this comment

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

I'm going to look at other parameterisations in another PR. I think this looks good for now. Maths makes sense to me.

@seabbs seabbs added this pull request to the merge queue Aug 9, 2024
Merged via the queue into main with commit d803f1f Aug 9, 2024
13 checks passed
@seabbs seabbs deleted the gp_updates branch August 9, 2024 17:19
@seabbs seabbs mentioned this pull request Aug 12, 2024
7 tasks
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.

2 participants