Skip to content

Compute Metropolis average accept probability with logsumexp #7783

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

jessegrabowski
Copy link
Member

@jessegrabowski jessegrabowski commented May 15, 2025

In the Metropolis step stats that we report on the progressbar, we have a probability of acceptance. This is currently computed in a very direct way, just np.mean(np.exp(log_accept_prob)). We can do better with np.exp(logsumexp(log_accept_prob) - log(len(log_accept_prob))).

In really gnarly models, I've encountered overflows in this step. This might also be because there's a max(delta_logp, 1) missing from the acceptance probabilities. But even if we add that, the computation method in this PR is more numerically stable and accurate.


📚 Documentation preview 📚: https://pymc--7783.org.readthedocs.build/en/7783/

Copy link

codecov bot commented May 15, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.84%. Comparing base (bd28ee9) to head (0827196).
Report is 11 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #7783      +/-   ##
==========================================
+ Coverage   92.83%   92.84%   +0.01%     
==========================================
  Files         107      107              
  Lines       18354    18381      +27     
==========================================
+ Hits        17039    17066      +27     
  Misses       1315     1315              
Files with missing lines Coverage Δ
pymc/step_methods/metropolis.py 93.24% <100.00%> (+0.01%) ⬆️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jessegrabowski
Copy link
Member Author

@ricardoV94 any thoughts about just clipping the acceptance probability to be in 0,1 ?

Co-authored-by: Ricardo Vieira <28983449+ricardoV94@users.noreply.github.com>
@ricardoV94
Copy link
Member

@ricardoV94 any thoughts about just clipping the acceptance probability to be in 0,1 ?

Is that why you were getting nans after the exp? Shouldn't hurt, as long as it's all cheap stuff

@jessegrabowski
Copy link
Member Author

Yes I assume it was overflowing. In the progress bar I was seeing large values for acceptance

@ricardoV94
Copy link
Member

Yes I assume it was overflowing. In the progress bar I was seeing large values for acceptance

Then feel free to clip the exp from above or the log to be at most 0 for the returned stat

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants