-
Notifications
You must be signed in to change notification settings - Fork 36
Description
Following on from the removal of SamplingContext, there are only four more parent contexts: Condition/Fix, Prefix, and Gibbs. All of these are completely internal to DynamicPPL and there is little reason for the average user to hook into the implementation of this, or indeed to define their own parent context.
I think this offers us a chance to clean up some APIs. Firstly, we can remove NodeTrait
; its main purpose is to distinguish between leaf and parent contexts. I think a nicer way to do it would be to do a hierarchy of contexts:
abstract type AbstractContext end
abstract type ParentContext <: AbstractContext end
One might argue there should be a LeafContext
too, but I think that's overkill. Essentially if something subtypes AbstractContext
but doesn't subtype ParentContext
, it can be treated as a leaf context. Because nothing except internal contexts needs to subtype ParentContext
, the whole idea of a ParentContext
can effectively be hidden away from users (and ultimately removed).
That simplifies the required context interface (after this, if someone wants to define their own leaf context, all they need to do is to subtype AbstractContext
and overload tilde_assume!!
and tilde_observe!!
) and also allows us to improve method dispatch on some methods, most notably:
DynamicPPL.jl/src/context_implementations.jl
Lines 15 to 19 in 5a98037
function tilde_assume!!( | |
context::AbstractContext, right::Distribution, vn::VarName, vi::AbstractVarInfo | |
) | |
return tilde_assume!!(childcontext(context), right, vn, vi) | |
end |
which claims to work for all contexts but will of course error in an unintuitive way for leaf contexts that don't override tilde_assume!!
.
As part of the work on this, the meaning of contexts and how to create a new leaf context should be written up somewhere in the docs. In particular it needs to be mentioned that tilde_obssume!!
should call accumulate_obssume!!
somehow.