I1137 horowitz langevin mcmc #1138
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1138 +/- ##
==========================================
Coverage 100.00% 100.00%
==========================================
Files 87 81 -6
Lines 8936 7961 -975
==========================================
- Hits 8936 7961 -975
Continue to review full report at Codecov.
|
|
@MichaelClerx @chonlei @ben18785 |
|
To clarify: |
|
@DavAug I've just tried it, it seems working for me (both the master and this branch). I am using v2.4.4 for Sphinx. Have you got Sphinx installed properly? Can you try to reinstall pints with |
|
Just thinking given their similarity, would it be worth to have this HorowitzLangevinMCMC inherits from the HamiltonianMCMC class...? |
|
Thanks I’ll try that.
I think that would be a really good idea if all methods from HMC would be in the HorowitzLangevin sampler. But since the leapfrog steps have been removed does it still make sense?
I guess what could be done is rewriting MALA into a more obvious form of HMC with one leapfrog step and then let HMC inherit from Langevin and also Horowitz from Langevin. But I’m not sure whether those changes are desirable right now?
On 15 May 2020, at 18:34, Chon Lok Lei <notifications@github.com> wrote:
Just thinking given their similarity, would it be worth to have this HorowitzLangevinMCMC inherits from the HamiltonianMCMC class...?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#1138 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AEY2T3XC65QDD2SYME3Z3YTRRVVAVANCNFSM4NBQRFAQ>.
|
There was a problem hiding this comment.
Thanks @DavAug looks like a really great start. My thoughts are:
- a) let's call this sampler the ``NealLangevin'' method
- b) let's then include a hyperparameter that allows switching on/off the non-reversible bit (i.e. changing u)
Also, could we write an example notebook that shows that this is working?
|
This sampler is not the Neal Langevin Sampler yet. The updates of u are not included.
I was under the impression that we wanted to include both MCMC samplers? Horowitz-Langevin and Neal Langevin. Even though Neal reduces to Horowitz when we turn of the u updates and Horowitz reduces to Langevin when turning of the persistence of the momentum, does it not still make sense to have them as separate methods?
Get Outlook for iOS<https://aka.ms/o0ukef>
…________________________________
From: ben18785 <notifications@github.com>
Sent: Friday, May 15, 2020 11:35:48 PM
To: pints-team/pints <pints@noreply.github.com>
Cc: David Augustin <david.augustin@cs.ox.ac.uk>; Mention <mention@noreply.github.com>
Subject: Re: [pints-team/pints] I1137 horowitz langevin mcmc (#1138)
@ben18785 requested changes on this pull request.
Thanks @DavAug<https://github.com/DavAug> looks like a really great start. My thoughts are:
* a) let's call this sampler the ``NealLangevin'' method
* b) let's then include a hyperparameter that allows switching on/off the non-reversible bit (i.e. changing u)
Also, could we write an example notebook that shows that this is working?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#1138 (review)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AEY2T3S6GCVUMEPBUVHR343RRWYTJANCNFSM4NBQRFAQ>.
|
|
Sorry, I was not clear when we spoke this morning! (Was still trying to understand how the sampler worked and the best way to implement it.)
If we include Horrowitz and the Neal sampler as separate objects, that means we’ve got a load of duplicate code that all needs to be maintained.
Also, I think that Horrowitz probably isn’t worth having on its own: it’s very similar to MALA and this sort of persistent momentum sampling isn’t thought to help much.
The Neal sampler may do though. Having a hyperparameter that switches on/off reversibility allows the best of both worlds, I reckon.
… On 15 May 2020, at 22:40, David Augustin ***@***.***> wrote:
This sampler is not the Neal Langevin Sampler yet. The updates of u are not included.
I was under the impression that we wanted to include both MCMC samplers? Horowitz-Langevin and Neal Langevin. Even though Neal reduces to Horowitz when we turn of the u updates and Horowitz reduces to Langevin when turning of the persistence of the momentum, does it not still make sense to have them as separate methods?
Get Outlook for iOS<https://aka.ms/o0ukef>
________________________________
From: ben18785 ***@***.***>
Sent: Friday, May 15, 2020 11:35:48 PM
To: pints-team/pints ***@***.***>
Cc: David Augustin ***@***.***>; Mention ***@***.***>
Subject: Re: [pints-team/pints] I1137 horowitz langevin mcmc (#1138)
@ben18785 requested changes on this pull request.
Thanks @DavAug<https://github.com/DavAug> looks like a really great start. My thoughts are:
* a) let's call this sampler the ``NealLangevin'' method
* b) let's then include a hyperparameter that allows switching on/off the non-reversible bit (i.e. changing u)
Also, could we write an example notebook that shows that this is working?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#1138 (review)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AEY2T3S6GCVUMEPBUVHR343RRWYTJANCNFSM4NBQRFAQ>.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
|
This is the completed NealLangevin Sampler.
|
|
@DavAug Besides the documentation that you have included in the Python class, you also have to include and provide a link to the docs in order to let the Sphinx knows what to link to where! |
|
@DavAug I have added those to the docs for you, so that you don't have to find where to add this time. Remember to pull and merge before doing any updates/push. Also if you now run the make, you will see some warming to fix for the documentation that you have in the MCMC class. Can you please fix those? And you might want to change the title to "Neal Langenvin MCMC" instead of "Horowitz Langenvin MCMC" too! |
There was a problem hiding this comment.
Thanks @DavAug. I haven't looked through this in detail yet but is looking good.
My thoughts are:
- I'd be interested to hear what @MichaelClerx thinks about this but, seeing as this method is so similar to (essentially derived from) HMC, should this method be derived from that?
- Can we have an example notebook please?
|
@chonlei Great thanks! It makes sense, but I didn't know that you had to give further instructions to Sphinx :D |
|
@ben18785 Yes, I can make an example notebook. |
|
I somehow don't seem to have the permission to look at codecov's analysis and see which lines where not hit. Is there something we can do about it? |
Even after you hit "Login with github" ? |
|
Yes, a Github API error is thrown, but it's apparently a Safari issue. It works with Chrome. |
Closes #1137
The algorithm is largely the same as the existing a
pints.HamiltonianMCMC. I made references in the ask and tell methods where the code has been adapted.The other change was to remove the hyper parameter leapfrog_steps and introduce a new one alpha that controls the persistence of the momentum.