Skip to content

Allow usage of array literals on LHS of ~ #261

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.

Sign up for GitHub

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 11 commits into from
Jun 9, 2021
Merged

Allow usage of array literals on LHS of ~ #261

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
4138516
treat vectors on LHS as literals
torfjelde Jun 9, 2021
f9ae0a8
version bump
torfjelde Jun 9, 2021
d76f60b
added test for array literals
torfjelde Jun 9, 2021
d6a1e42
simplified the isliteral check
torfjelde Jun 9, 2021
412548d
formatting
torfjelde Jun 9, 2021
b993b97
dont allow empty literals
torfjelde Jun 9, 2021
6bb5412
added some tests for isliteral
torfjelde Jun 9, 2021
d328583
formatting
torfjelde Jun 9, 2021
f8593cb
fixed typo in tests
torfjelde Jun 9, 2021
3810eff
bump Bijectors for tests
torfjelde Jun 9, 2021
8532d8d
Merge branch 'master' into tor/support-for-array-literals
yebai Jun 9, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Project.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
name = "DynamicPPL"
uuid = "366bfd00-2699-11ea-058f-f148b4cae6d8"
version = "0.11.2"
version = "0.11.3"

[deps]
AbstractMCMC = "80f14c24-f653-4e6a-9b94-39d6b0f70001"
Expand Down
13 changes: 11 additions & 2 deletions src/compiler.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,15 @@ end
# failsafe: a literal is never an assumption
isassumption(expr) = :(false)

"""
isliteral(expr)

Return `true` if `expr` is a literal, e.g. `1.0` or `[1.0, ]`, and `false` otherwise.
"""
isliteral(e) = false
isliteral(::Number) = true
isliteral(e::Expr) = !isempty(e.args) && all(isliteral, e.args)
Copy link
Member

@devmotion devmotion Jun 9, 2021

Choose a reason for hiding this comment

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

This might be a bit general, e.g., also tuples of numbers or arrays of tuples of numbers or arrays of arrays of numbers etc are considered as literals now, in contrast to what the docstring mentions. But maybe this was your intention?

In any case, I guess it would be good to add some simple tests.

Copy link
Member Author

@torfjelde torfjelde Jun 9, 2021

Choose a reason for hiding this comment

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

That was my intention yeah. These might still throw errors later when calling the model, e.g. mby there's no impl for assume for a tuple, but figured it was a good idea to not be very restrictive in the actual model-macro as we/people might add implementations for their distribution or w/e.

In any case, I guess it would be good to add some simple tests.

Yeah I'll add some 👍

Copy link
Member Author

@torfjelde torfjelde Jun 9, 2021

Choose a reason for hiding this comment

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

Good now?


"""
check_tilde_rhs(x)

Expand Down Expand Up @@ -240,7 +249,7 @@ variables.
"""
function generate_tilde(left, right)
# If the LHS is a literal, it is always an observation
if !(left isa Symbol || left isa Expr)
if isliteral(left)
return quote
$(DynamicPPL.tilde_observe)(
__context__,
Expand Down Expand Up @@ -290,7 +299,7 @@ Generate the expression that replaces `left .~ right` in the model body.
"""
function generate_dot_tilde(left, right)
# If the LHS is a literal, it is always an observation
if !(left isa Symbol || left isa Expr)
if isliteral(left)
return quote
$(DynamicPPL.dot_tilde_observe)(
__context__,
Expand Down
2 changes: 1 addition & 1 deletion test/Project.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ Zygote = "e88e6eb3-aa80-5325-afca-941959d7151f"
[compat]
AbstractMCMC = "2.1, 3.0"
AbstractPPL = "0.1.3"
Bijectors = "0.8.2, 0.9"
Bijectors = "0.9.5"
Distributions = "0.24, 0.25"
DistributionsAD = "0.6.3"
Documenter = "0.26.1"
Expand Down
26 changes: 26 additions & 0 deletions test/compiler.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -423,4 +423,30 @@ end
x = [Laplace(), Normal(), MvNormal(3, 1.0)]
@test DynamicPPL.check_tilde_rhs(x) === x
end
@testset "isliteral" begin
@test DynamicPPL.isliteral(:([1.0]))
@test DynamicPPL.isliteral(:([[1.0], 1.0]))
@test DynamicPPL.isliteral(:((1.0, 1.0)))

@test !(DynamicPPL.isliteral(:([x])))
@test !(DynamicPPL.isliteral(:([[x], 1.0])))
@test !(DynamicPPL.isliteral(:((x, 1.0))))
end

@testset "array literals" begin
# Verify that we indeed can parse this.
@test @model(function array_literal_model()
# `assume` and literal `observe`
m ~ MvNormal(2, 1.0)
return [10.0, 10.0] ~ MvNormal(m, 0.5 * ones(2))
end) isa Function

@model function array_literal_model()
# `assume` and literal `observe`
m ~ MvNormal(2, 1.0)
return [10.0, 10.0] ~ MvNormal(m, 0.5 * ones(2))
end

@test array_literal_model()() == [10.0, 10.0]
end
end