No need to test AD for `SamplingContext{<:HMC}` and friends #2645

penelopeysm · 2025-08-05T00:23:54Z

Previously, HMC (and HMCDA, NUTS, SGLD, SGHMC) would construct a LogDensityFunction with a SamplingContext(sampler) as the context. But then the tilde-pipeline wasn't overloaded for those samplers so effectively SamplingContext behaved exactly the same as DefaultContext.

The use of SamplingContext for these samplers was already removed a few PRs back (see e.g. #2550 (comment)).

This also means that there's no need to test AD with these SamplingContexts since they aren't ever used.

This PR removes the corresponding tests.

github-actions · 2025-08-05T00:25:48Z

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

penelopeysm · 2025-08-05T00:26:35Z

test/ad.jl

-                spl_model = DynamicPPL.contextualize(
-                    model, DynamicPPL.SamplingContext(rng, DynamicPPL.Sampler(HMC(0.1, 10)))
-                )
-                @test run_ad(spl_model, adtype; test=true, benchmark=false) isa Any
+                @test run_ad(model, adtype; test=true, benchmark=false) isa Any


the model is no longer contextualised when it is passed to the HMC sub-sampler so no need to do it here too.

codecov · 2025-08-05T00:29:41Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (mhauru/dppl-0.37@d92fd56). Learn more about missing BASE report.

Additional details and impacted files

@@                 Coverage Diff                 @@
##             mhauru/dppl-0.37    #2645   +/-   ##
===================================================
  Coverage                    ?   85.90%           
===================================================
  Files                       ?       22           
  Lines                       ?     1447           
  Branches                    ?        0           
===================================================
  Hits                        ?     1243           
  Misses                      ?      204           
  Partials                    ?        0

☔ 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.

mhauru

So we just never evaluate a model with SamplingContext{<:Hamiltonian}, right? In that case I agree, these tests are pointless.

mhauru · 2025-08-05T14:56:25Z

test/ad.jl

 @testset verbose = true "AD / GibbsContext" begin
-    # Gibbs sampling also needs extra AD testing because the models are
+    # Gibbs sampling needs some extra AD testing because the models are


I appreciate the effort here for not having to adjust the line breaks in the comment to keep line width consistent.

That was coincidental, haha

Although maybe topical: do you use 92 for line width? I think my vim is still configured for 80, and I'm not sure if it's supposed to be longer. JuliaFormatter usually takes care of most things, but AFAIK it doesn't touch line widths in comments.

Yeah, I have 92 set in vim for .jl files. Autoformat still fails though, so I end up tweaking comments and docstrings manually, and it annoys me. Should look for a solution.

Our codebase is quite inconsistent in docstring and comment line widths though, so I'm not too bothered by them. Especially ones that are unnecessarily short.

* First efforts towards DPPL 0.37 compat, WIP * More DPPL 0.37 compat work, WIP * Add [sources] for [email protected] * 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 <[email protected]> * "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 <[email protected]> * 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 <[email protected]> --------- Co-authored-by: Markus Hauru <[email protected]> * 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 <[email protected]> --------- Co-authored-by: Markus Hauru <[email protected]> * Update changelog for PG in Gibbs --------- Co-authored-by: Penelope Yong <[email protected]>

* [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 [email protected] * 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 <[email protected]> * "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 <[email protected]> * 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 <[email protected]> --------- Co-authored-by: Markus Hauru <[email protected]> * 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 <[email protected]> --------- Co-authored-by: Markus Hauru <[email protected]> * Update changelog for PG in Gibbs --------- Co-authored-by: Penelope Yong <[email protected]> --------- Co-authored-by: Markus Hauru <[email protected]>

No need to test AD for SamplingContext{<:HMC}

7356090

github-actions bot assigned penelopeysm Aug 5, 2025

penelopeysm commented Aug 5, 2025

View reviewed changes

penelopeysm requested a review from mhauru August 5, 2025 00:47

mhauru approved these changes Aug 5, 2025

View reviewed changes

Merge branch 'mhauru/dppl-0.37' into py/no-hmc-samplingcontext

d4b6324

penelopeysm merged commit 806c82d into mhauru/dppl-0.37 Aug 5, 2025
19 of 30 checks passed

penelopeysm deleted the py/no-hmc-samplingcontext branch August 5, 2025 16:40

penelopeysm added a commit that referenced this pull request Aug 7, 2025

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

8af36e1

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

No need to test AD for `SamplingContext{<:HMC}` and friends #2645

No need to test AD for `SamplingContext{<:HMC}` and friends #2645

Uh oh!

penelopeysm commented Aug 5, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Aug 5, 2025

Uh oh!

penelopeysm Aug 5, 2025

Uh oh!

codecov bot commented Aug 5, 2025 •

edited

Loading

Uh oh!

mhauru left a comment

Uh oh!

mhauru Aug 5, 2025

Uh oh!

penelopeysm Aug 5, 2025

Uh oh!

penelopeysm Aug 5, 2025

Uh oh!

mhauru Aug 5, 2025

Uh oh!

Uh oh!

Uh oh!

No need to test AD for SamplingContext{<:HMC} and friends #2645

No need to test AD for SamplingContext{<:HMC} and friends #2645

Uh oh!

Conversation

penelopeysm commented Aug 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Aug 5, 2025

Uh oh!

penelopeysm Aug 5, 2025

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Aug 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

mhauru left a comment

Choose a reason for hiding this comment

Uh oh!

mhauru Aug 5, 2025

Choose a reason for hiding this comment

Uh oh!

penelopeysm Aug 5, 2025

Choose a reason for hiding this comment

Uh oh!

penelopeysm Aug 5, 2025

Choose a reason for hiding this comment

Uh oh!

mhauru Aug 5, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

No need to test AD for `SamplingContext{<:HMC}` and friends #2645

No need to test AD for `SamplingContext{<:HMC}` and friends #2645

penelopeysm commented Aug 5, 2025 •

edited

Loading

codecov bot commented Aug 5, 2025 •

edited

Loading