BradGreig / Hybrid21CM

1 stars 3 forks source link

Ensure samples are always within ranges for efficiency #20

Closed steven-murray closed 5 years ago

steven-murray commented 5 years ago

Current behaviour is that if a parameter is sampled outside its defined range (in MCMC), it will quickly return -inf. It is apparent that this may not be optimal, due to load-balancing: if it returns much more quickly than other processes, it sits idle while other processes do the processing. What's more, its return value does not give a great deal of information to the sampler, so it could be seen as semi-wasted cycles.

To fix this behaviour, a number of ideas come up:

  1. Force the sampler to pick only values in the range, by re-sampling until every walker is inside the range. The upsides are that it is conceptually simple, and I think it is consistent with the algorithm (i.e. nothing should go wrong, as long as it is re-sampled independently each time, until it's in the range). The downside is that it requires adjustments to the EnsembleSampler object from emcee. From the looks of it, only one method will need to be overwritten, maybe two (if you include the __init__), but still, this is not ideal for future upgrades etc.
  2. Force the sampler to pick values in the range via a co-ordinate transform (ala Stan). The upsides here are that I know it is legitimate, and that nothing needs to be hacked. The downsides are that it involves a little more mathematical complexity.
  3. Attempt to fix the problem by enhancing the load balancing rather than the sampling. As it stands, it seems that the default behaviour is to dynamically assign tasks to processes, which means that a quickly-returning task should not be too much of a problem, unless of course you randomly end up in the situation that 1 or 2 extra calls have to be done at the end as "stragglers" fairly consistently.

Given that (3) seems to be an "okay" solution, I think it is fine to basically leave it as-is for a while. Any thoughts on a long-term solution?

BradGreig commented 5 years ago

I can see plenty of issues with (3), as it is difficult to properly load balance the specific algorithm. Just the way it works.

I definitely would prefer a solution to (1). What about trying to submit an issue/pull request including the exact behaviour we need? That way we can at the very least be future proofed.

steven-murray commented 5 years ago

OK. I'll do (1). I don't think this particular issue will fit the goals of emcee -- it has no concept of parameter ranges, and leaves it up to the user to deal with that. Putting that in will open up pandora's box for them, so I doubt they'll go for it. I can subclass it. Hopefully this will be "reasonably" future-proof because it's only modifying a single method.

steven-murray commented 5 years ago

This is done in 5cca1eb3548bc483e94b2ef2f84f40628b8af354. However, I don't really know how to test it (other than running previous MCMC tests, which do run through fine). Any ideas?

BradGreig commented 5 years ago

When I can I will look into it and see how it is functioning etc. I agree outside of running previous examples there isn't much more that can be done. Just verifying that all threads are being used should be reasonable.

We should probably explain that we are doing this somewhere in the docs.

steven-murray commented 5 years ago

Cool. Yeah I'll add it to my to-do list 🙂.

On Tue, Dec 11, 2018, 4:04 PM BradGreig <notifications@github.com wrote:

When I can I will look into it and see how it is functioning etc. I agree outside of running previous examples there isn't much more that can be done. Just verifying that all threads are being used should be reasonable.

We should probably explain that we are doing this somewhere in the docs.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/BradGreig/Hybrid21CM/issues/20#issuecomment-446396584, or mute the thread https://github.com/notifications/unsubscribe-auth/ABNo3rFHXNjsb_zUCI5LRaPq_ORRhiDXks5u4Dn5gaJpZM4ZMbzQ .

steven-murray commented 5 years ago

Done in 58c2597f2de7f9fe7e454e2cc305c140f35cde69.