-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Implement iterative NUTS algorithm #1381
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
Conversation
2432f4b to
2299a4c
Compare
springcoil
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes look good to me after a first pass
|
It would be great to have a PDF version written up somewhere, as the recursive implementation has been a "barrier to entry" for myself and several colleagues. |
|
That failure looks funny, and might have more to do with conda than with anything else. It passed on my branch and when I reinstalled things locally -- I'll see if force pushing my branch will kick off another build that fixes it. @kyleabeauchamp -- I'll type up the notes I made this weekend, which might also help to find any problems! |
2299a4c to
7a24f66
Compare
|
apparently it is a thing! i'll look into it -- we should probably be pinning all the versions in conda. |
|
These conda version issues are pretty annoying. Maybe this is now fixed already on their end? |
|
Wow this error is annoying :) |
7a24f66 to
04bd9a6
Compare
|
Ok I'll merge this now - great work @ColCarroll I look forward to that IPyNB or PDF write up or blog post (however you wish to do it) on how this algorithm actually works. I've not seen it anywhere else. |
This seems more readable, and perhaps modestly more efficient than the recursive implementation. As far as I can tell, it is also novel: the original c++ and matlab versions, as well as versions in julia, python, and R are all recursive (and use the original notation).
It passes all existing tests (except this
test_parallel_startone?), but would appreciate another look.