Skip to content

Gibbs fixes for DPPL 0.37 (plus tiny bugfixes for ESS + HMC) #2628

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
Jul 23, 2025

Conversation

penelopeysm
Copy link
Member

@penelopeysm penelopeysm commented Jul 22, 2025

CI status on this PR vs #2625 (pass / fail / error):

everything else #2625 - 1392 / 3 / 28 
everything else #2628 - 1421 / 2 / 10

Inference #2625 - 68 / 0 / 3
Inference #2628 - 68 / 0 / 3

Gibbs #2625 - 2248 / 33 / 26
Gibbs #2628 - 2511 / 4 / 10

I believe most (if not all) of the remaining test failures relate to pMCMC and VariableOrderAccumulator.


Well, let's see if I actually understand what I'm doing now. 🫠

In fairness to myself, before this PR, Gibbs would give wrong results like this:

julia> using Turing, Random

julia> @model function f()
           x ~ Normal()
           y ~ Normal(x)
           1.0 ~ Normal(y)
       end
f (generic function with 2 methods)

julia> chn = sample(Xoshiro(468), model, Gibbs(:x => HMC(0.1, 20), :y => MH()), 20000)
[...]
Summary Statistics
  parameters      mean       std      mcse     ess_bulk     ess_tail      rhat   
      Symbol   Float64   Float64   Float64      Float64      Float64   Float64   

           x   -0.0072    0.9965    0.0046   46727.7614   20309.9910    1.0000   
           y    0.6848    0.7669    0.0083    8493.5698    7860.8056    1.0003   
[...]

The correct result should be somewhere around (x, y) = (1/3, 2/3) (can be verified with NUTS).

With this PR:

julia> chn = sample(Xoshiro(468), model, Gibbs(:x => HMC(0.1, 20), :y => MH()), 20000)
[...]
Summary Statistics
  parameters      mean       std      mcse    ess_bulk    ess_tail      rhat   e 
      Symbol   Float64   Float64   Float64     Float64     Float64   Float64     

           x    0.3416    0.8140    0.0084   9366.9879   6877.5666    1.0002     
           y    0.6854    0.8271    0.0167   2476.9080   1648.9534    1.0009     
[...]

On top of that, I also ran some ESS+ESS Gibbs tests on both main and this PR, and both branches gave the same results when seeded, which is quite reassuring.

@penelopeysm penelopeysm changed the base branch from main to mhauru/dppl-0.37-pmcmc July 22, 2025 23:27
@penelopeysm penelopeysm marked this pull request as draft July 22, 2025 23:27
Copy link
Contributor

Turing.jl documentation for PR #2628 is available at:
https://TuringLang.github.io/Turing.jl/previews/PR2628/

Copy link

codecov bot commented Jul 22, 2025

Codecov Report

Attention: Patch coverage is 75.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 24.57%. Comparing base (b0df6a6) to head (c0c298e).
Report is 1 commits behind head on mhauru/dppl-0.37-pmcmc.

Files with missing lines Patch % Lines
src/mcmc/ess.jl 0.00% 2 Missing ⚠️
Additional details and impacted files
@@                    Coverage Diff                     @@
##           mhauru/dppl-0.37-pmcmc    #2628      +/-   ##
==========================================================
+ Coverage                   24.47%   24.57%   +0.10%     
==========================================================
  Files                          22       22              
  Lines                        1479     1481       +2     
==========================================================
+ Hits                          362      364       +2     
  Misses                       1117     1117              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@penelopeysm penelopeysm changed the title Gibbs fixes for DPPL 0.37 Gibbs fixes for DPPL 0.37 (plus an ESS bugfix) Jul 23, 2025
@penelopeysm penelopeysm changed the title Gibbs fixes for DPPL 0.37 (plus an ESS bugfix) Gibbs fixes for DPPL 0.37 (plus tiny bugfixes for ESS + HMC) Jul 23, 2025
@penelopeysm penelopeysm marked this pull request as ready for review July 23, 2025 01:16
@penelopeysm penelopeysm requested a review from mhauru July 23, 2025 12:18
@mhauru mhauru merged commit 31f7331 into mhauru/dppl-0.37-pmcmc Jul 23, 2025
10 of 33 checks passed
@mhauru mhauru deleted the py/dppl-0.37-gibbs branch July 23, 2025 14:13
@penelopeysm penelopeysm mentioned this pull request Jul 26, 2025
penelopeysm added a commit that referenced this pull request Jul 31, 2025
* Progress in DPPL 0.37 compat for particle MCMC

