-
Notifications
You must be signed in to change notification settings - Fork 6
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
Add Vecchia approximation #147
Add Vecchia approximation #147
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for opening this @samanklesaria . Will be good to have!
Does the Vecchia approximation provide an approximation to the log marginal likelihood? Would be great to have that available if possible.
This reverts commit acc8fb1.
@samanklesaria please do let me know when this is ready for another review. |
@willtebbutt all set for another review! I don't know why all the CI jobs are failing, but it seems to be unrelated to the code here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for improving the PR. I've made a few cosmetic remarks, but I think we're nearly ready to go now.
Co-authored-by: Will Tebbutt <[email protected]>
Ah, sorry, you'll need to import |
My gradient tests are failing now- investigating it. |
Everything seems to be working! Thanks for the suggestions. |
Wonderful -- just a couple of formatting suggestions to fix, then we can merge. |
Are the changes from |
I'm going to merge as-is if you're happy @samanklesaria -- the AD issues will need to be addressed in another PR. |
Sounds good! I can't seem to reproduce the AD issues. Might have been a quirk with an inconsistent |
Ooo before I merge, could you please bump the version number in the Project.toml to 0.4.6 so that I can tag a release? |
Thanks for bumping the version. I'm going to merge + tag despite the test failures, because AD problems need to be addressed elsewhere. |
d5ef8a9
into
JuliaGaussianProcesses:master
Thanks for the contribution @samanklesaria |
I thought I might add the Vecchia approximation, as described in https://arxiv.org/pdf/2102.13299.pdf.