TuringLang / DynamicPPL.jl

Implementation of domain-specific language (DSL) for dynamic probabilistic programming
https://turinglang.org/DynamicPPL.jl/
MIT License
164 stars 29 forks source link

Also pass in `context` as an argument to `acclogp!!` #563

Closed torfjelde closed 11 months ago

torfjelde commented 11 months ago

Some samplers have very specific ways of accumulating log-probabilities which they do through overloading of the tilde-pipeline, e.g. PG in Turing overloads both assume and observe because it needs to accumulate the log-probabilities in the task local varinfo rather than the "global" varinfo.

But this means that, in certain scenarios, e.g. PG, usage of @acclogprob!! in a model will (silently) result in target the incorrect model!

One way we can fix this is to also pass in the context to @acclogprob!! (and acclogp!!), which can then alter how the log-probs are accumualted.

A few examples where this is useful:

github-actions[bot] commented 11 months ago

Pull Request Test Coverage Report for Build 6923549774


Files with Coverage Reduction New Missed Lines %
src/utils.jl 33 82.28%
<!-- Total: 33 -->
Totals Coverage Status
Change from base Build 6906848787: 0.04%
Covered Lines: 2541
Relevant Lines: 3165

💛 - Coveralls
codecov[bot] commented 11 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (c9489aa) 80.24% compared to head (1cfe231) 80.28%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #563 +/- ## ========================================== + Coverage 80.24% 80.28% +0.03% ========================================== Files 25 25 Lines 3159 3165 +6 ========================================== + Hits 2535 2541 +6 Misses 624 624 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

torfjelde commented 11 months ago

AFAIK it's not breaking; we're still keeping the old acclogprob!! definition too.

EDIT: bumped patch version