Generally support container of proposals #58
Conversation
|
Looks good to me, I'll take another look after tests pass. |
|
Turned out that MALA required some fixes. I extracted the computation of the log acceptance probability to a separate function but I am going to revert it I think - it feels too hacky how it is used in the implementation of MALA, so I think it is cleaner to just implement |
| proposal(-gradient_logdensity_candidate), state, candidate | ||
| ) - q(proposal(-gradient_logdensity_state), candidate, state) |
There was a problem hiding this comment.
This is not modified by this PR but I'm a bit confused why one uses proposal(-gradient_logdensity_candidate) here even though the algorithm uses proposal(gradient_logdensity_candidate) for sampling the proposal. I guess there's a simple explanation and I'm just too tired to see it right now 😴
There was a problem hiding this comment.
Not sure, honestly. @briandepasquale do you know why the negative is used here?
There was a problem hiding this comment.
Maybe let's open an issue and discuss it separately since it does not change the current implementation?
There was a problem hiding this comment.
Hi all. I recall that minus sign... Let me take a look. Yes perhaps within an issue is more appropriate. I will be a little slow though - I am deep in an academic job search right now and about to start interviewing, very time intensive and stressful. But hopefully won't be months, like my original PR :)
This PR addresses #38 (comment) (no implicit construction of
Transitionand computation of the log density when generating the proposal) and #53 (first class support for containers of proposals).Currently, the latter is only supported if the proposals are part of
MHSampler. Moreover, I imagine this allows us to remove support forProposals which itself include an array of proposals in a second step.