more tests to the god of tests

Author: bvdmitri

src/graph_engine.jl

@@ -962,6 +962,11 @@ function add_edge!(model::Model, src, dst, data)
     return Graphs.add_edge!(model.graph.graph, code_src, code_dst)
 end
 
+function has_edge(model::Model, src, dst)
+    code_src, code_dst = MetaGraphsNext.code_for(model.graph, src), MetaGraphsNext.code_for(model.graph, dst)
+    return Graphs.has_edge(model.graph.graph, code_src, code_dst)
+end
+
 """
     copy_markov_blanket_to_child_context(child_context::Context, interfaces::NamedTuple)
 
@@ -1511,11 +1516,16 @@ function add_edge!(
     label = EdgeLabel(interface_name, index)
     neighbor_node_label = unroll(variable_node_id)
     addneighbor!(factor_node_propeties, neighbor_node_label, label, model[neighbor_node_label])
-    edge_added = add_edge!(model, unroll(variable_node_id), factor_node_id, label)
+    edge_added = add_edge!(model, neighbor_node_label, factor_node_id, label)
     if !edge_added
-        error(
-            lazy"Trying to create duplicate edge ($(unroll(variable_node_id)), $(factor_node_id)) while creating edge $(label) of factor node with functional form $(factor_node_propeties.fform)"
-        )
+        # Double check if the edge has already been added
+        if has_edge(model, neighbor_node_label, factor_node_id)
+            error(
+                lazy"Trying to create duplicate edge $(label) between variable $(neighbor_node_label) and factor node $(factor_node_id). Make sure that all the arguments to the `~` operator are unique (both left hand side and right hand side)."
+            )
+        else
+            error(lazy"Cannot create an edge $(label) between variable $(neighbor_node_label) and factor node $(factor_node_id).")
+        end
     end
     return label
 end

test/graph_construction_tests.jl

@@ -1524,46 +1524,89 @@ end
 
     include("testutils.jl")
 
+    @model function simple_model_duplicate_1()
+        x ~ Normal(0.0, 1.0)
+        y ~ x + x
+    end
+
+    @model function simple_model_duplicate_2()
+        x ~ Normal(0.0, 1.0)
+        y ~ x + x + x
+    end
+
+    @model function simple_model_duplicate_3()
+        x ~ Normal(0.0, 1.0)
+        y ~ Normal(x, x)
+    end
+
+    @model function simple_model_duplicate_4()
+        x ~ Normal(0.0, 1.0)
+        hide_x = x
+        y ~ Normal(hide_x, x)
+    end
+
+    @model function simple_model_duplicate_5()
+        x ~ Normal(0.0, 1.0)
+        x ~ Normal(x, 1)
+    end
+
+    @model function simple_model_duplicate_6()
+        x ~ Normal(0.0, 1.0)
+        hide_x = x
+        hide_x ~ Normal(x, 1)
+    end
+
+    for modelfn in [
+        simple_model_duplicate_1,
+        simple_model_duplicate_2,
+        simple_model_duplicate_3,
+        simple_model_duplicate_4,
+        simple_model_duplicate_5,
+        simple_model_duplicate_6
+    ]
+        @test_throws r"Trying to create duplicate edge.*Make sure that all the arguments to the `~` operator are unique.*" create_model(
+            modelfn()
+        )
+    end
+
     @model function my_model(obs, N, sigma)
-        local p
-        for i in 1:N
-            p[i] ~ Beta(1, 1)
-        end
         local x
         for i in 1:N
-            x[i] ~ Bernoulli(p[i])
+            x[i] ~ Bernoulli(0.5)
         end
         local C
+        # This model creation is not allowed since `C` is used twice in the `~` operator
         for i in 1:N
             C ~ C + x[i]
         end
         obs ~ NormalMeanVariance(C, sigma^2)
     end
 
-    @test_throws "Trying to create duplicate edge" create_model(my_model(N = 3, sigma = 1.0)) do model, ctx
+    @test_throws r"Trying to create duplicate edge.*Make sure that all the arguments to the `~` operator are unique.*" create_model(
+        my_model(N = 3, sigma = 1.0)
+    ) do model, ctx
         obs = getorcreate!(model, ctx, NodeCreationOptions(kind = :data, factorized = true), :obs, LazyIndex(0.0))
         return (obs = obs,)
     end
 
     @model function my_model(obs, N, sigma)
-        local p
-        for i in 1:N
-            p[i] ~ Beta(1, 1)
-        end
         local x
         for i in 1:N
-            x[i] ~ Bernoulli(p[i])
+            x[i] ~ Bernoulli(0.5)
         end
         accum_C = x[1]
         for i in 2:N
+            # Here `next_C` will be used twice on the second iteration 
             next_C ~ accum_C + x[i]
             accum_C = next_C
         end
         obs ~ NormalMeanVariance(accum_C, sigma^2)
     end
 
-    @test_throws "Trying to create duplicate edge" create_model(my_model(N = 3, sigma = 1.0)) do model, ctx
+    @test_throws r"Trying to create duplicate edge.*Make sure that all the arguments to the `~` operator are unique.*" create_model(
+        my_model(N = 3, sigma = 1.0)
+    ) do model, ctx
         obs = getorcreate!(model, ctx, NodeCreationOptions(kind = :data, factorized = true), :obs, LazyIndex(0.0))
         return (obs = obs,)
     end
-end
+end

test/graph_engine_tests.jl

@@ -759,6 +759,7 @@ end
         EdgeLabel,
         getname,
         add_edge!,
+        has_edge,
         getproperties
 
     include("testutils.jl")
@@ -770,12 +771,21 @@ end
     b = NodeLabel(:b, 2)
     model[a] = NodeData(ctx, VariableNodeProperties(name = :a, index = nothing))
     model[b] = NodeData(ctx, FactorNodeProperties(fform = sum))
+    @test !has_edge(model, a, b)
+    @test !has_edge(model, b, a)
     add_edge!(model, b, getproperties(model[b]), a, :edge, 1)
+    @test has_edge(model, a, b)
+    @test has_edge(model, b, a)
     @test length(edges(model)) == 1
 
     c = NodeLabel(:c, 2)
     model[c] = NodeData(ctx, FactorNodeProperties(fform = sum))
+    @test !has_edge(model, a, c)
+    @test !has_edge(model, c, a)
     add_edge!(model, c, getproperties(model[c]), a, :edge, 2)
+    @test has_edge(model, a, c)
+    @test has_edge(model, c, a)
+
     @test length(edges(model)) == 2
 
     # Test 2: Test getting all edges from a model with a specific node