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

NUTS adapatation samples appear in full chain #635

Closed
cpfiffer opened this issue Dec 29, 2018 · 7 comments
Closed

NUTS adapatation samples appear in full chain #635

cpfiffer opened this issue Dec 29, 2018 · 7 comments
Assignees

Comments

@cpfiffer
Copy link
Member

cpfiffer commented Dec 29, 2018

It looks like the samples from the NUTS adaptation phase are being included in the full chain. Is this expected behavior? If not, we need some kind of infrastructure in place to separate out the adaptation samples from the chains so that parameter estimates are more appropriate.

Let me know if this needs to be moved over to MCMCChains.

@goedman
Copy link

goedman commented Jan 14, 2019

The links are no longer correct.

@trappmartin
Copy link
Member

I think it's not expected behaviour. I'm currently looking into the NUTS code. I'll create a PR on this and on some minor performance improvements.

@trappmartin trappmartin self-assigned this Feb 5, 2019
@yebai yebai mentioned this issue Feb 19, 2019
56 tasks
@yebai
Copy link
Member

yebai commented May 8, 2019

@kai @cpfiffer Has this been fixed by recent changes in AHMC and MCMCChains?

@cpfiffer
Copy link
Member Author

cpfiffer commented May 9, 2019

No, MCMCChains still does not have any internals to catch adaptation samples. I'm wondering if we should fix this on the Turing side by defaulting to only return the post-warmup samples, and discarding the adaptation samples unless the user specifically requests them (using a keyword like NUTS(. . . , keep_warmup=true) or something). Does that seem reasonable? Is there any reason to keep the warmup samples around if most people just get rid of them?

@robsmith11
Copy link

As a beginner attempting to reproduce the resuts of a simple linear model from Stan, I was surprised to the see the warmup samples included in the summary stats. I expected them to have been removed.

@yebai
Copy link
Member

yebai commented May 12, 2019

Excluding warmup samples from chains sounds reasonable. In the longer term, we might need an argument for the sample interface (#746) to decide to retain/discard warmup samples.

@cpfiffer
Copy link
Member Author

I've opened up a PR Turing-side to fix this: #784

@yebai yebai closed this as completed May 12, 2019
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

No branches or pull requests

5 participants