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

Progress message ratio_divergent_transitions_during_adaption bug #373

Open
sefffal opened this issue Jul 28, 2024 · 3 comments
Open

Progress message ratio_divergent_transitions_during_adaption bug #373

sefffal opened this issue Jul 28, 2024 · 3 comments

Comments

@sefffal
Copy link

sefffal commented Jul 28, 2024

This is difficult to demonstrate, but I believe the value of ratio_divergent_transitions_during_adaption printed during sampling when progress=true is currently incorrect.

The behaviour I've seen is:

  1. During adaptation, numerical errors are encountered causing ratio_divergent_transitions_during_adaption to increase
  2. By the end of adaptation, the numerical errors stop
  3. Sampling continues after adaptation and ratio_divergent_transitions stays zero.
  4. ratio_divergent_transitions_during_adaption starts to drop (even though adaptation is now finished!) until it reaches a lower value, eg 50% lower if there are equal numbers of samples during and after adaptation.

I couldn't find the right location in the code, but I believe that ratio_divergent_transitions_during_adaption might be calculating the ratio using the total number of samples so far, and not stopping updating after adaptation completes.

@sefffal
Copy link
Author

sefffal commented Jul 28, 2024

I think I see the issue:
https://github.com/TuringLang/AdvancedHMC.jl/blob/2b3814ccbb36806eb0f21c2c937146606a873884/src/sampler.jl#L198C1-L199C62

percentage_divergent_transitions = num_divergent_transitions / I
percentage_divergent_transitions_during_adaption =
    num_divergent_transitions_during_adaption / i

should instead be:

percentage_divergent_transitions = num_divergent_transitions / I
percentage_divergent_transitions_during_adaption =
    num_divergent_transitions_during_adaption / min(i, n_adapts)

@sefffal
Copy link
Author

sefffal commented Oct 11, 2024

Bump on this issue; it's minor but the fix is one line! Thanks.

@torfjelde
Copy link
Member

Oh, good catch @sefffal ! Would you mind making a PR or you want one of us to just do it?:)

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

No branches or pull requests

2 participants