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

Update examples #93

Merged
merged 3 commits into from
Jan 7, 2021
Merged

Update examples #93

merged 3 commits into from
Jan 7, 2021

Conversation

devmotion
Copy link
Member

@devmotion devmotion commented Jan 6, 2021

This PR updates the examples.

It contains the following changes:

  • Fixes AdvancedHMC example broken #91 by correcting the log density - it did not take into account the prior
  • Consistently uses a Gaussian noise with standard deviation 0.1 (same as GPFlow original)
  • Merges the ESS and HMC example which contained identical and incorrectly copied parts
  • Adds DynamicHMC example
  • Uses softplus transformation (same as GPFlow original)

I tried to improve and add some more explanations but I am still not completely satisfied. In my opinion, it is also unsatisfying that the VI and MCMC example contain many duplicate parts, and even the different method sections in the MCMC example are very similar.

@devmotion
Copy link
Member Author

devmotion commented Jan 7, 2021

Maybe it would be best to merge the MCMC and VI example as well and just have one "Regression (1D)" example (similar to GPFlow) with different approaches and packages.

At some point it would also be nice to use a dataset with more than 12 observations...

@willtebbutt
Copy link
Member

I could on board with both of your above points. Are you up for tackling the merging examples one inside this PR, or would you prefer to do it separately?

@devmotion
Copy link
Member Author

I'll merge them in this PR since one only has to copy the VI section to the other file.

@theogf
Copy link
Member

theogf commented Jan 7, 2021

As an additional comment I saw multiple people getting confused with data having more than one feature, an example based on matrices/vector of vectors would also be great!
A simple 2D example for example

@willtebbutt willtebbutt mentioned this pull request Jan 7, 2021
10 tasks
@willtebbutt
Copy link
Member

willtebbutt commented Jan 7, 2021

As an additional comment I saw multiple people getting confused with data having more than one feature, an example based on matrices/vector of vectors would also be great!
A simple 2D example for example

This is a good point. I've opened this issue where I think that we should move the discussion to so that we can get this merged.

@willtebbutt
Copy link
Member

Is there any easy way to view the new documentation based on these examples, or should I pull and build locally?

@theogf
Copy link
Member

theogf commented Jan 7, 2021

Is there any easy way to view the new documentation based on these examples, or should I pull and build locally?

I think you need to pull locally but I managed to create a PR preview for website for Franklin, maybe I can manage the same thing for documentation, but that's for another PR

@devmotion
Copy link
Member Author

It would be possible to set up preview builds of the documentation but currently you have to build it locally.

@devmotion
Copy link
Member Author

I opened a PR: #96 We could wait for it, then it is probably easier to review the changes here.

@willtebbutt
Copy link
Member

I opened a PR: #96 We could wait for it, then it is probably easier to review the changes here.

Unless you are desperate to get this in quickly, I'm happy to wait.

@devmotion devmotion closed this Jan 7, 2021
@devmotion devmotion reopened this Jan 7, 2021
@codecov-io
Copy link

Codecov Report

Merging #93 (5b6fc20) into master (ee122e9) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #93   +/-   ##
=======================================
  Coverage   99.04%   99.04%           
=======================================
  Files           9        9           
  Lines         210      210           
=======================================
  Hits          208      208           
  Misses          2        2           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ee122e9...5b6fc20. Read the comment docs.

@devmotion
Copy link
Member Author

devmotion commented Jan 7, 2021

You can view the preview here: https://juliagaussianprocesses.github.io/AbstractGPs.jl/previews/PR93/

The "Doc Preview Cleanup" error can be ignored, the action was executed since I closed the PR but no preview existed since the preview builds were just enabled.

Copy link
Member

@willtebbutt willtebbutt 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 happy for this to be merged whenever you're happy @devmotion . I think the language / explanation is basically fine -- we can always iterate again later if needs be.

@devmotion
Copy link
Member Author

Yes, it can definitely be improved further, I was just too tired to optimize it any further at some point.

@devmotion devmotion merged commit 7a69762 into master Jan 7, 2021
@devmotion devmotion deleted the dw/examples branch January 7, 2021 13:10
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.

AdvancedHMC example broken
4 participants