Skip to content

Commit

Permalink
Improve error message when variable is used but not defined (#213)
Browse files Browse the repository at this point in the history 
When a variable is referenced on the RHS, but not provided in data or
appear on LHS, the compiler should error.

Previously, `compile` _will_ error on these cases, but the error message
can be cryptic -- usually in an array-out-of-bound type of errors.

This PR catches and reports these errors in a more graceful manner.

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
  • Loading branch information
@sunxd3 @github-actions
sunxd3 and github-actions[bot] authored Sep 17, 2024
1 parent c14b272 commit 46663e8
Show file tree
Hide file tree
Showing 5 changed files with 178 additions and 72 deletions.
5 changes: 4 additions & 1 deletion src/BUGSExamples/Volume_1/15_Bones.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,10 @@ data = (
0.8021 2.3873 3.9525 5.3198
5.0022 6.3704 8.2832 10.4988
4.0168 5.1537 7.1053 10.3038],
delta = [2.9541 0.6603 0.7965 1.0495 5.7874 3.8376 0.6324 0.8272 0.6968 0.8747 0.8136 0.8246 0.6711 0.978 1.1528 1.6923 1.0331 0.5381 1.0688 8.1123 0.9974 1.2656 1.1802 1.368 1.5435 1.5006 1.6766 1.4297 3.385 3.3085 3.4007 2.0906 1.0954 1.5329],
delta = [2.9541, 0.6603, 0.7965, 1.0495, 5.7874, 3.8376, 0.6324, 0.8272,
0.6968, 0.8747, 0.8136, 0.8246, 0.6711, 0.978, 1.1528, 1.6923, 1.0331,
0.5381, 1.0688, 8.1123, 0.9974, 1.2656, 1.1802, 1.368, 1.5435, 1.5006,
1.6766, 1.4297, 3.385, 3.3085, 3.4007, 2.0906, 1.0954, 1.5329],
ncat = [2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2,
3, 3, 3, 3, 3, 3, 3, 3, 4, 5, 5, 5, 5, 5, 5],
grade = [1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 2 1 1 1 1 1 1 1 1 2 1 1 2 1 1
Expand Down
2 changes: 1 addition & 1 deletion src/JuliaBUGS.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ end

function determine_array_sizes(model_def, data)
pass = CollectVariables(model_def, data)
analyze_block(pass, model_def; warn_loop_bounds=true)
analyze_block(pass, model_def; warn_loop_bounds=Ref(true))
non_data_scalars, non_data_array_sizes = post_process(pass)
return non_data_scalars, non_data_array_sizes
end
Expand Down
88 changes: 72 additions & 16 deletions src/compiler_pass.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ function analyze_block(
pass::CompilerPass,
expr::Expr,
loop_vars::NamedTuple=NamedTuple();
warn_loop_bounds=false,
warn_loop_bounds::Ref{Bool}=Ref(false),
)
if !Meta.isexpr(expr, :block)
error("The top level expression must be a block.")
Expand All @@ -21,13 +21,17 @@ function analyze_block(
lb = Int(simple_arithmetic_eval(env, lb))
ub = Int(simple_arithmetic_eval(env, ub))
if lb > ub
if warn_loop_bounds
if warn_loop_bounds[]
@warn "In BUGS, if the lower bound of for loop is greater than the upper bound, the loop will be skipped."
warn_loop_bounds[] = false
end
else
for loop_var_value in lb:ub
analyze_block(
pass, body, merge(loop_vars, (loop_var => loop_var_value,))
pass,
body,
merge(loop_vars, (loop_var => loop_var_value,));
warn_loop_bounds=warn_loop_bounds,
)
end
end
Expand Down Expand Up @@ -85,6 +89,14 @@ function CollectVariables(model_def::Expr, data::NamedTuple{data_vars}) where {d
end
end

for (var, num_dim) in zip(arrays, num_dims)
if var ∈ data_vars && num_dim != ndims(data[var])
error(
"Array variable $var has $num_dim dimensions in the model, but $(ndims(data[var])) dimensions in the data.",
)
end
end

data_arrays = Symbol[]
data_array_sizes = SVector[]
for k in data_vars
Expand All @@ -99,10 +111,12 @@ function CollectVariables(model_def::Expr, data::NamedTuple{data_vars}) where {d
for (var, num_dim) in zip(arrays, num_dims)
if var ∉ data_vars
push!(non_data_arrays, var)
push!(non_data_array_sizes, MVector{num_dim}(fill(1, num_dim)))
push!(non_data_array_sizes, MVector{num_dim}(fill(0, num_dim)))
end
end

error_on_undeclared_variables(model_def, non_data_scalars, non_data_arrays)

return CollectVariables{data_vars}(
data,
Tuple(data_scalars),
Expand All @@ -112,6 +126,28 @@ function CollectVariables(model_def::Expr, data::NamedTuple{data_vars}) where {d
)
end

function error_on_undeclared_variables(model_def::Expr, non_data_scalars, non_data_arrays)
logical_scalars, stochastic_scalars, logical_arrays, stochastic_arrays = extract_variables_assigned_to(
model_def
)

for scalar in non_data_scalars
if !(scalar in union(logical_scalars, stochastic_scalars))
error(
"Scalar variable $scalar is used in the model, but it is not defined in the model or data.",
)
end
end

for array in non_data_arrays
if !(array in union(logical_arrays, stochastic_arrays))
error(
"Array variable $array is used in the model, but it is not defined in the model or data.",
)
end
end
end

"""
evaluate(expr, env)
Expand Down Expand Up @@ -172,6 +208,13 @@ function evaluate(expr::Expr, env::NamedTuple{variable_names}) where {variable_n
end
if var in variable_names
if all_resolved
_indices = map(i -> i === Colon() ? 0 : last(i), indices) # if index is `:`, then we skip the check
if any(_indices .> size(env[var]))
@show indices size(env[var])
error(
"Variable $var[$(join(indices, ", "))] is used in the model, but it or its elements are not defined in the model or data.",
)
end
value = env[var][indices...]
if is_resolved(value)
return value
Expand Down Expand Up @@ -546,9 +589,12 @@ function evaluate_and_track_dependencies(var::Expr, env)
end

value = nothing
if all(indices) do i
i isa Int || i isa UnitRange{Int}
end
if all(i -> i isa Int || i isa UnitRange{Int}, indices)
if any(last.(indices) .> size(env[v]))
error(
"$v[$(join(indices, ", "))] is used in the model, but it or its elements are not defined in the model or data.",
)
end
value = env[v][indices...]
if is_resolved(value)
return value, Tuple(dependencies)
Expand Down Expand Up @@ -735,9 +781,7 @@ function analyze_statement(pass::AddVertices, expr::Expr, loop_vars::NamedTuple)
)
else
v, indices... = lhs
if any(indices) do i
i isa UnitRange
end
if any(i -> i isa UnitRange, indices)
pass.vertex_id_tracker[v][indices...] .= code_for(pass.g, vn)
else
pass.vertex_id_tracker[v][indices...] = code_for(pass.g, vn)
Expand Down Expand Up @@ -781,15 +825,27 @@ function analyze_statement(pass::AddEdges, expr::Expr, loop_vars::NamedTuple)

for var in dependencies
vertex_code = if var isa Symbol
pass.vertex_id_tracker[var]
_vertex_code = pass.vertex_id_tracker[var]
if _vertex_code isa AbstractArray
error("$(var) is an array, but referenced as a scalar at $expr")
end
[_vertex_code]
else
v, indices... = var
pass.vertex_id_tracker[v][indices...]
for idx in Iterators.product(indices...)
if iszero(pass.vertex_id_tracker[v][idx...]) && ismissing(pass.env[v][idx...])
error(
"Variable $v[$(join(indices, ", "))] is referenced, but not defined, at $expr.",
)
end
end
_vertex_code = pass.vertex_id_tracker[v][indices...]
if _vertex_code isa AbstractArray
filter(!iszero, _vertex_code)
else
iszero(_vertex_code) ? [] : [_vertex_code]
end
end

vertex_code = filter(
!iszero, vertex_code isa AbstractArray ? vertex_code : [vertex_code]
)
vertex_labels = [label_for(pass.g, code) for code in vertex_code]
for r in vertex_labels
if r != lhs_vn
Expand Down
75 changes: 59 additions & 16 deletions test/compile.jl
View file
Original file line number Diff line number Diff line change
@@ -1,26 +1,69 @@
@testset "compile corner cases" begin
# test variables exist on the left hand side of the both kinds of assignment
let ex = @bugs begin
a ~ Normal(0, 1)
b = a
b ~ Normal(0, 1)
@testset "test variables exist on the left hand side of the both kinds of assignment" begin
@testset "not transformed variable, so error" begin
ex = @bugs begin
a ~ Normal(0, 1)
b = a
b ~ Normal(0, 1)
end
@test_throws ErrorException compile(ex, (;), (;))
end

@testset "transformed variable, so no error" begin
ex = @bugs begin
a ~ Normal(0, 1)
b = a
b ~ Normal(0, 1)
end
compile(ex, (; a=1), (;))
end
@test_throws ErrorException compile(ex, (;), (;))
end

let ex = @bugs begin
a ~ Normal(0, 1)
b = a
b ~ Normal(0, 1)
@testset "assign array variable to another array variable" begin
model = compile(
(@bugs begin
b[1:2] ~ dmnorm(μ[:], σ[:, :])
a[1:2] = b[:]
end), (; μ=[0, 1], σ=[1 0; 0 1]), (;)
)
end
end

@testset "error messages on undeclared variables" begin
@testset "same variable names refer to both scalar and array" begin
model_def = @bugs begin
x[1] ~ dnorm(0, 1)
y ~ dnorm(x, 1)
end
compile(ex, (; a=1), (;))
@test_throws ErrorException compile(model_def, (;))
end

# assign array variable to another array variable
model = compile((@bugs begin
b[1:2] ~ dmnorm(μ[:], σ[:, :])
a[1:2] = b[:]
end), (; μ=[0, 1], σ=[1 0; 0 1]), (;))
@testset "undeclared scalar variable" begin
model_def = @bugs begin
x[1] ~ dnorm(0, 1)
y ~ dnorm(x[1], z)
end
@test_throws ErrorException compile(model_def, (;))
end
@testset "undeclared array variable" begin
model_def = @bugs begin
x[1] ~ dnorm(0, 1)
y ~ dnorm(0, x[2])
end
@test_throws ErrorException compile(model_def, (;))

model_def = @bugs begin
x[2] ~ dnorm(0, 1)
y ~ dnorm(x[1], 0)
end
@test_throws ErrorException compile(model_def, (;))

model = @bugs begin
x = sum(y[1:2])
y[1] ~ dnorm(0, 1)
end
@test_throws ErrorException compile(model, (;))
end
end

@testset "initialize!" begin
Expand Down
80 changes: 42 additions & 38 deletions test/passes.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,53 +106,57 @@ end
@testset "error cases" begin
data = (;)

model_def = @bugs begin
a = 1
a = 2
@testset "repeated scalar logical assignment" begin
model_def = @bugs begin
a = 1
a = 2
end
@test_throws ErrorException JuliaBUGS.determine_array_sizes(model_def, data)
end
scalars, array_sizes = JuliaBUGS.determine_array_sizes(model_def, data)
@test_throws ErrorException JuliaBUGS.check_repeated_assignments(
model_def, data, array_sizes
)

model_def = @bugs begin
a ~ Normal(0, 1)
a ~ Normal(0, 2)
@testset "repeated scalar stochastic assignment" begin
model_def = @bugs begin
a ~ Normal(0, 1)
a ~ Normal(0, 2)
end
@test_throws ErrorException JuliaBUGS.determine_array_sizes(model_def, data)
end
scalars, array_sizes = JuliaBUGS.determine_array_sizes(model_def, data)
@test_throws ErrorException JuliaBUGS.check_repeated_assignments(
model_def, data, array_sizes
)

model_def = @bugs begin
x[1] = 1
x[1] = 2
@testset "repeated array element assignment" begin
model_def = @bugs begin
x[1] = 1
x[1] = 2
end
scalars, array_sizes = JuliaBUGS.determine_array_sizes(model_def, data)
@test_throws ErrorException JuliaBUGS.check_repeated_assignments(
model_def, data, array_sizes
)
end
scalars, array_sizes = JuliaBUGS.determine_array_sizes(model_def, data)
@test_throws ErrorException JuliaBUGS.check_repeated_assignments(
model_def, data, array_sizes
)

model_def = @bugs begin
x[1] = 1
for i in 1:2
x[i:(i + 1)] = i

@testset "repeated array assignment with range" begin
model_def = @bugs begin
x[1] = 1
for i in 1:2
x[i:(i + 1)] = i
end
end
scalars, array_sizes = JuliaBUGS.determine_array_sizes(model_def, data)
@test_throws ErrorException JuliaBUGS.check_repeated_assignments(
model_def, data, array_sizes
)
end
scalars, array_sizes = JuliaBUGS.determine_array_sizes(model_def, data)
@test_throws ErrorException JuliaBUGS.check_repeated_assignments(
model_def, data, array_sizes
)

model_def = @bugs begin
x[1] ~ Normal(0, 1)
x[1:2] ~ MvNormal(a[:], b[:, :])
@testset "repeated array assignment with range" begin
model_def = @bugs begin
x[1] ~ Normal(0, 1)
x[1:2] ~ MvNormal(a[:], b[:, :])
end
data = (a=[1, 2], b=[1 0; 0 1])
scalars, array_sizes = JuliaBUGS.determine_array_sizes(model_def, data)
@test_throws ErrorException JuliaBUGS.check_repeated_assignments(
model_def, data, array_sizes
)
end
data = (a=[1, 2], b=[1 0; 0 1])
scalars, array_sizes = JuliaBUGS.determine_array_sizes(model_def, data)
@test_throws ErrorException JuliaBUGS.check_repeated_assignments(
model_def, data, array_sizes
)
end
end

Expand Down

2 comments on commit 46663e8

@sunxd3
Copy link
Member Author

@sunxd3 sunxd3 commented on 46663e8 Sep 17, 2024

Choose a reason for hiding this comment

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

@JuliaRegistrator register

@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/115337

Tip: Release Notes

Did you know you can add release notes too? Just add markdown formatted text underneath the comment after the text
"Release notes:" and it will be added to the registry PR, and if TagBot is installed it will also be added to the
release that TagBot creates. i.e.

@JuliaRegistrator register

Release notes:

## Breaking changes

- blah

To add them here just re-invoke and the PR will be updated.

Tagging

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.6.1 -m "<description of version>" 46663e804a6e26f184e382df6e6ba30cfc85b185
git push origin v0.6.1

Please sign in to comment.