-
Notifications
You must be signed in to change notification settings - Fork 6
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
Chain ranges differ #30
Comments
Chris, earlier there was some mention of a change in Turing dropping the warmup samples. I don't think that has happened yet. Could that be the culprit? |
Hey Rob, I can confirm that Turing v0.6.18, the newest version, automatically excludes warmup samples. Kai added a fix to reflect that change. So my best guess is that I made in error the the logic, which only became noticeable once the iterations were changed. |
Ah, good to know and in that case I also need to update the TuringModels.jl repo. Today I wanted to setup the docs framework for MCMCBenchmarks. |
The problem was that I was only changing 1 of the 2 fields in the CmdStan configuration object for num_samples and num_warmup, which caused the chain range problem. In the process of debugging, I ran a new benchmark and saw that Turing is closing the gap on AdvancedHMC : See the old benchmark for comparison. It looks like Turing still has more memory allocations: |
Very nice improvement! Should we drop the dynamicNUTS option? I don”t it will ever be mainstream and if it is not maintained by the Turing team it will be hard to get it reliably working. |
I agree. Dropping DynamicNUTS is a good idea. I'll make an issue and remove it over the weekend. |
Strange. This might be related to this issue with CmdStan. I'll try to look into this further by the weekend.
My best guess is that I made an error in excluding adaption trials or the chain object has the wrong range because something changed in Turing or CmdStan.
Here is the error from @xukai92:
The text was updated successfully, but these errors were encountered: