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

RFC find_good_eps #27

Closed
xukai92 opened this issue Mar 24, 2019 · 2 comments
Closed

RFC find_good_eps #27

xukai92 opened this issue Mar 24, 2019 · 2 comments
Milestone

Comments

@xukai92
Copy link
Member

xukai92 commented Mar 24, 2019

Copied from @yebai comment in #23 (comment)

    r = rand_momentum(rng, h)
    H = hamiltonian_energy(h, θ, r)

out of find_good_eps and wraping

        θ′, r′, _is_valid = step(Leapfrog(ϵ′), h, θ′, r′)
        H_new = _is_valid ? hamiltonian_energy(h, θ′, r′) : Inf

into a function

function A(ϵ, h, θ, r) 
        θ′, r′, _is_valid = step(Leapfrog(ϵ′), h, θ′, r′)
        return H_new = _is_valid ? hamiltonian_energy(h, θ′, r′) : Inf
end

Then, we can drop the dependency on AdvancedHMC from this function: find_good_eps(h::Hamiltonian, θ::AbstractVector{T}, A::Function; max_n_iters::Int=100), and move it into adaption/stepsize.jl.

@yebai yebai added this to the Release v0.2 milestone May 14, 2019
@yebai yebai mentioned this issue May 17, 2019
10 tasks
@xukai92
Copy link
Member Author

xukai92 commented Jul 6, 2019

I refactored it anyway (in 6cd458d) but I don't see any reason we have to drop the dependency any more. Plus it still depends on Hamiltonian any way?

xukai92 added a commit that referenced this issue Jul 10, 2019
* Initial design for new `Trajectory` and `Termination` types.

* fix some type definitions

* rename NUTS_Termination to NoUTurnTermination

* introduce DoublingTree type

* fix RFC typo

* initial DoublingTree impl

* add a isUturn function

* add a comment

* improve NUTS type

* improve abstraction

* bugfix for MH step and add more comments (#75)

* draft a docstring for the sample function (#75)

* RFC find_good_eps (#27) and add some more comments (#75)

* add some preparation code

* indicate the numerical error location (#71)

* Re-organise code to improve readability - no functionality change.

* Re-organise code to improve readability - no functionality change.

* remove replace sampling with samplerType

* revert naming

* improve test script

* not use find_good_eps for precondition only adaptation

* improve function names in tests

* breaking change: passed-in grad function is now suppoed to return a tuple of value and gradient

* bugfix

* actually use the cache mechanism; almost 2x speed-up

* add DiffResults to test deps

* improve comments and naming

* update packages in env

* Code sytle updates - no functionality change.

* Code sytle updates - no functionality change.

* Unify `merge` function via kwargs - no functionality change.

* Rename `sample` ==> `combine` - no functionality change.

* Rename `merge` ==> `combine` - no functionality change.

* Renamed `isUturn` ==> `isturn`, `iscontinued` ==> `isdivergent` - no functionality change.

* Make `∂H∂θ(h, θ)` return `DualValue` istead of `Tuple`.

* make rng the first argument of combine if used

* update README.md

* double sample numbers in test

* Update README.md
@xukai92
Copy link
Member Author

xukai92 commented Jul 10, 2019

Closed by PR #79.

@xukai92 xukai92 closed this as completed Jul 10, 2019
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

No branches or pull requests

2 participants