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

major rewrite #44

Merged
merged 48 commits into from
Aug 12, 2019
Merged

major rewrite #44

merged 48 commits into from
Aug 12, 2019

Conversation

tpapp
Copy link
Owner

@tpapp tpapp commented Apr 11, 2019

Addressing many issues, mostly as outlined in #30.

Organized as a set of smaller PRs, merged into this one.

API changes

  • major API change: entry point is now mcmc_with_warmup

  • refactor warmup code, add initial optimizer

  • use the LogDensityProblems v0.9.0 API

  • use Julia's Logging module for progress messages

  • diagnostics moved to DynamicHMC.Diagnostics

    • report turning and divergence positions

    • add leapfrog_trajectory for exploration

Implementation changes

  • factor out the tree traversal code

    • abstract trajectory interface

    • separate random and non-random parts

    • stricter and more exact unit tests

  • refactor Hamiltonian code slightly

    • caching is now in EvaluatedLogDensity

    • functions renamed

  • misc

    • remove dependency on DataStructures, Suppressor

    • cosmetic changes to dual averaging code

    • large test cleanup

TODO

Warmup and diagnostics postponed after this PR, see #54.

Hopefully the new registry will be ready by this PR is done.
@codecov
Copy link

codecov bot commented Apr 11, 2019

Codecov Report

Merging #44 into master will decrease coverage by 7.27%.
The diff coverage is 88.65%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #44      +/-   ##
==========================================
- Coverage   97.21%   89.94%   -7.28%     
==========================================
  Files           7        8       +1     
  Lines         287      348      +61     
==========================================
+ Hits          279      313      +34     
- Misses          8       35      +27
Impacted Files Coverage Δ
src/DynamicHMC.jl 100% <ø> (ø) ⬆️
src/diagnostics.jl 100% <100%> (ø) ⬆️
src/hamiltonian.jl 100% <100%> (ø) ⬆️
src/NUTS.jl 100% <100%> (ø)
src/stepsize.jl 98.59% <100%> (+5.83%) ⬆️
src/reporting.jl 12% <12.5%> (-79.18%) ⬇️
src/trees.jl 87.5% <87.5%> (ø)
src/mcmc.jl 93.75% <93.75%> (ø)
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4ca0caa...ed94519. Read the comment docs.

tpapp added 27 commits April 11, 2019 14:32
- Move all abstract trajectory API and samplers into trees.jl.

- Use bit flags for directions.
Internal checks: rule out calling is_turning on a leaf.
Remove proposal type, factor out the logic of biased progressive sampling.
Rewrite unit tests with log probabilities.
Incidentally: cosmetic cleanup of test code.
- make kinetic energy the first slot/type parameter in Hamiltonian

- longer function, slot, and type names, without abbreviations

- fix LogDensityProblems API use bugss

- rename neg_energy to logdensity

- support diagonal metric without forming a lower triangular W

- Hamiltonian tests and support functions cleanup, random z in rand_Hz

- get rid of some cruft in tests
Rename structs and internal functions.
No substantial code changed, just formatting, moved diagnostic
functions to appropriate file.
1. store position with evaluated log density and gradient in a single
object

2. remove redundant Hamiltonian functionality

3. mcmc stages with adaptation state replace tuning sequence

4. add optimization before initial stepsize finding initial stepsize

5. add diagonal metric (as the default)

6. simplify μ initialization

7. allow higher acceptance rate for initial stepsize
- replace `sample` with `chain`
- replace `adaptation` with `warmup`
- add missing docstrings
- rename higher-level functions
- add API change warnings
also factor out common test code for interactive use
as we are not testing mcmc logs
- propagate invalid tree information upwards and save it for
diagnostics, refactor tree code

- just collect visited node information (practically acceptance),
without divergence

- use log of total acceptance rate for more accurate calculation

- refactor diagnostics: don't use DataStructures for simple counting,
rename statistics summary function, change quantiles

- fix reporter propagation bug

- longer runs for simple mcmc diagnostics (fewer false positives)

- incidental docstring fixes
(may bring it back when tested)
@tpapp
Copy link
Owner Author

tpapp commented Aug 12, 2019

PR already huge, merging to master, TBC.

@tpapp tpapp merged commit d315cc2 into master Aug 12, 2019
@tpapp tpapp deleted the tp/major-api-rewrite-2.0 branch August 14, 2019 07:19
tpapp added a commit that referenced this pull request Aug 20, 2019
- bump Documenter project version
- updated documentation for #44 and subsequent changes
- don't document low-level API
- include a worked example
- deploy docs using 1.2
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

Successfully merging this pull request may close these issues.

create a DOI adapt a diagonal mass matrix by default unit tests for posterior
1 participant