Skip to content

Better documentation #417

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

Merged
merged 4 commits into from
Apr 4, 2025
Merged

Better documentation #417

merged 4 commits into from
Apr 4, 2025

Conversation

ErikQQY
Copy link
Collaborator

@ErikQQY ErikQQY commented Apr 1, 2025

Fix: #330
Fix: #413
Fix: #402
Replace #414

Preview can be checked in https://turinglang.org/AdvancedHMC.jl/previews/PR417

@ErikQQY ErikQQY requested review from mhauru, sunxd3 and sethaxen April 1, 2025 18:28
@yebai
Copy link
Member

yebai commented Apr 1, 2025

Thanks @ErikQQY. I suggest that we

  • move Interface news and API changes to doc or a separate markdown file too
  • keep a worked example in the README file based on AbstractMCMC.sample for illustration purposes and point the audience to relevant docs for more advanced use

Copy link
Member

@mhauru mhauru left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. I know this is mostly moving stuff around, but I took the opportunity to read through the docs as well, so I had a few small requests.

Copy link
Member

@sunxd3 sunxd3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some nitpicking.

Also, the reference list at https://turinglang.org/AdvancedHMC.jl/previews/PR417/#References and https://turinglang.org/AdvancedHMC.jl/previews/PR417/references/ are not identical. Not sure if we want to unify them.

@ErikQQY ErikQQY requested review from mhauru and sunxd3 April 4, 2025 14:17
@sunxd3
Copy link
Member

sunxd3 commented Apr 4, 2025

extra line at

, otherwise good from me

@yebai yebai merged commit 511d6ec into main Apr 4, 2025
14 of 17 checks passed
@yebai yebai deleted the qqy/docs branch April 4, 2025 19:02
@yebai
Copy link
Member

yebai commented Apr 4, 2025

Thanks @ErikQQY!

Copy link
Member

@mhauru mhauru left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @ErikQQY! I know this has been merged, but flagging a minor inconsistency for maybe fixing in the future. Good stuff overall though.

@@ -1,5 +1,5 @@
# Gradient in AdvancedHMC.jl

AdvancedHMC.jl supports automatic differentiation using [`LogDensityProblemsAD`](https://github.com/tpapp/LogDensityProblemsAD.jl) and user-specified gradients. While the default AD backend for AdvancedHMC.jl is ForwardDiff.jl, we can seamlessly change to other backend like Zygote.jl using various syntax like `Hamiltonian(metric, ℓπ, Zygote)`, `Hamiltonian(metric, ℓπ, Val(:Zygote))` or via ADTypes.jl `Hamiltonian(metric, ℓπ, AutoZygote())`.
AdvancedHMC.jl supports automatic differentiation using [`LogDensityProblemsAD`](https://github.com/tpapp/LogDensityProblemsAD.jl) across various AD backends and allows user-specified gradients. While the default AD backend for AdvancedHMC.jl is ForwardDiff.jl, we can seamlessly change to other backend like Mooncake.jl using various syntax like `Hamiltonian(metric, ℓπ, AutoZygote())`. Different AD backend can also be pluged in using `Hamiltonian(metric, ℓπ, Zygote)`, `Hamiltonian(metric, ℓπ, Val(:Zygote))` but we recommend using ADTypes since that would allow you to have more freedom for specifying the AD backend.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the mixture of Mooncake and Zygote here intentional? Would it be clearer to have the whole example use Mooncake?

Copy link
Member

@yebai yebai Apr 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I probably merge this too soon. Please open another follow-up PR.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Follow-up in #423

@yebai yebai mentioned this pull request Apr 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants