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

Integrate Slice Sampling: Covariance Matching MCMC #896

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

lorcandelaney
Copy link
Contributor

@lorcandelaney lorcandelaney commented Aug 8, 2019

See #772

Includes:

  1. Method for Covariance-Adaptive Slice Sampling: Covariance Matching.
  2. Tests for Covariance Matching method.
  3. TO DO: Notebooks.

@codecov
Copy link

codecov bot commented Aug 8, 2019

Codecov Report

Merging #896 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #896    +/-   ##
======================================
  Coverage     100%   100%            
======================================
  Files          66     67     +1     
  Lines        7052   7180   +128     
======================================
+ Hits         7052   7180   +128
Impacted Files Coverage Δ
pints/_mcmc/_slice_covariance_matching.py 100% <100%> (ø)
pints/toy/_repressilator_model.py 100% <0%> (ø) ⬆️
pints/residuals_diagnostics.py 100% <0%> (ø) ⬆️
pints/_mcmc/_population.py 100% <0%> (ø) ⬆️
pints/toy/_sir_model.py 100% <0%> (ø) ⬆️
pints/_core.py 100% <0%> (ø) ⬆️
pints/toy/_sho_model.py 100% <0%> (ø) ⬆️
pints/_optimisers/_xnes.py 100% <0%> (ø) ⬆️
pints/_mcmc/_mala.py 100% <0%> (ø) ⬆️
pints/_mcmc/_rao_blackwell_ac.py 100% <0%> (ø) ⬆️
... and 57 more

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 fe3e6d7...dbc221e. Read the comment docs.

Copy link
Collaborator

@ben18785 ben18785 left a comment

Choose a reason for hiding this comment

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

Very readable code -- nice work! I've suggested a few (very minor) changes. I can't see where this code deviates from what is written. The only thing I'm unsure about is whether ask / tell returns the gradient or negative gradient and if F and R need to be transposed (as this is the last part of your chud function).

Can you add an example notebook here that shows the issue? I couldn't find one.

pints/_mcmc/_slice_covariance_matching.py Show resolved Hide resolved
pints/_mcmc/_slice_covariance_matching.py Outdated Show resolved Hide resolved
pints/_mcmc/_slice_covariance_matching.py Show resolved Hide resolved
pints/_mcmc/_slice_covariance_matching.py Show resolved Hide resolved
pints/_mcmc/_slice_covariance_matching.py Outdated Show resolved Hide resolved
pints/_mcmc/_slice_covariance_matching.py Outdated Show resolved Hide resolved
pints/_mcmc/_slice_covariance_matching.py Outdated Show resolved Hide resolved
pints/_mcmc/_slice_covariance_matching.py Outdated Show resolved Hide resolved
pints/_mcmc/_slice_covariance_matching.py Outdated Show resolved Hide resolved
pints/_mcmc/_slice_covariance_matching.py Outdated Show resolved Hide resolved
@MichaelClerx
Copy link
Member

MichaelClerx commented Sep 3, 2019

@lorcandelaney if the other PR is "correct" does that mean this one can go??

Sorry it's a different method isn't it?

@MichaelClerx MichaelClerx mentioned this pull request Sep 3, 2019
49 tasks
@MichaelClerx
Copy link
Member

Is this ready for re-review @ben18785 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants