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

initialize chains from estimated posterior samples #1655

Merged
merged 3 commits into from
Jan 18, 2017

Conversation

aloctavodia
Copy link
Member

For advi and nuts and when njobs > 1 it will sample starting points from the estimated posterior, otherwise it will use the estimated posterior mean.

if njobs > 1:
start = pm.variational.sample_vp(v_params, njobs, progressbar=False, hide_transformed=False)
else:
start = v_params.means
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we always want a sample from the posterior to start in the "typical set". The mean can be far away from that, which is counter-intuitive, but true for high-dimensional models: https://www.youtube.com/watch?v=pHsuIaPbNbY

@aloctavodia
Copy link
Member Author

aloctavodia commented Jan 7, 2017 via email

@twiecki
Copy link
Member

twiecki commented Jan 7, 2017

ok. I will revert that. I guess nuts (when jobs = 1) should also start
from a posterior sample and not from the mean of the posterior, right?

Exactly.

@aloctavodia
Copy link
Member Author

That's right, but the discarded values will only affects the starting points. Then it will use the whole array to do the actual sampling.

@aloctavodia
Copy link
Member Author

@twiecki @ColCarroll any (new) thoughts on this?

@ColCarroll
Copy link
Member

Ah, just checked, and advi's random_seed implementation uses None as a default. I also think it might cause trouble using MRG_RandomStreams to set the seed (have to check whether that shares state with numpy.random). There might also be another subtle bug, but I will open a separate issue for that.

If you want to land this now, we'll have to remember to change this when the random_seed fix lands.

@ColCarroll
Copy link
Member

(so, this all looks good, but I think there's a subtle problem elsewhere!)

@ColCarroll
Copy link
Member

Oh, heh, #1656 is the problem!


start = {varname: np.mean(init_trace[varname]) for varname in init_trace.varnames}
init_trace = pm.sample(step=pm.NUTS(), draws=n_init,
random_seed=random_seed)[n_init//2:]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wrong indent.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aloctavodia once these minor issues are fixed we can start to merge this I think.

cov = np.power(model.dict_to_array(v_params.stds), 2)
elif init == 'advi_map':
start = pm.find_MAP()
v_params = pm.variational.advi(n=n_init, start=start)
v_params = pm.variational.advi(n=n_init, start=start,
random_seed=random_seed)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wrong indent

@twiecki twiecki merged commit b6827a6 into pymc-devs:master Jan 18, 2017
@twiecki
Copy link
Member

twiecki commented Jan 18, 2017

Thanks @aloctavodia!

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

Successfully merging this pull request may close these issues.

3 participants