* WIP PMCMC work

* Gibbs fixes for DPPL 0.37 (plus tiny bugfixes for ESS + HMC) (#2628)

* Obviously this single commit will make Gibbs work

* Fixes for ESS

* Fix HMC call

* improve some comments

* Fixes to ProduceLogLikelihoodAccumulator

* Use LogProbAccumulator for ProduceLogLikelihoodAccumulator

* use get_conditioned_gibbs

---------

Co-authored-by: Penelope Yong <penelopeysm@gmail.com>
penelopeysm added a commit that referenced this pull request Aug 12, 2025
* First efforts towards DPPL 0.37 compat, WIP

* More DPPL 0.37 compat work, WIP

* Add [sources] for DynamicPPL@0.37

* Remove context argument from `LogDensityFunction`

* Fix MH

* Remove spurious logging

* Remove residual OptimizationContext

* Delete files that were removed in previous releases

* Fix typo

* Simplify ESS

* Fix LDF

* Fix Prior(), fix a couple more imports

* fixes

* actually fix prior

* Remove extra return value from tilde_assume

* fix ldf

* actually fix prior

* fix HMC log-density

* fix ldf

* fix make_evaluate_...

* more fixes for evaluate!!

* fix hmc

* fix run_ad

* even more fixes (oh goodness when will this end)

* more fixes

* fix

* more fix fix fix

* fix return values of tilde pipeline

* even more fixes

* Fix missing import

* More MH fixes

* Fix conversion

* don't think it really needs those type params

* implement copy for LogPriorWithoutJacAcc

* Even more fixes

* More fixes; I think the remaining failures are pMCMC related

* Fix merge

* DPPL 0.37 compat for particle MCMC (#2625)

* Progress in DPPL 0.37 compat for particle MCMC

* WIP PMCMC work

* Gibbs fixes for DPPL 0.37 (plus tiny bugfixes for ESS + HMC) (#2628)

* Obviously this single commit will make Gibbs work

* Fixes for ESS

* Fix HMC call

* improve some comments

* Fixes to ProduceLogLikelihoodAccumulator

* Use LogProbAccumulator for ProduceLogLikelihoodAccumulator

* use get_conditioned_gibbs

---------

Co-authored-by: Penelope Yong <penelopeysm@gmail.com>

* "Fixes" for PG-in-Gibbs (#2629)

* WIP PMCMC work

* Fixes to ProduceLogLikelihoodAccumulator

* inline definition of `set_retained_vns_del!`

* Fix ProduceLogLikelihoodAcc

* Remove all uses of `set_retained_vns_del!`

* Use nice functions

* Remove PG tests with dynamic number of Gibbs-conditioned-observations

* Fix essential/container tests

* Update pMCMC implementation as per discussion

* remove extra printing statements

* revert unneeded changes

* Add back (some kind of) dynamic model test

* fix rebase

* Add a todo comment for dynamic model tests

---------

Co-authored-by: Markus Hauru <mhauru@turing.ac.uk>

* Use accumulators to fix all logp calculations when sampling (#2630)

* Use new `getlogjoint` for optimisation

* Change getlogjoint -> getlogjoint_internal where needed

* Enforce re-evaluation when constructing `Transition`

* fix tests

* Remove extra evaluations from SGLD and SGHMC

* Remove dead `transitions_from_chain` method (used to be part of `predict`)

* metadata -> getstats_with_lp

* Clean up some stray getlogp

* InitContext isn't for 0.37, update comments

* Fix merge

* Do not re-evaluate model for Prior (#2644)

* Allow Prior to skip model re-evaluation

* remove unneeded `default_chain_type` method

* add a test

* add a likelihood term too

* why not test correctness while we're at it

* No need to test AD for SamplingContext{<:HMC} (#2645)

* change breaking -> main

* Remove calls to resetlogp!! & add changelog (#2650)

* Remove calls to resetlogp!!

* Add a changelog for 0.40

* Update HISTORY.md

Co-authored-by: Markus Hauru <markus@mhauru.org>

---------

Co-authored-by: Markus Hauru <markus@mhauru.org>

* Remove `[sources]`

* Unify Turing `Transition`s, fix some tests (#2651)

* Unify `Transition` methods

* Add tests

* Add same test for SGLD/SGHMC

* Refactor so that it's nice and organised

* Fix failing test on 1.10

* just increase the atol

* Make addlogprob test more robust

* Remove stray `@show`

Co-authored-by: Markus Hauru <markus@mhauru.org>

---------

Co-authored-by: Markus Hauru <markus@mhauru.org>

* Update changelog for PG in Gibbs

---------

Co-authored-by: Penelope Yong <penelopeysm@gmail.com>
penelopeysm added a commit that referenced this pull request Aug 12, 2025
* [no ci] Bump to v0.40.0

* Uncomment tests that should be there

* Support DPPL 0.37 (#2550)

* First efforts towards DPPL 0.37 compat, WIP

* More DPPL 0.37 compat work, WIP

* Add [sources] for DynamicPPL@0.37

* Remove context argument from `LogDensityFunction`

* Fix MH

* Remove spurious logging

* Remove residual OptimizationContext

* Delete files that were removed in previous releases

* Fix typo

* Simplify ESS

* Fix LDF

* Fix Prior(), fix a couple more imports

* fixes

* actually fix prior

* Remove extra return value from tilde_assume

* fix ldf

* actually fix prior

* fix HMC log-density

* fix ldf

* fix make_evaluate_...

* more fixes for evaluate!!

* fix hmc

* fix run_ad

* even more fixes (oh goodness when will this end)

* more fixes

* fix

* more fix fix fix

* fix return values of tilde pipeline

* even more fixes

* Fix missing import

* More MH fixes

* Fix conversion

* don't think it really needs those type params

* implement copy for LogPriorWithoutJacAcc

* Even more fixes

* More fixes; I think the remaining failures are pMCMC related

* Fix merge

* DPPL 0.37 compat for particle MCMC (#2625)

* Progress in DPPL 0.37 compat for particle MCMC

* WIP PMCMC work

* Gibbs fixes for DPPL 0.37 (plus tiny bugfixes for ESS + HMC) (#2628)

* Obviously this single commit will make Gibbs work

* Fixes for ESS

* Fix HMC call

* improve some comments

* Fixes to ProduceLogLikelihoodAccumulator

* Use LogProbAccumulator for ProduceLogLikelihoodAccumulator

* use get_conditioned_gibbs

---------

Co-authored-by: Penelope Yong <penelopeysm@gmail.com>

* "Fixes" for PG-in-Gibbs (#2629)

* WIP PMCMC work

* Fixes to ProduceLogLikelihoodAccumulator

* inline definition of `set_retained_vns_del!`

* Fix ProduceLogLikelihoodAcc

* Remove all uses of `set_retained_vns_del!`

* Use nice functions

* Remove PG tests with dynamic number of Gibbs-conditioned-observations

* Fix essential/container tests

* Update pMCMC implementation as per discussion

* remove extra printing statements

* revert unneeded changes

* Add back (some kind of) dynamic model test

* fix rebase

* Add a todo comment for dynamic model tests

---------

Co-authored-by: Markus Hauru <mhauru@turing.ac.uk>

* Use accumulators to fix all logp calculations when sampling (#2630)

* Use new `getlogjoint` for optimisation

* Change getlogjoint -> getlogjoint_internal where needed

* Enforce re-evaluation when constructing `Transition`

* fix tests

* Remove extra evaluations from SGLD and SGHMC

* Remove dead `transitions_from_chain` method (used to be part of `predict`)

* metadata -> getstats_with_lp

* Clean up some stray getlogp

* InitContext isn't for 0.37, update comments

* Fix merge

* Do not re-evaluate model for Prior (#2644)

* Allow Prior to skip model re-evaluation

* remove unneeded `default_chain_type` method

* add a test

* add a likelihood term too

* why not test correctness while we're at it

* No need to test AD for SamplingContext{<:HMC} (#2645)

* change breaking -> main

* Remove calls to resetlogp!! & add changelog (#2650)

* Remove calls to resetlogp!!

* Add a changelog for 0.40

* Update HISTORY.md

Co-authored-by: Markus Hauru <markus@mhauru.org>

---------

Co-authored-by: Markus Hauru <markus@mhauru.org>

* Remove `[sources]`

* Unify Turing `Transition`s, fix some tests (#2651)

* Unify `Transition` methods

* Add tests

* Add same test for SGLD/SGHMC

* Refactor so that it's nice and organised

* Fix failing test on 1.10

* just increase the atol

* Make addlogprob test more robust

* Remove stray `@show`

Co-authored-by: Markus Hauru <markus@mhauru.org>

---------

Co-authored-by: Markus Hauru <markus@mhauru.org>

* Update changelog for PG in Gibbs

---------

Co-authored-by: Penelope Yong <penelopeysm@gmail.com>

---------

Co-authored-by: Markus Hauru <markus@mhauru.org>
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