Skip to content

Commit

Permalink
Fix for #371 (#372)
Browse files Browse the repository at this point in the history
This PR adds a method called `resolve_varnames(varname, dist)` and adds an additional generated variable for each `~` which now holds the RHS of `~`. 

It does address #371 but uncertain if this is the best way, so wouldn't recommend merging this just yet. But putting it here so we can colab on it.

Co-authored-by: Hong Ge <[email protected]>
  • Loading branch information
torfjelde and yebai committed Jun 15, 2022
1 parent d222316 commit 4938a31
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 6 deletions.
2 changes: 1 addition & 1 deletion Project.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
name = "DynamicPPL"
uuid = "366bfd00-2699-11ea-058f-f148b4cae6d8"
version = "0.19.1"
version = "0.19.2"

[deps]
AbstractMCMC = "80f14c24-f653-4e6a-9b94-39d6b0f70001"
Expand Down
18 changes: 13 additions & 5 deletions src/compiler.jl
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,9 @@ function unwrap_right_left_vns(
return unwrap_right_left_vns(right, left, vns)
end

resolve_varnames(vn::VarName, _) = vn
resolve_varnames(vn::VarName, dist::NamedDist) = dist.name

#################
# Main Compiler #
#################
Expand Down Expand Up @@ -379,16 +382,19 @@ function generate_tilde(left, right)

# Otherwise it is determined by the model or its value,
# if the LHS represents an observation
@gensym vn isassumption value
@gensym vn isassumption value dist

# HACK: Usage of `drop_escape` is unfortunate. It's a consequence of the fact
# that in DynamicPPL we the entire function body. Instead we should be
# more selective with our escape. Until that's the case, we remove them all.
return quote
$vn = $(AbstractPPL.drop_escape(varname(left)))
$dist = $right
$vn = $(DynamicPPL.resolve_varnames)(
$(AbstractPPL.drop_escape(varname(left))), $dist
)
$isassumption = $(DynamicPPL.isassumption(left, vn))
if $isassumption
$(generate_tilde_assume(left, right, vn))
$(generate_tilde_assume(left, dist, vn))
else
# If `vn` is not in `argnames`, we need to make sure that the variable is defined.
if !$(DynamicPPL.inargnames)($vn, __model__)
Expand All @@ -397,7 +403,7 @@ function generate_tilde(left, right)

$value, __varinfo__ = $(DynamicPPL.tilde_observe!!)(
__context__,
$(DynamicPPL.check_tilde_rhs)($right),
$(DynamicPPL.check_tilde_rhs)($dist),
$(maybe_view(left)),
$vn,
__varinfo__,
Expand Down Expand Up @@ -442,7 +448,9 @@ function generate_dot_tilde(left, right)
# if the LHS represents an observation
@gensym vn isassumption value
return quote
$vn = $(AbstractPPL.drop_escape(varname(left)))
$vn = $(DynamicPPL.resolve_varnames)(
$(AbstractPPL.drop_escape(varname(left))), $right
)
$isassumption = $(DynamicPPL.isassumption(left, vn))
if $isassumption
$(generate_dot_tilde_assume(left, right, vn))
Expand Down
11 changes: 11 additions & 0 deletions src/distribution_wrappers.jl
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,17 @@ end

NamedDist(dist::Distribution, name::Symbol) = NamedDist(dist, VarName{name}())

Distributions.logpdf(dist::NamedDist, x::Real) = Distributions.logpdf(dist.dist, x)
function Distributions.logpdf(dist::NamedDist, x::AbstractArray{<:Real})
return Distributions.logpdf(dist.dist, x)
end
function Distributions.loglikelihood(dist::NamedDist, x::Real)
return Distributions.loglikelihood(dist.dist, x)
end
function Distributions.loglikelihood(dist::NamedDist, x::AbstractArray{<:Real})
return Distributions.loglikelihood(dist.dist, x)
end

struct NoDist{variate,support,Td<:Distribution{variate,support}} <:
Distribution{variate,support}
dist::Td
Expand Down
12 changes: 12 additions & 0 deletions test/compiler.jl
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,18 @@ end
@test vi2.metadata.y.vns[1] == @varname(y[2][:, 1])
@test haskey(vi3.metadata, :y)
@test vi3.metadata.y.vns[1] == @varname(y[1])

# Conditioning
f1_c = f1() | (y=1,)
f2_c = f2() | NamedTuple((Symbol(@varname(y[2][:, 1])) => 1,))
f3_c = f3() | NamedTuple((Symbol(@varname(y[1])) => 1,))
@test f1_c() == 1
# TODO(torfjelde): We need conditioning for `Dict`.
@test_broken f2_c() == 1
@test_broken f3_c() == 1
@test_broken getlogp(VarInfo(f1_c)) ==
getlogp(VarInfo(f2_c)) ==
getlogp(VarInfo(f3_c))
end
@testset "custom tilde" begin
@model demo() = begin
Expand Down

2 comments on commit 4938a31

@devmotion
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JuliaRegistrator
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Registration pull request created: JuliaRegistries/General/62442

After the above pull request is merged, it is recommended that a tag is created on this repository for the registered package version.

This will be done automatically if the Julia TagBot GitHub Action is installed, or can be done manually through the github interface, or via:

git tag -a v0.19.2 -m "<description of version>" 4938a31a4ced04c30e11d9d4875345f17fe69e70
git push origin v0.19.2

Please sign in to comment.