-
Notifications
You must be signed in to change notification settings - Fork 18
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
Ensure ensemble sampling is reproducible #97
Conversation
Codecov Report
@@ Coverage Diff @@
## master #97 +/- ##
==========================================
+ Coverage 97.73% 97.74% +0.01%
==========================================
Files 7 7
Lines 221 222 +1
==========================================
+ Hits 216 217 +1
Misses 5 5
Continue to review full report at Codecov.
|
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.
Ah, good fix and good catch!
I rolled the version up because I submitted 3.3.1 too early, woops 😥 |
Hmm are you sure? Wasn't 3.3.1 correct? With 3.3.2 automerge complains: JuliaRegistries/General#55232 |
I noticed that in #95 I accidentally introduced a reproducibility issue for multithreaded sampling: If seeds are set for each chunk (as in the PR), then the samples will depend on the number of threads. This PR fixes the issue and reverts this change: For each chain, the RNG is set to a specific reproducible seed.
Additionally, I added the same seeds to the serial sampling (they are already used in the distributed sampling). This ensures that the samples do not depend on the chosen ensemble methods. It could be removed from the PR if not desired.