Skip to content

Conversation

@torfjelde
Copy link
Member

In the recent breaking release we mistakenly removed the call to observe(::AbstractSampler, right left, vi) as a fallback for DefaultContext, leading to certain sampler breaking in Turing (TuringLang/Turing.jl#1636).

This PR adds back a proper fallback, making overloads such as https://github.com/TuringLang/Turing.jl/blob/tor%2Fdppl-update/src/inference/AdvancedSMC.jl#L353-L356 work as before 0.11.

@devmotion
Copy link
Member

Seems fine as long as there is a fallback without samplers and you don't have to implement these methods if not needed. Seems this is the case with the additional observe fallback 👍

Another question, is tilde_observe and observe consistent with dot_tilde_observe and dot_observe?

torfjelde and others added 2 commits June 10, 2021 12:49
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@torfjelde
Copy link
Member Author

I just tested it locally, and things are working now 👍

Seems fine as long as there is a fallback without samplers and you don't have to implement these methods if not needed. Seems this is the case with the additional observe fallback +1

Exactly, so this PR doesn't do anything other than allow samplers to specify certain observe behavior.

Another question, is tilde_observe and observe consistent with dot_tilde_observe and dot_observe?

And yes it is. I had already done this for dot_observe, but had messed it up for observe. But I just simplified the generic impl of dot_observe a bit now while I was at it.

@torfjelde
Copy link
Member Author

bors r+

bors bot pushed a commit that referenced this pull request Jun 10, 2021
In the recent breaking release we mistakenly removed the call to `observe(::AbstractSampler, right left, vi)` as a fallback for `DefaultContext`, leading to certain sampler breaking in Turing (TuringLang/Turing.jl#1636). 

This PR adds back a proper fallback, making overloads such as https://github.com/TuringLang/Turing.jl/blob/tor%2Fdppl-update/src/inference/AdvancedSMC.jl#L353-L356 work as before 0.11.
@bors bors bot changed the title Fixed fallback for observe [Merged by Bors] - Fixed fallback for observe Jun 10, 2021
@bors bors bot closed this Jun 10, 2021
@bors bors bot deleted the tor/default-assume-observe branch June 10, 2021 12:19
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.

4 participants