Adding a custom dynesty sampler
This implements a locally-defined dynesty rwalk sampler. The initial motivation to do this is we believe dynesty has a bug in the case when "None" boundaries are used. We will, after testing put this fix back into dynesty itself so that other members of the community can benefit. But, it is also useful to be able to fiddle in the guts of the sampler!
For future reference, the changes in this MR are largely procedural. Except, the line
while nc + nfail < walks or accept == 0:In dynesty, this line is
while nc < walks or accept == 0:The change was suggested by @colm.talbot. My interpretation is that, without adding the nfail, the times when the sampler walks outside the bounds aren't counted causing a bias.
What will this effect?
This will effect any runs where the boundary=None for one of the priors. In the past, we have recommended people always use boundary='reflective' because of the odd behaviour we saw when using no prior boundary. However, the demonstration below suggests that this was in fact just buggy behaviour.
Demonstration for GW150914
The following runs are done using a mass_ratio prior with None boundary. Note that runs done as part of the initial bilby review where done with a reflective prior. It is likely these suffered with this issue at some level, but it was perhaps not so clear due to large numbers of walks etc.
Here is the mass parameters for GW150914 with the old behaviour:
Here is the mass parameters for GW150914 with the new behaviour:
The mass ratio here rail, but smoothly against the prior, in much better agreement with runs for LALInference.
Merge request reports
Activity
changed milestone to %0.6.0
added Sampling label
mentioned in commit a61ded22
mentioned in merge request !643 (merged)