Adding the Kombine Sampler
This modifies/uses the emcee EnsembleSampler wrapper to be used as a wrapper to allow sampling using Ben Farr's kombine code (It has a mostly similar interface as emcee so it was not a huge stretch to extend for kombine to be used).
https://github.com/bfarr/kombine
I am not sure if this will be useful to have kombine has an available sampler but I know Ben (my advisor) has been interested to see it work in bilby.
Merge request reports
Activity
Hi @bruce.edelman this sounds like a great idea! Thanks for looking into it.
It is currently failing the CI because it tries to run and doesn't have kombine installed. While I see you updated the Dockerfile, that first needs to be merged to master in order to get built into the C.I. image. (It is all very circular!)
To resolve this, can you first make a MR to update the Docker file alone. We'll then merge that ASAP which will update the image.
Hi @gregory.ashton , Was just going to ask about that! should I remove that commit in this merge request (New to the contributing procedures here) while I add a MR with the dockerfile change alone?
mentioned in merge request !638 (merged)
Very nice, @bruce.edelman! Just a couple of comments.
-
I would vote against including an example of using this sampler, it's a fairly simple use and doesn't need any unusual options. I'm not wild about the example bloat that this repository often suffers from.
-
You note that this is very similar to the implementation of
emcee, is there a reason not to inherit directly from theEmceeclass?
-
-
Hi @colm.talbot, Ah! yes, I meant to remove that example from my branch as well. And that is a good point as well. There are not any large reason not to inherit from the Emcee class itself with a few overwrites, I will make that change to save some lines of code. Thanks!
added 1 commit
- 2ff34507 - refactored Kombine sampler wrapeper to inherit from emcee class
added 1 commit
- c569a524 - fixed kombine test of default kwargs and flake issues in kombine.py
added 1 commit
- 40239924 - took out f-strings to be python2.7 compatible
added 15 commits
- 39d52b56 - added kombine to dockerfile sampler installations for merge request sudo git...
- f442baf3 - Revert "generalized the JointPrior object structure from Matthew Pitkin's...
- 2c390398 - Add kombine sampler
- aa79cb45 - kombine sampler runs on 2d gaussian example. Does not do checkpointing or resuming yet
- abd8251d - finished fixing kombine checkpointing
- 7a3b3bd6 - fixed python formatting and added kombine to sampler requirements
- 7de6ecc3 - fixed sampler test translate kwargs test for kombine
- f4ab6594 - started GW testing with kombine
- 813766fb - fixed sampler test translate kwargs test for kombine
- 0832b193 - fixed some formatting and commenting
- 008e1881 - revert GW150914.py example to default bilby ex
- d576fe08 - refactored Kombine sampler wrapeper to inherit from emcee class
- 0a8b0eb5 - fixed kombine test of default kwargs and flake issues in kombine.py
- 5d73d1ca - took out f-strings to be python2.7 compatible
- 7f3ea40a - Merge branch 'kombine' of https://git.ligo.org/bruce.edelman/bilby into kombine
Toggle commit listadded 1 commit
- 04c3efaa - removed kombine from docker containers (fixed in talapas ) and TestKombine was...
added Sampling + 1 deleted label
added 1 commit
- 549d21d4 - add check to make sure sure _previous_iterations doesn't error if no previousruns
-
@gregory.ashton It seems the python2.7 CI is failing still because it doesn't have kombine installed. It does see kombine installed during the python3.7 CI though. Is there a known reason for this maybe?
Also related. The python 3.7 tests failing are one thing that doesn't seem related to my changes so I'm not sure what's going on there. The other failure is because the pre-release flag for Emcee is not getting triggered for some reason. I am doing the same thing as in the current master branch just moved it into a function so that the Kombine inherited object didn't have to also do that check, while inheriting the init method. Any thoughts? Thanks!
-
Hi @bruce.edelman sorry for the delayed response.
The python2.7 image was, somehow, broken and not building at all. As such the test which built off it was on an outdated version of the image (the last time it built, about 3 months ago). I removed it on master so if you rebase that issue should be resolved.
I'm trying to update all of the images at moment so that we have a more stable environment for testing. Sorry for the mess with this :) please try to bear with me and I'll be sure to get this merged in soon.
added 7 commits
- 3ed6496d - Fixes persistent warning messaged when using the alligned-spin prior
- c9f33640 - Collection of fixes to resolve issues with testing
- 402e629d - Adding a custom dynesty sampler
- 7e5bce36 - Change infer function call in bilby.hyper.model
- 8c894a1b - allow html writing of waveform plots
- bd879f6b - Adds a set of conda-based test images
- 2fb79f6d - generalized the JointPrior object structure from Matthew Pitkin's...
Toggle commit listadded 21 commits
-
2fb79f6d...1cd1b611 - 16 commits from branch
lscsoft:master - e267cf80 - Merge branch 'master' of http://git.ligo.org/lscsoft/bilby
- 2c0a8138 - add pycharm .idea to gitignore
- 68ca87a6 - Merge branch 'master' of https://git.ligo.org/lscsoft/bilby
- 47ba59cf - Merge branch 'master' of https://git.ligo.org/lscsoft/bilby
- bca9044e - Merge branch 'master' of https://git.ligo.org/bruce.edelman/bilby
Toggle commit list-
2fb79f6d...1cd1b611 - 16 commits from branch
-
Thanks @gregory.ashton ! It looks like it passing all the pipelines now!
added 9 commits
-
bca9044e...8e54566f - 8 commits from branch
lscsoft:master - 69804ef8 - Merge branch 'master' into 'kombine'
-
bca9044e...8e54566f - 8 commits from branch
-
Adding a progress bar would be rather challenging with the current implemenation of the autoburnin method as it is impossible to know how many iterations you will go through unless you hit the max allowed.
tqdmcan run inwhileloops, we do that for ourdynestyprogress bar. I'd be happy to leave this to a future issue though. changed milestone to %0.6.0
mentioned in commit ce6463eb