ENH Add burn and thin kwargs to sample.

[twiecki]

Closes #1010.

[ColCarroll]

LGTM

[fonnesbeck]

There is never a good reason to thin traces.

[twiecki]

Not even to save memory?

…

On Nov 29, 2016 1:06 PM, "Chris Fonnesbeck" ***@***.***> wrote: There is never a good reason to thin traces. — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#1562 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AApJmNbfu5qzt1YBlGBMg6Vk-8lOoTM2ks5rDBU5gaJpZM4K-qIt> .

[fonnesbeck]

If you are thinning just to save memory, then you should not have sampled as much in the first place. Almost always (in fact, I can't think of any other use), thinning is performed to deal with autocorrelation. All that autocorrelation does (in a converged chain) is reduce your effective sample size. So, thinning is doing exactly what autocorrelation is doing -- reducing your sample size. If you want to reduce autocorrelation, you need to change your model, change your sampler, or both. Thinning is of no benefit.

[fonnesbeck]

I'm definitely not a fan of adding two condition checks at every iteration to add a feature we already had.

[twiecki]

OK let's revert the PR for now.

…

On Nov 29, 2016 1:50 PM, "Chris Fonnesbeck" ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In pymc3/sampling.py <#1562 (review)>: > @@ -265,8 +276,9 @@ def _iter_sample(draws, step, start=None, trace=None, chain=0, tune=None, if i == tune: step = stop_tuning(step) point = step.step(point) - strace.record(point) - yield strace + if (i % thin == 0) and (i >= burn): I'm definitely not a fan of adding *two* condition checks at every iteration to add a feature we already had. — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#1562 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AApJmDlo2QO5zr8SrfYAPYZaHRImsJqbks5rDB-kgaJpZM4K-qIt> .