Skip to content

Remove calls to resetlogp!! & add changelog #2650

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 3 commits into from
Aug 11, 2025
Merged

Conversation

penelopeysm
Copy link
Member

@penelopeysm penelopeysm commented Aug 10, 2025

Previously, pMCMC used to hijack the varinfo's logp field for its own purposes. For example, contributions from Gibbs-conditioned-variables and addlogprob used to be added there, and then after each observation the logp would be reset to 0.

Since we now use ProduceLogLikelihoodAccumulator to control particle weights and resampling, we no longer need to call resetlogp on the varinfo explicitly (indeed the varinfo's logp could simply be used in a normal way, i.e., to accumulate the logp during model evaluation --- although I don't think we actually use it anywhere.)

@penelopeysm penelopeysm changed the base branch from main to mhauru/dppl-0.37 August 10, 2025 22:41
Copy link
Contributor

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

@penelopeysm penelopeysm changed the title Remove calls to resetlogp!! Remove calls to resetlogp!! & add changelog Aug 11, 2025
@penelopeysm
Copy link
Member Author

Since this PR was so boring, I wrote up a changelog as well to make it more exciting. Happy to edit as needed.

Copy link

codecov bot commented Aug 11, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 45.52%. Comparing base (5743ff7) to head (65d16a7).
⚠️ Report is 2 commits behind head on mhauru/dppl-0.37.

❗ There is a different number of reports uploaded between BASE (5743ff7) and HEAD (65d16a7). Click for more details.

HEAD has 11 uploads less than BASE
Flag BASE (5743ff7) HEAD (65d16a7)
17 6
Additional details and impacted files
@@                  Coverage Diff                  @@
##           mhauru/dppl-0.37    #2650       +/-   ##
=====================================================
- Coverage             85.90%   45.52%   -40.38%     
=====================================================
  Files                    22       22               
  Lines                  1447     1430       -17     
=====================================================
- Hits                   1243      651      -592     
- Misses                  204      779      +575     

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

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. One optional suggestion for you to consider.

Co-authored-by: Markus Hauru <[email protected]>
@penelopeysm penelopeysm merged commit 57e6f9c into mhauru/dppl-0.37 Aug 11, 2025
2 checks passed
@penelopeysm penelopeysm deleted the py/resetlogp branch August 11, 2025 13:30
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 [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]>
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 [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]>
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