Tidy up AD interface + remove cleandual! and realpart!. Do not merge. #515
Conversation
This file contains 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
|
The changes look good to me in general. I will take a review on it when it's ready. |
|
Great, I've requested a review from you preemptively. |
|
I've taken the liberty of addressing #450 since it's a one-line fix. |
willtebbutt
changed the title
xukai92
reviewed
Sep 14, 2018
| θ .= Tracker.data.(θ) | ||
|
|
||
| # Return non-tracked gradient value | ||
| return l, ∂l∂θ |
There was a problem hiding this comment.
Do you think it might be good for us to wrapper the result with DiffResults.DiffResult (http://www.juliadiff.org/DiffResults.jl/stable/#Constructing-a-DiffResult-1). And retrieve them later using DiffResults.value and DiffResults.gradient.
There was a problem hiding this comment.
Good idea, but let's do this in a separate PR when incorporating DynamicHMC.
This was referenced Sep 14, 2018
This was referenced Sep 14, 2018
yebai
pushed a commit
that referenced
this pull request
Sep 18, 2018
* Moving over to Documenter.jl groundword. * More documentation cleaning and prep * Moving over to Documenter.jl groundword. * More documentation cleaning and prep * Travis settings updated * Travis fixes * Changed doc/build format to .md * Move to Documenter.jl native HTML builds * Update logo-build.jl * Minor updates as discussed in pull #512 * Moved from Pkg notation to the new Pkg3 notation. * Getting this to serve from `docs/` directory. * Adding `docs/build` files. * Set theme jekyll-theme-cayman * Revert "Set theme jekyll-theme-cayman" This reverts commit 38f5916. * index.md relocation * Commented out unused/index.md * Set up doc/ directory to hold Documenter.jl generated documentation * Testing a second index.md file * Fixing error in make.jl * Changed _config.yml to ignore src/ directory * Small cleanup. * refactoring of compiler * removed data macro * refactoring of tilde macro and fixed minor bug. Observer test broken. * improved comment * changed IS to work with new model function scheme * changed hmc core to work with new model function, changed style accoring to guidelines * changed model function calls * fixing compiler tests * fixed varinfo test * re-added kwarg checking * fixed missing data insertion problem * minor fix * lot of changes, wip * code now uses MacroTools, with a few exceptions * work in progress * more refactoring, solving prior sampling in #446 * removed strange tabs * minor change in particlecontainer test * added fix for multiple return statements * info -> debug * fixed typo * runmodel -> runmodel! * unifying initilistation for unconstrained distributions * changing vi.logp inplace, added some tests * added fix for #475 by implementing a data(Dict, Vector{Symbol}) function * typo * changed default assume behavior to sample from the prior * added more tests for the compiler * changed back to use initialisation instead of prior draws * changed default value of CHUNKSIZE to 10 * Introduce a special algorithm for initializing HMC. * Updates to meet suggestions in #512 * Bug fixes, latex support. * a little cleanup * remove AD auto_tune function and sym_str macro * move io.jl and util.jl from core to utilities * logsum -> logsumexp * remove some unused stuff * remove some type annotation because VarInfo is defined later * fix tests * make logsumexp more efficient * use StatsFuns for logsumexp * clean test REQUIRE * clean up * minor fix * added MacroTools to REQUIRE * Minor fixes + style from meeting * Tidy up AD interface + remove cleandual! and realpart!. (#515) Lots of commits. Some stuff went really wrong. Sorry. * Moving over to Documenter.jl groundword. * Unify AD interface * Use new gradient + rename stuff in hmc_core@ * Let SGHMC and SGLD use reverse-mode * Remove cleandual * More documentation cleaning and prep * Moving over to Documenter.jl groundword. * More documentation cleaning and prep * Travis settings updated * Travis fixes * Changed doc/build format to .md * Move to Documenter.jl native HTML builds * Prevent gradient from mutating it's argument * Minor changes * Fix bug involving types of traces * Resolve a bunch of formatting issues * Some more tidying up * Misc reformatting / removal of redundant code * Fix bug, remove printing, and remove code * Remove redundant code * Fix typo * Refactor leapfrog. * Copy old documentation over * Remove some other stuff that I shouldn't have added * Resolves conflicts with #523 * Small fix. * Small fix. * Update REQUIRE * Fix to move .html files to the gh-pages branch. * Updated .gitignore * Tidying up * Changes for #512. * changed LineNumberNode initialisation, issue #534 * Updated guides to document scalar prior sampling, #544. * Added a 'quick start' page. #512 * Removed git workflow section, added links to more maintained guides. #535 * Update README.md * Better testing for transforms + code which conforms to the style guide (#543) * Test logpdf_with_trans for univariate distributions * Make sure that MvNormal and LogMvNormal tests pass * Adds broken tests * Various other tests + implementation for LogMvNormal * Update Turing.jl * Minor guide updates. * Moving over to Documenter.jl groundword. * Moving over to Documenter.jl groundword. * More documentation cleaning and prep * More documentation cleaning and prep * Travis settings updated * Travis fixes * Changed doc/build format to .md * Move to Documenter.jl native HTML builds * Update logo-build.jl * Minor updates as discussed in pull #512 * Moved from Pkg notation to the new Pkg3 notation. * Getting this to serve from `docs/` directory. * Set theme jekyll-theme-cayman * Revert "Set theme jekyll-theme-cayman" This reverts commit 38f5916. * index.md relocation * Commented out unused/index.md * Set up doc/ directory to hold Documenter.jl generated documentation * Testing a second index.md file * Fixing error in make.jl * Changed _config.yml to ignore src/ directory * Updates to meet suggestions in #512 * Bug fixes, latex support. * Fix to move .html files to the gh-pages branch. * Updated .gitignore * Tidying up * Changes for #512. * Updated guides to document scalar prior sampling, #544. * Added a 'quick start' page. #512 * Removed git workflow section, added links to more maintained guides. #535 * Minor guide updates.
yebai
pushed a commit
that referenced
this pull request
Sep 18, 2018
Lots of commits. Some stuff went really wrong. Sorry. * Moving over to Documenter.jl groundword. * Unify AD interface * Use new gradient + rename stuff in hmc_core@ * Let SGHMC and SGLD use reverse-mode * Remove cleandual * More documentation cleaning and prep * Moving over to Documenter.jl groundword. * More documentation cleaning and prep * Travis settings updated * Travis fixes * Changed doc/build format to .md * Move to Documenter.jl native HTML builds * Prevent gradient from mutating it's argument * Minor changes * Fix bug involving types of traces * Resolve a bunch of formatting issues * Some more tidying up * Misc reformatting / removal of redundant code * Fix bug, remove printing, and remove code * Remove redundant code * Fix typo * Refactor leapfrog. * Copy old documentation over * Remove some other stuff that I shouldn't have added
yebai
pushed a commit
that referenced
this pull request
Sep 18, 2018
* Moving over to Documenter.jl groundword. * More documentation cleaning and prep * Moving over to Documenter.jl groundword. * More documentation cleaning and prep * Travis settings updated * Travis fixes * Changed doc/build format to .md * Move to Documenter.jl native HTML builds * Update logo-build.jl * Minor updates as discussed in pull #512 * Moved from Pkg notation to the new Pkg3 notation. * Getting this to serve from `docs/` directory. * Adding `docs/build` files. * Set theme jekyll-theme-cayman * Revert "Set theme jekyll-theme-cayman" This reverts commit 38f5916. * index.md relocation * Commented out unused/index.md * Set up doc/ directory to hold Documenter.jl generated documentation * Testing a second index.md file * Fixing error in make.jl * Changed _config.yml to ignore src/ directory * Small cleanup. * refactoring of compiler * removed data macro * refactoring of tilde macro and fixed minor bug. Observer test broken. * improved comment * changed IS to work with new model function scheme * changed hmc core to work with new model function, changed style accoring to guidelines * changed model function calls * fixing compiler tests * fixed varinfo test * re-added kwarg checking * fixed missing data insertion problem * minor fix * lot of changes, wip * code now uses MacroTools, with a few exceptions * work in progress * more refactoring, solving prior sampling in #446 * removed strange tabs * minor change in particlecontainer test * added fix for multiple return statements * info -> debug * fixed typo * runmodel -> runmodel! * unifying initilistation for unconstrained distributions * changing vi.logp inplace, added some tests * added fix for #475 by implementing a data(Dict, Vector{Symbol}) function * typo * changed default assume behavior to sample from the prior * added more tests for the compiler * changed back to use initialisation instead of prior draws * changed default value of CHUNKSIZE to 10 * Introduce a special algorithm for initializing HMC. * Updates to meet suggestions in #512 * Bug fixes, latex support. * a little cleanup * remove AD auto_tune function and sym_str macro * move io.jl and util.jl from core to utilities * logsum -> logsumexp * remove some unused stuff * remove some type annotation because VarInfo is defined later * fix tests * make logsumexp more efficient * use StatsFuns for logsumexp * clean test REQUIRE * clean up * minor fix * added MacroTools to REQUIRE * Minor fixes + style from meeting * Tidy up AD interface + remove cleandual! and realpart!. (#515) Lots of commits. Some stuff went really wrong. Sorry. * Moving over to Documenter.jl groundword. * Unify AD interface * Use new gradient + rename stuff in hmc_core@ * Let SGHMC and SGLD use reverse-mode * Remove cleandual * More documentation cleaning and prep * Moving over to Documenter.jl groundword. * More documentation cleaning and prep * Travis settings updated * Travis fixes * Changed doc/build format to .md * Move to Documenter.jl native HTML builds * Prevent gradient from mutating it's argument * Minor changes * Fix bug involving types of traces * Resolve a bunch of formatting issues * Some more tidying up * Misc reformatting / removal of redundant code * Fix bug, remove printing, and remove code * Remove redundant code * Fix typo * Refactor leapfrog. * Copy old documentation over * Remove some other stuff that I shouldn't have added * Resolves conflicts with #523 * Small fix. * Small fix. * Update REQUIRE * Fix to move .html files to the gh-pages branch. * Updated .gitignore * Tidying up * Changes for #512. * changed LineNumberNode initialisation, issue #534 * Updated guides to document scalar prior sampling, #544. * Added a 'quick start' page. #512 * Removed git workflow section, added links to more maintained guides. #535 * Update README.md * Better testing for transforms + code which conforms to the style guide (#543) * Test logpdf_with_trans for univariate distributions * Make sure that MvNormal and LogMvNormal tests pass * Adds broken tests * Various other tests + implementation for LogMvNormal * Update Turing.jl * Minor guide updates. * Moving over to Documenter.jl groundword. * Moving over to Documenter.jl groundword. * More documentation cleaning and prep * More documentation cleaning and prep * Travis settings updated * Travis fixes * Changed doc/build format to .md * Move to Documenter.jl native HTML builds * Update logo-build.jl * Minor updates as discussed in pull #512 * Moved from Pkg notation to the new Pkg3 notation. * Getting this to serve from `docs/` directory. * Set theme jekyll-theme-cayman * Revert "Set theme jekyll-theme-cayman" This reverts commit 38f5916. * index.md relocation * Commented out unused/index.md * Set up doc/ directory to hold Documenter.jl generated documentation * Testing a second index.md file * Fixing error in make.jl * Changed _config.yml to ignore src/ directory * Updates to meet suggestions in #512 * Bug fixes, latex support. * Fix to move .html files to the gh-pages branch. * Updated .gitignore * Tidying up * Changes for #512. * Updated guides to document scalar prior sampling, #544. * Added a 'quick start' page. #512 * Removed git workflow section, added links to more maintained guides. #535 * Minor guide updates.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR does quite a few things. Firstly, it moves
sghmcandsgldover to using the newgradientinterface, and resolves an interesting bug that that threw up.The second important thing is that it improves
gradientby ensuring that it has no side-effects, and cleans up after itself. In particular it ensures that all tracking information is removed fromvi.vals. This enables the satisfactory resolution of #509 and further enables the complete removal ofrealpart, as we're no longer carrying around which appears to concerned precisely the removal of tracking information.There are also a few formatting fixes that I undertook whenever significant modifications were made to a file.
There remain a couple of loose ends to tie up, and I would advise against merging this until #513 is merged anyway to avoid clashes.