add acceptance rate info to Transition struct

[cnrrobertson]

Solves (at least partially) #40.

[devmotion]

Could this break Turing? IIRC at some point (and maybe still?) we constructed Transition objects there.

[cnrrobertson]

This latest iteration pushes accepted_draws and total_draws to the MHSampler type objects rather than keeping them in the Transition. Note that this required adjusting the constructors (particularly the MALA constructor).

[cnrrobertson]

Any thoughts on this since the last changes? I need to go through Turing to find the small adjustments that will be needed to work with this but I want to get any feedback on this implementation before I do.

[devmotion]

I don't remember the previous discussion but this design seems a bit surprising to me. I think the number of accepted and total draws is not a property of the sampler but rather of the resulting chain.

[cnrrobertson]

Yeah, the thought here was that samplers have a record of success or failure. This would be useful for implementing delayed rejection or other sample massaging. But that information could probably be retrieved from the vector of transitions along the way.

With that in mind, maybe we can stick with Transition objects just storing whether the sample was accepted or rejected and compute final stats in chain construction or in sampling if needed. I know this PR is already a mess, but I'll revert and make the adjustments.

[Red-Portal]

Is there any plan to push this forward?

[cpfiffer]

I personally see no issue with this. If tests pass I say we merge it in.

[cnrrobertson]

Great! My local tests are passing. Any idea why the nightly tests are failing in the CI?

[cpfiffer]

I'm going to merge this in for now. Test failures are all on nightly. As a side note, we should maybe consider not testing on nightly, or at least permitting failure.

[cpfiffer]

Tbh I'm not terribly concerned with nightly at the moment. Thank you @cnrrobertson for the PR! Sorry it took so long to merge. I'll increment the patch number separately and flag a release.

[devmotion]

My impression the last time I checked the PR was that AbstractMCMC features such as discard = ..., thinning = ... etc. will break this functionality. But maybe it's fine if your acceptance rate is wrong if you don't perform "vanilla" sampling?