Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor step methods to use their own random stream #5797

Closed
Tracked by #7053
lucianopaz opened this issue May 24, 2022 · 2 comments · Fixed by #7508
Closed
Tracked by #7053

Refactor step methods to use their own random stream #5797

lucianopaz opened this issue May 24, 2022 · 2 comments · Fixed by #7508

Comments

@lucianopaz
Copy link
Contributor

Description of your problem

This issue is a spin off from a discussion in #5787. At the moment of that PR, our step methods rely on the global numpy random state to generate random numbers. It would be great to modify the step methods to allow them to have their own random generator that could then be used to generate all of their random number proposals.

Surprisingly, metropolis has some proposal distributions that could in principle take a random stream, the problem is that the actual steppers don't expose an easy to access random stream generator argument for users (or pm.sample) to set. HMC on the other hand is plagued with calls to numpy.random.some_method

@ricardoV94
Copy link
Member

Yeah the metropolis RNG interface was added so that SMC sampling did not rely on global seeding in #5131.

@Armavica
Copy link
Member

I had a look at this issue and I have several questions about it:

  • Work has started on this functionality in Use Random Generator in SMC instead of global seeding #5131, creating two API points to specify a local prng for the Proposal distributions: at __init__ where a random_seed can be specified, and at __call__ where a rng can be passed. Are these two options necessary in principle, or could we just do with one of them?
  • This implementation has been partially reverted in Partially revert #5131 #5135 because it made some tests flaky. Would it be acceptable to make the incriminated tests fix the local rng instead of the global one in order to un-revert the original PR and build from there?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants