Add AbstractMCMC.getstats and AbstractMCMC.requires_unconstrained_space
#182
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
|
AbstractMCMC.jl documentation for PR #182 is available at: |
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.
In general there are two things that samplers return per iteration: parameter values, and sampler statistics.
getparamswas added previously to cover the first one, but there's no interface function for the second.Currently, for Turing's external sampler interface, external samplers are expected to implement
Turing.Inference.getparamsandTuring.Inference.getstatsThis is a mistake, because external samplers shouldn't need to depend on Turing, they should only depend on AbstractMCMC and LogDensityProblems (because they only need to take an
AbstractMCMC.LogDensityModeland callLogDensityProblems.logdensityon its wrapped model). I'm now trying to unravel this, hence the need for this function.In the same vein, this PR also moves
requires_unconstrained_spacefrom Turing to here. I'm less certain about this. Please see Slack for discussion. But I think that whether a sampler requires unconstrained space or not is a property of the sampler (and thus belongs in the package that defines AbstractSampler), not a property of how it interacts with Turing.