Merge pull request #69 from Herb-AI/bug/propagate-on-filled-holes

Fix propagate on filled holes

Project.toml

@@ -1,7 +1,7 @@
 name = "HerbConstraints"
 uuid = "1fa96474-3206-4513-b4fa-23913f296dfc"
 authors = ["Jaap de Jong <[email protected]>"]
-version = "0.2.4"
+version = "0.2.5"
 
 [deps]
 DataStructures = "864edb3b-99cc-5e75-8d2d-829cb0a9cfe8"

src/lessthanorequal.jl

@@ -100,13 +100,25 @@ function make_less_than_or_equal!(
     @assert isfeasible(solver)
     @match (isfilled(hole1), isfilled(hole2)) begin
         (true, true) => begin
-            #(RuleNode, RuleNode)
+            #(RuleNode | Hole [domain size == 1], RuleNode | Hole [domain size == 1])
             if get_rule(hole1) < get_rule(hole2)
                 return LessThanOrEqualSuccessLessThan()
             elseif get_rule(hole1) > get_rule(hole2)
                 return LessThanOrEqualHardFail()
             end
-            return make_less_than_or_equal!(solver, hole1.children, hole2.children, guards)
+
+            # domain of hole must have been 1 because it `isfilled`
+            # but still has no children, so there's nothing more that
+            # can be deduced before the hole is simplified
+            # ideally, these holes should be simplified before making
+            # it to the lessthanorequal step.
+            if hole1 isa Hole && !isempty(get_children(hole2))
+                return LessThanOrEqualSoftFail(hole1)
+            elseif hole2 isa Hole && !isempty(get_children(hole1))
+                return LessThanOrEqualSoftFail(hole2)
+            end
+
+            return make_less_than_or_equal!(solver, get_children(hole1), get_children(hole2), guards)
         end
         (true, false) => begin
             #(RuleNode, AbstractHole)

src/patternmatch.jl

@@ -168,11 +168,23 @@ The '(RuleNode, AbstractHole)' case could still include two nodes of type `Abstr
 """
 function pattern_match(h1::Union{RuleNode, AbstractHole}, h2::Union{RuleNode, AbstractHole}, vars::Dict{Symbol, AbstractRuleNode})::PatternMatchResult
     @match (isfilled(h1), isfilled(h2)) begin
-        #(RuleNode, RuleNode)
+        #(RuleNode | Hole [domain size == 1], RuleNode | Hole [domain size == 1])
         (true, true) => begin
             if get_rule(h1) ≠ get_rule(h2)
                 return PatternMatchHardFail()
             end
+
+            # domain of hole must have been 1 because it `isfilled`
+            # but still has no children, so there's nothing more that
+            # can be deduced before the hole is simplified
+            # ideally, these holes should be simplified before making
+            # it to the pattern matching step.
+            if h1 isa Hole && !isempty(get_children(h2))
+                return PatternMatchSoftFail(h1)
+            elseif h2 isa Hole && !isempty(get_children(h1))
+                return PatternMatchSoftFail(h2)
+            end
+
             return pattern_match(get_children(h1), get_children(h2), vars)
         end
 

src/solver/generic_solver/treemanipulations.jl

@@ -247,6 +247,9 @@ function simplify_hole!(solver::GenericSolver, path::Vector{Int})
 
     #the hole will be simplified and replaced with a `new_node`
     if !isnothing(new_node)
+        # Ideally, we should try to simplify holes with domain size of 1 here
+        # before substituting. This would remove some duplicated logic when calling
+        # pattern_match and lessthanorequal
         substitute!(solver, path, new_node, is_domain_increasing=false)
         for i ∈ 1:length(new_node.children)
             # try to simplify the new children

test/test_lessthanorequal.jl

@@ -121,6 +121,40 @@ using HerbCore, HerbGrammar
         @test HerbConstraints.make_less_than_or_equal!(solver, left, right) isa HerbConstraints.LessThanOrEqualSoftFail
     end
 
+    @testset "left hole softfails chemistry grammar" begin
+        grammar = @csgrammar begin
+            Species = "A" #1
+            Species = "B" #2
+
+            SpeciesList = Species #3
+            SpeciesList = SpeciesList + SpeciesList #4
+            Reaction = SpeciesList --> SpeciesList #5
+
+            ReactionList = Reaction #6
+            ReactionList = ReactionList + ReactionList #7
+        end
+
+        left = Hole(BitVector((0, 0, 0, 0, 1, 0, 0)))
+        right = Hole(BitVector((0, 0, 0, 0, 1, 0, 0)))
+
+        solver = GenericSolver(grammar, :ReactionList)
+        tree = RuleNode(7, [
+            RuleNode(6, [
+               left 
+            ]),
+            RuleNode(6, [right])
+        ])
+
+        leftpath = get_path(tree, left)
+        rightpath = get_path(tree, right)
+        new_state!(solver, tree)
+        left = get_node_at_location(solver, leftpath) #leftnode might have been simplified by `new_state!`
+        left = get_node_at_location(solver, rightpath) #rightnode might have been simplified by `new_state!`
+
+        @test HerbConstraints.make_less_than_or_equal!(solver, left, right) isa HerbConstraints.LessThanOrEqualSoftFail
+        @test HerbConstraints.make_less_than_or_equal!(solver, right, left) isa HerbConstraints.LessThanOrEqualSoftFail
+    end
+
     @testset "left hole gets filled once, two holes remain" begin
         left = Hole(BitVector((0, 0, 1, 1)))
         right = RuleNode(3, [RuleNode(2), RuleNode(2)])

test/test_pattern_match.jl

@@ -219,6 +219,13 @@ using HerbGrammar
         @test pattern_match(rn2, mn) isa HerbConstraints.PatternMatchSoftFail
     end
 
+    @testset "PatternMatchSoftFail, 1 hole with a domain of 1, matching rulenode" begin
+        rn = Hole([0, 0, 0, 1])
+        mn = RuleNode(4, [VarNode(:a), VarNode(:a)])
+        @test pattern_match(rn, mn) isa HerbConstraints.PatternMatchSoftFail
+        @test pattern_match(mn, rn) isa HerbConstraints.PatternMatchSoftFail
+    end
+
     @testset "PatternMatchSoftFail, 2 holes with valid domains" begin
         rn = RuleNode(4, [Hole(BitVector((1, 1, 1, 1, 1, 1))), Hole(BitVector((1, 1, 1, 1, 1, 1)))])
         mn = RuleNode(4, [RuleNode(1), RuleNode(1)])