Fix #483: Rename neg_energy → neg_kinetic_energy#490
Conversation
Rename the kinetic-energy-specific neg_energy overloads (those taking momentum r) to neg_kinetic_energy to eliminate the naming ambiguity: - neg_energy(z::PhasePoint) → total negative Hamiltonian (unchanged) - neg_energy(h, θ) → negative potential energy (unchanged) - neg_kinetic_energy(h,r,θ) → negative kinetic energy (renamed) Add an explicit energy(h, r, θ) = -neg_kinetic_energy(h, r, θ) dispatch so the energy interface used in tests continues to work after the rename. 4 files changed: src/hamiltonian.jl, src/riemannian/hamiltonian.jl, test/hamiltonian.jl, test/riemannian.jl. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
|
||
| function phasepoint( | ||
| h::Hamiltonian, θ::T, r::T; ℓπ=∂H∂θ(h, θ), ℓκ=DualValue(neg_energy(h, r, θ), ∂H∂r(h, r)) | ||
| h::Hamiltonian, θ::T, r::T; ℓπ=∂H∂θ(h, θ), ℓκ=DualValue(neg_kinetic_energy(h, r, θ), ∂H∂r(h, r)) |
There was a problem hiding this comment.
[JuliaFormatter] reported by reviewdog 🐶
| h::Hamiltonian, θ::T, r::T; ℓπ=∂H∂θ(h, θ), ℓκ=DualValue(neg_kinetic_energy(h, r, θ), ∂H∂r(h, r)) | |
| h::Hamiltonian, | |
| θ::T, | |
| r::T; | |
| ℓπ=∂H∂θ(h, θ), | |
| ℓκ=DualValue(neg_kinetic_energy(h, r, θ), ∂H∂r(h, r)), |
| end | ||
|
|
||
| energy(args...) = -neg_energy(args...) | ||
| energy(h::Hamiltonian, r::AbstractVecOrMat, θ::AbstractVecOrMat) = -neg_kinetic_energy(h, r, θ) |
There was a problem hiding this comment.
[JuliaFormatter] reported by reviewdog 🐶
| energy(h::Hamiltonian, r::AbstractVecOrMat, θ::AbstractVecOrMat) = -neg_kinetic_energy(h, r, θ) | |
| function energy(h::Hamiltonian, r::AbstractVecOrMat, θ::AbstractVecOrMat) | |
| -neg_kinetic_energy(h, r, θ) | |
| end |
|
|
||
| h = Hamiltonian(UnitEuclideanMetric(T, D), ℓπ, ∂ℓπ∂θ) | ||
| @test -AdvancedHMC.neg_energy(h, r_init, θ_init) == sum(abs2, r_init) / 2 | ||
| @test -AdvancedHMC.neg_kinetic_energy(h, r_init, θ_init) == sum(abs2, r_init) / 2 |
There was a problem hiding this comment.
[JuliaFormatter] reported by reviewdog 🐶
| @test -AdvancedHMC.neg_kinetic_energy(h, r_init, θ_init) == sum(abs2, r_init) / 2 | |
| @test -AdvancedHMC.neg_kinetic_energy(h, r_init, θ_init) == | |
| sum(abs2, r_init) / 2 |
| M⁻¹ = m' * m | ||
| h = Hamiltonian(DenseEuclideanMetric(M⁻¹), ℓπ, ∂ℓπ∂θ) | ||
| @test -AdvancedHMC.neg_energy(h, r_init, θ_init) ≈ r_init' * M⁻¹ * r_init / 2 | ||
| @test -AdvancedHMC.neg_kinetic_energy(h, r_init, θ_init) ≈ r_init' * M⁻¹ * r_init / 2 |
There was a problem hiding this comment.
[JuliaFormatter] reported by reviewdog 🐶
| @test -AdvancedHMC.neg_kinetic_energy(h, r_init, θ_init) ≈ r_init' * M⁻¹ * r_init / 2 | |
| @test -AdvancedHMC.neg_kinetic_energy(h, r_init, θ_init) ≈ | |
| r_init' * M⁻¹ * r_init / 2 |
|
|
||
| h = Hamiltonian(UnitEuclideanMetric(T, 2 * D), ℓπ, ∂ℓπ∂θ) | ||
| @test -AdvancedHMC.neg_energy(h, r_init, θ_init) == sum(abs2, r_init) / 2 | ||
| @test -AdvancedHMC.neg_kinetic_energy(h, r_init, θ_init) == sum(abs2, r_init) / 2 |
There was a problem hiding this comment.
[JuliaFormatter] reported by reviewdog 🐶
| @test -AdvancedHMC.neg_kinetic_energy(h, r_init, θ_init) == sum(abs2, r_init) / 2 | |
| @test -AdvancedHMC.neg_kinetic_energy(h, r_init, θ_init) == | |
| sum(abs2, r_init) / 2 |
| M⁻¹ = ComponentArray(m' * m, ax, ax) | ||
| h = Hamiltonian(DenseEuclideanMetric(M⁻¹), ℓπ, ∂ℓπ∂θ) | ||
| @test -AdvancedHMC.neg_energy(h, r_init, θ_init) ≈ r_init' * M⁻¹ * r_init / 2 | ||
| @test -AdvancedHMC.neg_kinetic_energy(h, r_init, θ_init) ≈ r_init' * M⁻¹ * r_init / 2 |
There was a problem hiding this comment.
[JuliaFormatter] reported by reviewdog 🐶
| @test -AdvancedHMC.neg_kinetic_energy(h, r_init, θ_init) ≈ r_init' * M⁻¹ * r_init / 2 | |
| @test -AdvancedHMC.neg_kinetic_energy(h, r_init, θ_init) ≈ | |
| r_init' * M⁻¹ * r_init / 2 |
| @testset "Kinetic energy" begin | ||
| Σ = hamiltonian.metric.map(hamiltonian.metric.G(x)) | ||
| @test neg_energy(hamiltonian, r, x) ≈ logpdf(MvNormal(zeros(D), Σ), r) | ||
| @test neg_kinetic_energy(hamiltonian, r, x) ≈ logpdf(MvNormal(zeros(D), Σ), r) |
There was a problem hiding this comment.
[JuliaFormatter] reported by reviewdog 🐶
| @test neg_kinetic_energy(hamiltonian, r, x) ≈ logpdf(MvNormal(zeros(D), Σ), r) | |
| @test neg_kinetic_energy(hamiltonian, r, x) ≈ | |
| logpdf(MvNormal(zeros(D), Σ), r) |
|
AdvancedHMC.jl documentation for PR #490 is available at: |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #490 +/- ##
==========================================
- Coverage 77.71% 77.65% -0.07%
==========================================
Files 21 21
Lines 1270 1271 +1
==========================================
Hits 987 987
- Misses 283 284 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
neg_energy → neg_kinetic_energy
I'll check the changes before marking it as ready for review.
Human content above
Claude content below
Rename
neg_energy→neg_kinetic_energyProblem
neg_energyis overloaded with three distinct meanings:neg_energy(z::PhasePoint)→ total negative Hamiltonian energy (ℓπ + ℓκ)neg_energy(h, θ)→ negative potential energy (log density)neg_energy(h, r, θ)→ negative kinetic energyThe third overload (the one taking momentum
r) is the confusing one: internally it is cached asℓκ(the "neg kinetic energy") inPhasePoint, and the Riemannian code even has a comment# Negative kinetic energydirectly above its definition. The naming mismatch makes it hard to understand whatphasepoint(h, θ, r)is actually caching.Change
4 files changed (21 insertions, 20 deletions):
src/hamiltonian.jl— rename 5neg_energy(h, r, θ)definitions toneg_kinetic_energy; update the twophasepointcall sites; add explicitenergy(h, r, θ)dispatch that routes toneg_kinetic_energy(theenergy(args...)catch-all can no longer reach the renamed function)src/riemannian/hamiltonian.jl— rename the 6th kinetic overload (Riemannian metric); update import andphasepointcall sitetest/hamiltonian.jl— update 6 test assertionstest/riemannian.jl— update import and 1 assertionMWE
See
483-rename-neg-kinetic-energy.jl. Checks thatAdvancedHMC.neg_kinetic_energyis defined and callable, and thatneg_energy(z::PhasePoint)still works. Fails onmainbecauseneg_kinetic_energydoes not exist.Notes
neg_energy(z::PhasePoint)andneg_energy(h, θ)are untouched — only the kinetic-energy-specific overloads are renamed.energy(h, r, θ)dispatch preserves the existingenergyinterface used intest/riemannian.jl.MWE
483-rename-neg-kinetic-energy.jlmain output:
worktree output:
Fixes #483