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 the Metropolis-within-Gibbs example #558

Merged
merged 4 commits into from
Sep 15, 2023

Conversation

twhentschel
Copy link
Contributor

@twhentschel twhentschel commented Jul 14, 2023

Update the Metropolis-within-Gibbs example markdown notebook to be compatible with API changes, as mentioned in #551.

Included a function that takes the variance of a normal distribution as a parameter and returns a RMH kernel with a normal proposal generator. Also added minor changes to text to keep consistency with updates. I have a couple more thoughts for a few more minor edits, so I'm opening this up a draft PR (also I'm a first time contributor).

potential (nitpicky) edits:

  • AlgorithmState is mentioned, but is this an actual object in the code base referring to some generic class or type? If not, I think it would be better to refer to a generic state as "algorithm state", so its not confused as something in blackjax.
  • Change references of blackjax.mcmc.algorithm.init() to just init(), as the former makes it seem like there is an algorithm submodule.

Also, make test reports that 3 tests are failing related to tests/optimizers/test_pathfinder.py. But I don't think it's related to my changes.

Thank you for opening a PR!

A few important guidelines and requirements before we can merge your PR:

  • If I add a new sampler, there is an issue discussing it already;
  • We should be able to understand what the PR does from its title only;
  • There is a high-level description of the changes;
  • There are links to all the relevant issues, discussions and PRs;
  • The branch is rebased on the latest main commit;
  • Commit messages follow these guidelines;
  • The code respects the current naming conventions;
  • Docstrings follow the numpy style guide
  • pre-commit is installed and configured on your machine, and you ran it before opening the PR;
  • There are tests covering the changes;
  • The doc is up-to-date;
  • If I add a new sampler* I added/updated related examples

Consider opening a Draft PR if your work is still in progress but you would like some feedback from other contributors.

Tommy Hentschel and others added 4 commits July 13, 2023 14:16
Update the Metropolis-within-Gibbs example markdown notebook to be
compatible with API changes.

Minor changes to text to keep consistency with code.
@junpenglao junpenglao marked this pull request as ready for review September 15, 2023 08:51
@junpenglao junpenglao enabled auto-merge (squash) September 15, 2023 08:51
@junpenglao
Copy link
Member

Thanks for your contribution! Actually the change could be a bit more simple, please see the patch that i push

@codecov
Copy link

codecov bot commented Sep 15, 2023

Codecov Report

Merging #558 (5382a1b) into main (1817ee9) will not change coverage.
Report is 1 commits behind head on main.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #558   +/-   ##
=======================================
  Coverage   99.14%   99.14%           
=======================================
  Files          49       49           
  Lines        2099     2099           
=======================================
  Hits         2081     2081           
  Misses         18       18           
Files Changed Coverage Δ
blackjax/mcmc/nuts.py 100.00% <100.00%> (ø)

@junpenglao junpenglao merged commit 8018fc4 into blackjax-devs:main Sep 15, 2023
junpenglao added a commit that referenced this pull request Mar 12, 2024
* Update Metropolis-within-Gibbs example

Update the Metropolis-within-Gibbs example markdown notebook to be
compatible with API changes.

Minor changes to text to keep consistency with code.

* Fix trailing whitespace

* simplify changes

---------

Co-authored-by: Tommy Hentschel <th584@cornell.edu>
Co-authored-by: Junpeng Lao <junpenglao@gmail.com>
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.

2 participants