Return log-densities + divergent transition info from impulse.infer#2958
Open
penelopeysm wants to merge 4 commits into
Open
Return log-densities + divergent transition info from impulse.infer#2958penelopeysm wants to merge 4 commits into
impulse.infer#2958penelopeysm wants to merge 4 commits into
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Currently
inferonly returns a single diagnostic, a boolean which indicates whether or not the transition was accepted. Naturally, it's often useful to have more info :)Right now (some of) this info is already calculated but is thrown away. This PR is a pretty mechanical way of surfacing that. The
diagnosticstensor is expanded to include a second column which indicates whether the transition is divergent, plus a separate new tensor containing the logdensities is returned.This is a bit ugly and not incredibly generalisable, but since it's quite closely tied to the sampling algorithm and
inferonly admits two algorithms, I think it's fine for now. My feeling is that properly generalising diagnostics only really makes sense in the context of broader programmable inference APIs.This ofc needs changes in Reactant as well which I'll push separately. I have tested this locally end to end and it all works as intendeds