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

Issue 757: Vectorise GP stan code #742

Merged
merged 107 commits into from
Aug 29, 2024
Merged

Issue 757: Vectorise GP stan code #742

merged 107 commits into from
Aug 29, 2024

Conversation

seabbs
Copy link
Contributor

@seabbs seabbs commented Aug 12, 2024

Description

This PR builds on #741 and closes #757 by vectorising the spectral density functions, using a single matern function, and introducing periodic kernals. It uses ideas found in https://avehtari.github.io/casestudies/Motorcycle/motorcycle_gpcourse.html.

It takes advantage of this refactor to do some prior predictive checking for the priors related to the GP, and then leverages the improved performance to reduce stan settings and relax initialisation constraints (see the news section).

Initial submission checklist

  • My PR is based on a package issue and I have explicitly linked it.
  • 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.

epinow-epinow-1.png Outdated Show resolved Hide resolved

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

Copy link
Contributor

@jamesmbaazam jamesmbaazam left a comment

Choose a reason for hiding this comment

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

Thanks @seabbs. I've now reviewed the code aspects as much as I can. I am currently exploring the various gp kernels locally and will report if I find anything worth discussing.

NEWS.md Outdated Show resolved Hide resolved
R/create.R Outdated Show resolved Hide resolved
R/estimate_infections.R Show resolved Hide resolved
R/opts.R Outdated Show resolved Hide resolved
NEWS.md Outdated Show resolved Hide resolved
inst/stan/estimate_infections.stan Outdated Show resolved Hide resolved
inst/stan/functions/delays.stan Show resolved Hide resolved
tests/testthat/test-create_gp_data.R Outdated Show resolved Hide resolved
R/opts.R Outdated Show resolved Hide resolved
R/opts.R Show resolved Hide resolved
@seabbs

This comment was marked as outdated.

@seabbs seabbs requested a review from jamesmbaazam August 28, 2024 14:42
@seabbs seabbs mentioned this pull request Aug 28, 2024
@seabbs seabbs changed the title Vectorise GP stan code Issue 757: Vectorise GP stan code Aug 28, 2024

This comment was marked as outdated.

Copy link
Contributor

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

  • 🚀default: 46.5s -> 23.7s [-63.53%, -34.48%]
  • 🚀no_delays: 52.7s -> 28.3s [-71.8%, -20.95%]
  • ✔️random_walk: 9.7s -> 10.6s [-1.39%, +20.1%]
  • 🚀stationary: 21.3s -> 15.6s [-41.99%, -10.88%]
  • 🚀uncertain: 1.22m -> 37.5s [-63.74%, -33.68%]
    Further explanation regarding interpretation and methodology can be found in the documentation.

@seabbs seabbs mentioned this pull request Aug 29, 2024
@seabbs seabbs added this pull request to the merge queue Aug 29, 2024
Merged via the queue into main with commit 025693c Aug 29, 2024
15 checks passed
@seabbs seabbs deleted the vectorise-spectral-density branch August 29, 2024 13:49
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.

Vectorise GPs
3 participants