-
Notifications
You must be signed in to change notification settings - Fork 9
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
Remove pre_sample, pre_warmup from NUTS and fix warning #545
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @amal-ghamdi. Really nice! I only had a comment related to the deepcopy. Perhaps we can find a more efficient method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @amal-ghamdi , this PR looks good and I don't have any technical comments; but I did try the two demos on my machine and the results look great. BTW, it's good to know this trick branch = repo.active_branch
: )
Thank you @chaozg for your review! |
closes #524
closes #537
Verification Code 1:
Code verifies this change still gives same results as old NUTS (also this is verified by regression tests)
Results main branch:
Results this branch:
Verification Code 2 (added as a test in the PR, for smaller sample size):
results main branch
results this branch