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

DRAM ACMC #982

Merged
merged 28 commits into from
Apr 1, 2020
Merged

DRAM ACMC #982

merged 28 commits into from
Apr 1, 2020

Conversation

ben18785
Copy link
Collaborator

@ben18785 ben18785 commented Sep 30, 2019

Closes #295

@codecov
Copy link

codecov bot commented Sep 30, 2019

Codecov Report

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

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #982    +/-   ##
======================================
  Coverage     100%   100%            
======================================
  Files          68     69     +1     
  Lines        7230   7335   +105     
======================================
+ Hits         7230   7335   +105
Impacted Files Coverage Δ
pints/_mcmc/_dram_ac.py 100% <100%> (ø)

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 146ea5f...8bd5a27. Read the comment docs.

@MichaelClerx
Copy link
Member

Hey @ben18785
Looks like this needs updating to the new structure? Imports a GlobalAdaptiveCovarianceMCMC at some point that's no longer define

Copy link
Member

@MichaelClerx MichaelClerx left a comment

Choose a reason for hiding this comment

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

Looks good, but notebook could do with a bit of interpretation of the results.

"Try with 2 proposal kernels instead." Could you point out the issues in the first run in the images? And show how that gets better?

Will make some small suggestions in the code now too :-)

@ben18785
Copy link
Collaborator Author

Looks good, but notebook could do with a bit of interpretation of the results.

"Try with 2 proposal kernels instead." Could you point out the issues in the first run in the images? And show how that gets better?

Will make some small suggestions in the code now too :-)

Very good point -- the notebook was pretty bare before! haha

Have now added more detail and made the notebook a bit more readable. See what you think.

@MichaelClerx
Copy link
Member

Thanks Ben, looks great!
To me the traces look wild! Is that a good thing? Massively-mixing mcmc?

@MichaelClerx
Copy link
Member

@ben18785 are you happy for me to merge this?

@ben18785
Copy link
Collaborator Author

ben18785 commented Mar 31, 2020 via email

@MichaelClerx MichaelClerx merged commit 2f558b2 into master Apr 1, 2020
@MichaelClerx MichaelClerx deleted the dram branch April 1, 2020 09:01
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.

Implement delayed acceptance adaptive Metropolis (DRAM)
2 participants