From 6c09034c45ab4f717104426b514146e4ec2af66f Mon Sep 17 00:00:00 2001 From: Reuben Gardos Reid <5456207+ReubenJ@users.noreply.github.com> Date: Fri, 17 Jan 2025 17:17:20 +0100 Subject: [PATCH 1/3] Add failing test for filled holes --- test/test_lessthanorequal.jl | 34 ++++++++++++++++++++++++++++++++++ test/test_pattern_match.jl | 7 +++++++ 2 files changed, 41 insertions(+) diff --git a/test/test_lessthanorequal.jl b/test/test_lessthanorequal.jl index c4b9ab6..9372652 100644 --- a/test/test_lessthanorequal.jl +++ b/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)]) diff --git a/test/test_pattern_match.jl b/test/test_pattern_match.jl index fa7fe61..774cfb3 100644 --- a/test/test_pattern_match.jl +++ b/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)]) From c77fa43d9589763033116c1e4c75a8be1201b67a Mon Sep 17 00:00:00 2001 From: Reuben Gardos Reid <5456207+ReubenJ@users.noreply.github.com> Date: Fri, 17 Jan 2025 17:17:47 +0100 Subject: [PATCH 2/3] Fix bug constraints propagating on filled holes --- src/lessthanorequal.jl | 16 ++++++++++++++-- src/patternmatch.jl | 14 +++++++++++++- src/solver/generic_solver/treemanipulations.jl | 3 +++ 3 files changed, 30 insertions(+), 3 deletions(-) diff --git a/src/lessthanorequal.jl b/src/lessthanorequal.jl index 0596f74..794c2de 100644 --- a/src/lessthanorequal.jl +++ b/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) diff --git a/src/patternmatch.jl b/src/patternmatch.jl index 7e643b3..bd9c79f 100644 --- a/src/patternmatch.jl +++ b/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 diff --git a/src/solver/generic_solver/treemanipulations.jl b/src/solver/generic_solver/treemanipulations.jl index ca386ea..c4ce357 100644 --- a/src/solver/generic_solver/treemanipulations.jl +++ b/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 From 9bf2bd795bea18cbd0eab653842179149e8fc5f4 Mon Sep 17 00:00:00 2001 From: Reuben Gardos Reid <5456207+ReubenJ@users.noreply.github.com> Date: Thu, 23 Jan 2025 17:02:27 +0100 Subject: [PATCH 3/3] Bump patch version number --- Project.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Project.toml b/Project.toml index e684f65..6f47cd6 100644 --- a/Project.toml +++ b/Project.toml @@ -1,7 +1,7 @@ name = "HerbConstraints" uuid = "1fa96474-3206-4513-b4fa-23913f296dfc" authors = ["Jaap de Jong "] -version = "0.2.4" +version = "0.2.5" [deps] DataStructures = "864edb3b-99cc-5e75-8d2d-829cb0a9cfe8"