BUGFIX: fix random number generation with parallelization

This MR fixes a fairly long-standing bug with how we use parallelization, see, e.g., here for a related discussion. Currently, we naively use the np.random, this has been discouraged for a while in favour of a generator-based approach as the same seed is used across all spawned parallel processes. This means that we get essentially the same stream of random numbers for Bilby operations in parallel threads.

There are quite a lot of small changes here, basically, I changed every instance of np.random to use a new generator and added a small extra module to facilitate this.

This does not impact dynesty sampling as that has it's own parallel rng handling.

This may impact bilby_mcmc, I haven't checked in detail (@gregory.ashton).

This is responsible for weird behaviour we've seen a lot recently with optimal SNR and distance having posterior chains that look like

The bunching is because the same random number is used in the marginalized parameter reconstruction for each block of npool calls. With the changes here the same job becomes (the difference in the number of samples is due to a different inconsistent use of random seeds)

This means that for any event where the distance traces look like the first the sky localization will be unreliable as we have a much smaller effective sample size.

The same feature in the optimal SNR is because of the same effect.

changed milestone to %2.2.0

added >100 lines Bug labels

approved this merge request

added 1 commit

• 79b57f38 - PRECOMMIT: precommit fixes

Compare with previous version

added 1 commit

• 12cb3cf3 - EXAMPLE: fix seed settings

Compare with previous version

@colm.talbot so just to understand: it doesn't effect the dynesty sampling, but will effect current PE runs in the reconstruction? If so, do we need to create a separate hot fix release?

With regards to Bilby-MCMC it will effect the reconstruction in the same way. It will also affect the sampling, but because every parallel process is on a different temperature I think that the sampling can't be biased, but could be less efficient (it doesn't break details balance in the swaps between the two lowest temperature chains).

Overall looks good to me: I checked the replacements all made sense. My only remaining question is: do we need to set the seed for each chain in bilby-mcmc, or is that not handled by the new rng?

I'm working on fixes for the failing tests so this isn't ready for approval quite yet, sorry for jumping the gun.

added 1 commit

• 89c4d721 - TEST: update tests to use new rng method

Compare with previous version

added 1 commit

• 612174fe - TYPO: fix indentation

Compare with previous version

added 2 commits

• 2c777b0e - CI: remove py3.8 testing
• 36fc69ea - TEST: allow almost equal for hardware differences

Compare with previous version

approved this merge request

approved this merge request

resolved all threads

mentioned in commit 089401e9

merged

changed milestone to %2.1.2

picked the changes into the branch release/2.1.x with commit 2eb13904

mentioned in commit 2eb13904