Refactor HMC #1571
Closed
Refactor HMC #1571
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…tting (e.g. nanguardmode) for Theano functions
* Started to write Base class for pymc3.models * mode `add_var` to public api * Added some docstrings * Added some docstrings * added getitem and fixed a typo * added assertion check * added resolve var method * decided not to add resolve method * Added linear component * Docs fix * patsy's intercept is inited properly now * refactored code * updated docs * added possibility to init coefficients with random variables * added glm * refactored api, fixed formula init * refactored linear model, extended acceptable types * moved useful matrix and labels creation to utils file * code style * removed redundant evaluation of shape * refactored resolver for constructing matrix and labels * changed error message * changed signature of init * simplified utils any_to_tensor_and_labels code * tests for `any_to_tensor_and_labels` * added docstring for `any_to_tensor_and_labels` util * forgot to document return type in `any_to_tensor_and_labels` * refactored code for dict * dict tests fix(do not check labels there) * added access to random vars of model * added a shortcut for all variables so there is a unified way to get them * added default priors for linear model * update docs for linear * refactored UserModel api, made it more similar to pm.Model class * Lots of refactoring, tests for base class, more plain api design * deleted unused module variable * fixed some typos in docstring * Refactored pm.Model class, now it is ready for inheritance * Added documentation for Model class * Small typo in docstring * nested contains for treedict (needed for add_random_variable) * More accurate duplicate implementation of treedict/treelist * refactored treedict/treelist * changed `__imul__` of treelist * added `root` property and `isroot` indicator for base model * protect `parent` and `model` attributes from violation * travis' python2 did not fail on bad syntax(maybe it's too new), fixed * decided not to use functools wrapper Unfortunately functools wrapper fails when decorating built-in methods so I need to fix that improper behaviour. Some bad but needed tricks were implemented * Added models package to setup script * Refactor utils * Fix some typos in pm.model
Member
Author
|
Ah frig, I think we need to rebase 3.1 off master, then I will rebase this. Happy for thoughts in the meantime. |
ColCarroll
force-pushed
the
refactor_hmc
branch
from
379754c to
d6f57a7
Compare
twiecki
reviewed
Dec 3, 2016
pymc3/step_methods/hmc/base_hmc.py
Outdated
| """ | ||
| Parameters | ||
| ---------- | ||
| vars : list of theano variables |
Member
There was a problem hiding this comment.
Don't need to indent that level.
Member
|
I skimmed over it, looks like a good refactor. Seems like you didn't touch much of the math, correct? |
Member
Author
|
No math changes -- I brought a test case in from master to make sure the sampling stayed exactly the same. The only change an end user will see from this is the performance speedup in |
Member
|
Sorry, I rebased the 3.1 branch off of master and resolved a conflict, looks like you'll have to rebase this one. |
Member
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Started refactoring Hamiltonian methods into a common base class. This lets
HamiltonianMCshare some theano machinery from NUTS, and provides a ~2x speedup using the benchmark from #1570. The same PR also shows that I haven't changed either algorithm at all.My larger plan is to use the approach from @betanalpha's exhaustive monte carlo paper (#1497) to provide different theano functions for expanding trajectory, checking trajectory, and checking termination so that HMC, NUTS, and XHMC can share (optimized) machinery, and perhaps there can be easy experiments with further mixing of methods. I'm hoping to have a PR soon where
BaseHMCimplements algorithm 2 from the paper asastep, so that the implementation ofNUTSandHamiltonianMCis only an__init__which prepares the appropriate functions.Also, note that this will be merged into 3.1, not 3.0.