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

Moving max_num_doublings of NUTS from build_kernel to kernel #566

Merged
merged 1 commit into from
Sep 4, 2023

Conversation

weiyaw
Copy link
Contributor

@weiyaw weiyaw commented Sep 4, 2023

Implement #564

  1. Making the argument consistent with 'num_integration_steps' in HMC
  2. Allows running adaptation (e.g. window_adaptation) with an arbitrary max_num_doublings

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.

Making the argument consistent with 'num_integration_steps' in HMC
@junpenglao
Copy link
Member

Could you also check whether any of the code in https://github.com/blackjax-devs/sampling-book needs updating?

@codecov
Copy link

codecov bot commented Sep 4, 2023

Codecov Report

Merging #566 (b6fd52b) into main (1817ee9) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #566   +/-   ##
=======================================
  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%> (ø)

@weiyaw
Copy link
Contributor Author

weiyaw commented Sep 4, 2023

Could you also check whether any of the code in https://github.com/blackjax-devs/sampling-book needs updating?

No update required as far as I know. But I do discover a few bugs related to the API changes from previous commits.

@junpenglao junpenglao merged commit 655c36b into blackjax-devs:main Sep 4, 2023
7 checks passed
@junpenglao
Copy link
Member

No update required as far as I know. But I do discover a few bugs related to the API changes from previous commits.

Thank you for double checking. If you could update those it would be much appreciated!

@weiyaw weiyaw deleted the move-nuts-kernel-args branch September 5, 2023 05:28
junpenglao pushed a commit that referenced this pull request Mar 12, 2024
Making the argument consistent with 'num_integration_steps' in HMC
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