Convert acceptance rate of nan to 0 in dual averaging #29
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bob-carpenter
requested changes
Sep 25, 2025
include/walnuts/dual_average.hpp
Outdated
| * @pre alpha > 0 | ||
| */ | ||
| inline void observe(S alpha) noexcept { | ||
| if (std::isnan(alpha)) { |
There was a problem hiding this comment.
I think this should add || std::isinf(alpha) test to catch the -infinity case, too. I don't think we should ever see +infinity, but it could be std::isinf(alpha) && alpha < 0 or I think we can actually just compare to -infinity.
More generally, we could condition to [0, 1] by mapping NaN and anything < 0 to 0 and anything > 1 to 1. I opened another issue to test his upper bounding for adaptation---as is, we pass in Metropolis ratios here, which can be greater than 1.
bob-carpenter
approved these changes
Sep 25, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
While looking at #26 @bob-carpenter noticed it was possible for a model to have the parameters end up as nan and stay there forever. I tracked it down to this sequence of events:
nannanI believe Stan traps this behavior here:
https://github.com/stan-dev/stan/blob/develop/src/stan/mcmc/hmc/nuts/base_nuts.hpp#L259-L260
Rather than do exactly what they do, I've moved this to inside the dual averaging. This moves it out of the code that we're relying on dead code optimization to handle for us when using the
NoOpHandlerin non-adaptive walnuts.I believe 0 is the correct value, working through what would happen if I did exactly what Stan did and set the bad energy value to inf, the acceptance probability would work out to exp(-inf) = 0