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

Print warnings for some bad traces #2002

Merged
merged 2 commits into from Apr 7, 2017

Conversation

Projects
None yet
5 participants
@aseyboldt
Copy link
Member

aseyboldt commented Apr 6, 2017

Right now we don't report anything if a trace if obviously nonsense. This PR prints warnings if a trace does not meet some minimum requirements.
The test about the target acceptance probability is a bit hairy, I don't know any obvious choices for the threshold. The one I implemented works out to printing a warning if:

  • If the target accept is 0.99, fail if mean accept is not between 0.96 and 0.9997.
  • If target_accept = 0.8, fail if mean accept is not between 0.72, 0.87.
    I'm open for better ideas...
@ColCarroll

This comment has been minimized.

Copy link
Member

ColCarroll commented Apr 6, 2017

This looks really useful. Neal (https://arxiv.org/pdf/1206.1901.pdf) has some calculations for optimal acceptance rate for Metropolis and HMC/NUTS on page 30. Not sure how wide the band should be around that...

@aseyboldt aseyboldt force-pushed the aseyboldt:check-stats branch from 0f6107c to 99dcccb Apr 6, 2017

@twiecki

This comment has been minimized.

Copy link
Member

twiecki commented Apr 7, 2017

Very useful. Seems like there are some legit test errors.

@aseyboldt aseyboldt force-pushed the aseyboldt:check-stats branch from 99dcccb to 8576e04 Apr 7, 2017

@springcoil

This comment has been minimized.

Copy link
Member

springcoil commented Apr 7, 2017

@aseyboldt

This comment has been minimized.

Copy link
Member

aseyboldt commented Apr 7, 2017

I guess this also means we need a way to pass parameters to nuts though pm.sample. It's a bad idea to tell users to increase target_accept, but to do this you need to manually specify your step methods and automatic initialization with advi doesn't work anymore.

@twiecki

This comment has been minimized.

Copy link
Member

twiecki commented Apr 7, 2017

@aseyboldt Yeah, I had this on my mind before, should definitely pass kwargs. Maybe best way is to have step_method_kwargs and init_kwargs (or advi_kwargs?).

@junpenglao

This comment has been minimized.

Copy link
Member

junpenglao commented Apr 7, 2017

I guess this also means we need a way to pass parameters to nuts though pm.sample. It's a bad idea to tell users to increase target_accept, but to do this you need to manually specify your step methods and automatic initialization with advi doesn't work anymore.

@aseyboldt isn't that always an option https://github.com/pymc-devs/pymc3/blob/master/pymc3/sampling.py#L491 ?

@aseyboldt

This comment has been minimized.

Copy link
Member

aseyboldt commented Apr 7, 2017

@junpenglao Yes, but this does not work through pm.sample.

@aseyboldt

This comment has been minimized.

Copy link
Member

aseyboldt commented Apr 7, 2017

See #2004

@twiecki twiecki merged commit 856d675 into pymc-devs:master Apr 7, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.04%) to 86.566%
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment