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

Type design for PhasePoint and DualValue. #51

Merged
merged 56 commits into from
May 20, 2019
Merged

Conversation

yebai
Copy link
Member

@yebai yebai commented Apr 16, 2019

This PR aims at introducing some additional types that are discussed in #16. More specifically, the following types are introduced:

  • PhasePoint: stores θ, r and cached Potential energy (and its gradient). Consider introduce the PhasePoint type #17
  • DualValue: stores log density logπ(θ), and cache its gradient.
  • LogDensityFunction: stores logπ and its gradient function ∂logπ∂θ.

TODOs

  • Refactor numerical error handling code leapfrog using PhasePoint
  • Remove DualFunction?
  • Refactor build_tree using PhasePoint
  • Return more information for each step Return more information for each step #59
  • Add a Termination type.

@yebai yebai requested a review from xukai92 April 16, 2019 20:58
src/hamiltonian.jl Outdated Show resolved Hide resolved
src/hamiltonian.jl Outdated Show resolved Hide resolved
src/hamiltonian.jl Outdated Show resolved Hide resolved
src/hamiltonian.jl Outdated Show resolved Hide resolved
@xukai92
Copy link
Member

xukai92 commented Apr 19, 2019

I updated the version in Project.toml from v0.1.2 to v0.1.3. You may want to merge with the master again.

Update: and an interface bug (f8226a7)

@xukai92
Copy link
Member

xukai92 commented Apr 21, 2019

Do you plan to address the problem of returning H_new for NUTS in this PR?

@yebai
Copy link
Member Author

yebai commented Apr 21, 2019

Do you plan to address the problem of returning H_new for NUTS in this PR?

yes, this will be addressed in this PR since PhasePoint caches H and its gradients.

@xukai92
Copy link
Member

xukai92 commented Apr 22, 2019

So for the purpose of #59, maybe we should consider here somehow make it flexible to return more information within a single step?

@yebai yebai changed the title [WIP]: Draft type design for PhasePoint and LogDensity. Draft type design for PhasePoint and LogDensity. May 17, 2019
@xukai92
Copy link
Member

xukai92 commented May 17, 2019

Hi Hong, when can I do a review?

@yebai
Copy link
Member Author

yebai commented May 17, 2019

It's ready for a look!

@yebai yebai changed the title Draft type design for PhasePoint and LogDensity. Draft type design for PhasePoint and DualValue. May 17, 2019
src/hamiltonian.jl Outdated Show resolved Hide resolved
src/hamiltonian.jl Show resolved Hide resolved
src/integrator.jl Show resolved Hide resolved
src/sampler.jl Show resolved Hide resolved
src/trajectory.jl Show resolved Hide resolved
src/hamiltonian.jl Outdated Show resolved Hide resolved
test/hamiltonian.jl Outdated Show resolved Hide resolved
test/hamiltonian.jl Outdated Show resolved Hide resolved
@xukai92
Copy link
Member

xukai92 commented May 17, 2019

I commented on some places for discussion. Please take a look!

@yebai yebai force-pushed the hg/some-type-refactoring branch from 4d9389c to be734f0 Compare May 18, 2019 19:17
@yebai
Copy link
Member Author

yebai commented May 18, 2019

Ready for another look! @xukai92

@xukai92
Copy link
Member

xukai92 commented May 20, 2019

Looks great to me! Just wonder what's "bugfix" in the commit message of 435ac08 mean? I didn't see bugfix?

@yebai
Copy link
Member Author

yebai commented May 20, 2019

Looks great to me! Just wonder what's "bugfix" in the commit message of 435ac08 mean? I didn't see bugfix?

It refers to the leftover logπ (should be ℓπ).

@yebai
Copy link
Member Author

yebai commented May 20, 2019

Ready to merge from my side @xukai92

@yebai yebai changed the title Draft type design for PhasePoint and DualValue. Type design for PhasePoint and DualValue. May 20, 2019
@xukai92 xukai92 merged commit 5b9e7b7 into master May 20, 2019
@xukai92
Copy link
Member

xukai92 commented May 20, 2019

Merged! Thanks for your great work!

@yebai yebai deleted the hg/some-type-refactoring branch May 20, 2019 22:10
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.

2 participants