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

Distributed draws evenly among chains by default #2743

Closed
wants to merge 3 commits into from

Conversation

fonnesbeck
Copy link
Member

This addresses the issue in #2739 where the number of samples drawn does not reflect what was requested. Draws are now evenly distributed among chains. Now, the following occurs, as expected:

In [1]: import pymc3 as pm

In [2]: with pm.Model() as m: x = pm.Normal('x')

In [3]: with m: trace = pm.sample(1000)
Auto-assigning NUTS sampler...
Initializing NUTS using jitter+adapt_diag...
100%|█████████████████████████████████████| 1000/1000 [00:00<00:00, 1273.81it/s]

In [4]: trace['x'].shape
Out[4]: (1000,)

I also removed the sampler warning for chains with <500 samples. If you ask for fewer than 500 samples, you should receive them without generating a warning.

@ColCarroll
Copy link
Member

I like this -- right now, asking for 1000 samples with njobs=4 gives you a progress bar with 1500 iterations, and then the actual trace has 4000 samples.

One corner case is that int(draws / chains) * chains might not be equal to draws. Perhaps that is the place to give a warning?

@junpenglao
Copy link
Member

Is this change really necessary? The current behaviour is similar to Stan, which might help minimize the mental switch for ppl working with both or changing from Stan to PyMC3.

@aseyboldt
Copy link
Member

I guess whether we want to distribute the draws or not depends a bit on how you interpret a call like sample(draws=1000, chains=4). Does that mean: "give me 1000 samples, but generate those in 4 chains", or does it mean "give me 4 chains of 1000 samples each".
I usually think about it the second way, but I can't think of a good reason why one of them would be "right".

A couple of arguments either way:

  • Most of the time people don't specify chains explicitly. So that would favour the first interpretation.
  • Maybe we want people to think about chains, because they are important for convergence checking.
  • The first option suggests that multiple chains are a means to get more samples faster. I don't think this actually works well and should be encouraged, as a huge amount of work goes into initialisation, and for larger models even single chains often are parallel already.
  • traces don't distinguish between chains unless you ask them, so that would also favour the first option.
  • If the number of chains is relatively high, you easily end up running mostly tuning steps. For example, if you ask for 1000 samples in 4 chains, you'd end up doing 4 * 500 + 4 * 250 steps. So two thirds of the samples are discarded, and in many model the tuning steps are even slower than the final steps. I don't think rhat or n_effective would be particularly accurate with only 250 samples per chain. So we'd have to increase the default number of draws and also teach people not to use draws=500 or even draws=1000.

Overall, I like the current behaviour more...

@fonnesbeck
Copy link
Member Author

fonnesbeck commented Dec 4, 2017

I think the critical distinction is that when you ask for n draws (particularly with no other arguments), you get n samples and not 2n, etc. Getting 2000 samples when you ask for 1000 doesn't seem like the right behavior.

It’s true you end up with a lot of tuning with multiple chains, but I don’t think it’s a huge issue given how few tuning steps are needed for most models. I would imagine 2 or 4 chains is what most folks end up using most of the time.

At the end of the day, I think most users just want a certain number of samples, and aren't necessarily concerned with how they got there. All we are using parallel chains for is to calculate diagnostics and to improve the efficiency of sampling.

@fonnesbeck
Copy link
Member Author

How do we want to proceed here? One option is to revert to sampling from one core by default, so that asking for n draws without other arguments gives you n samples. Otherwise, we would need something like this PR to divide them evenly among the cores.

@junpenglao
Copy link
Member

If we are going forward with this change, we should increase the default number of draws.

@twiecki
Copy link
Member

twiecki commented Dec 11, 2017

I'm in favor of merging. If I think about how many samples I want I don't think about the number of chains. We also had the occasional question why multiprocessing wasn't faster, so this would be less confusing in this regard as well.

@fonnesbeck
Copy link
Member Author

This PR is obsolete now; closing.

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

Successfully merging this pull request may close these issues.

5 participants