Fixes from review

.dev/Project.toml

@@ -1,2 +1,3 @@
 [deps]
+Git = "d7ba0133-e1db-5d97-8f8c-041e4b3a1eb2"
 JuliaFormatter = "98e50ef6-434e-11e9-1051-2b60c6c9e899"

Project.toml

@@ -7,4 +7,6 @@ DocStringExtensions = "ffbed154-4ef7-542d-bbb7-c09d3a79fcae"
 Herb = "c09c6b7f-4f63-49de-90d9-97a3563c0f4a"
 
 [compat]
+DocStringExtensions = "0.9.3"
+Herb = "0.4.1"
 julia = "^1.8"

src/Garden.jl

@@ -1,13 +1,9 @@
 module Garden
 
 using DocStringExtensions
-using Herb
 
-include("utils.jl")
 include("frangel/method.jl")
-export 
-    frangel, 
-    decide_frangel, 
-    modify_grammar_frangel!,
-    get_promising_programs
+
+export FrAngel
+
 end # module Garden

src/frangel/method.jl

@@ -1,23 +1,30 @@
-using Herb.HerbCore: AbstractRuleNode, AbstractGrammar, get_rule
+module FrAngel
+
+using DocStringExtensions
+using Herb.HerbCore: AbstractRuleNode, AbstractGrammar, get_rule, isfilled, depth
 using Herb.HerbSpecification: AbstractSpecification, Problem
 using Herb.HerbSearch: ProgramIterator, evaluate
 using Herb.HerbInterpret: SymbolTable
 using Herb.HerbConstraints: get_grammar, freeze_state
-using Herb.HerbGrammar: rulenode2expr, isterminal, iscomplete, add_rule!
+using Herb.HerbGrammar: ContextSensitiveGrammar, grammar2symboltable, rulenode2expr,
+                        isterminal, iscomplete, add_rule!
 
 @enum SynthResult optimal_program=1 suboptimal_program=2
 
 struct NoProgramFoundError <: Exception
     message::String
 end
 
-
 """
     $(TYPEDSIGNATURES)
 
 Synthesize a program using the `grammar` that follows the `spec` following the method from 
 ["FrAngel: component-based synthesis with control structures"](https://doi.org/10.1145/3290386).
 
+!!! note
+
+    This implementation includes fragment mining but *excludes angelic conditions*!
+
 Constructs an iterator of type `iterator_type`. Iteratively, collects promising programs, mines fragments from them and adds them to the grammar. 
 This is then re-started with the updated grammar.
 
@@ -31,19 +38,21 @@ function frangel(
         grammar::AbstractGrammar,
         starting_sym::Symbol,
         problem::Problem;
-        max_iterations::Int=typemax(Int),
-        frangel_iterations::Int=3,
-        max_iteration_time::Int=typemax(Int),
+        max_iterations::Int = typemax(Int),
+        frangel_iterations::Int = 3,
+        max_iteration_time::Int = typemax(Int),
         kwargs...
-)::Union{AbstractRuleNode, Nothing} where T<:ProgramIterator
+)::Union{AbstractRuleNode, Nothing} where {T <: ProgramIterator}
     # FrAngel config arguments
 
-    for frangel_iteration in 1:frangel_iterations
+    for _ in 1:frangel_iterations
         # Gets an iterator with some limit (the low-level budget)
-        iterator = construct_iterator(iterator_type, grammar, starting_sym; kwargs...)
+        iterator = iterator_type(grammar, starting_sym; kwargs...)
 
         # Run a budgeted search
-        promising_programs, result_flag = get_promising_programs(iterator, problem; max_time=max_iteration_time, max_enumerations=max_iterations)
+        promising_programs, result_flag = get_promising_programs(
+            iterator, problem; max_time = max_iteration_time,
+            max_enumerations = max_iterations)
 
         if result_flag == optimal_program
             return only(promising_programs) # returns the only element
@@ -64,11 +73,10 @@ function frangel(
         modify_grammar_frangel!(selected_fragments, grammar)
     end
 
-    @warn "No solution found."
+    @warn "No solution found. Within $frangel_iterations iterations."
     return nothing
 end
 
-
 """
     $(TYPEDSIGNATURES)
 
@@ -82,7 +90,7 @@ function decide_frangel(
         symboltable::SymbolTable
 )
     expr = rulenode2expr(program, grammar)
-    score = evaluate(problem, expr, symboltable, shortcircuit=false)
+    score = evaluate(problem, expr, symboltable, shortcircuit = false)
     return score
 end
 
@@ -95,9 +103,8 @@ This function adds "Fragment_{type}" to the grammar to denote added fragments.
 function modify_grammar_frangel!(
         fragments::AbstractVector{<:AbstractRuleNode},
         grammar::AbstractGrammar;
-        max_fragment_rules::Int=typemax(Int)
+        max_fragment_rules::Int = typemax(Int)
 )
-
     for f in fragments
         ind = get_rule(f)
         type = grammar.types[ind]
@@ -121,13 +128,13 @@ Selects the smallest (fewest number of nodes) fragments from the set of mined fr
 `num_programs` determines how many programs should be selected.
 """
 function select_smallest_fragments(
-    fragments::Set{AbstractRuleNode};
-    num_programs::Int=3
+        fragments::Set{AbstractRuleNode};
+        num_programs::Int = 3
 )::AbstractVector{<:AbstractRuleNode}
     sorted_nodes = sort(collect(fragments), by = x -> length(x))
-    
+
     # Select the top 3 elements
-    return sorted_nodes[1:min(num_programs, length(sorted_nodes))] 
+    return sorted_nodes[1:min(num_programs, length(sorted_nodes))]
 end
 
 """
@@ -137,16 +144,18 @@ Selects the shallowest (smallest depth) fragments from the set of mined fragment
 `num_programs` determines how many programs should be selected.
 """
 function select_shallowest_fragments(
-    fragments::Set{AbstractRuleNode};
-    num_programs::Int=3
+        fragments::Set{AbstractRuleNode};
+        num_programs::Int = 3
 )::AbstractVector{<:AbstractRuleNode}
     sorted_nodes = sort(collect(fragments), by = x -> depth(x))
-    
+
     # Select the top 3 elements
-    return sorted_nodes[1:min(num_programs, length(sorted_nodes))] 
+    return sorted_nodes[1:min(num_programs, length(sorted_nodes))]
 end
 
-select_fragments(fragments::Set{AbstractRuleNode}) = select_smallest_fragments(fragments; num_programs=3)
+function select_fragments(fragments::Set{AbstractRuleNode})
+    select_smallest_fragments(fragments; num_programs = 3)
+end
 
 """
     $(TYPEDSIGNATURES)
@@ -158,34 +167,31 @@ If a program solves some of the problem (e.g. some but not all examples) it is a
 The set of promising programs is returned eventually.
 """
 function get_promising_programs(
-    iterator::ProgramIterator,
-    problem::Problem;
-    max_time = typemax(Int),
-    max_enumerations = typemax(Int),
-    mod::Module=Main
+        iterator::ProgramIterator,
+        problem::Problem;
+        max_time = typemax(Int),
+        max_enumerations = typemax(Int),
+        mod::Module = Main
 )::Tuple{Set{AbstractRuleNode}, SynthResult}
     start_time = time()
     grammar = get_grammar(iterator.solver)
-    symboltable :: SymbolTable = grammar2symboltable(grammar, mod)
+    symboltable::SymbolTable = grammar2symboltable(grammar, mod)
 
     promising_programs = Set{AbstractRuleNode}()
 
-    for (i, candidate_program) ∈ enumerate(iterator)
-        # Create expression from rulenode representation of AST
-
-        # Evaluate the expression
-
+    for (i, candidate_program) in enumerate(iterator)
         score = decide_frangel(candidate_program, problem, grammar, symboltable)
+
         if score == 1
             push!(promising_programs, freeze_state(candidate_program))
-            return (promising_programs, optimal_program) 
+            return (promising_programs, optimal_program)
         elseif score > 0
             push!(promising_programs, freeze_state(candidate_program))
         end
 
         # Check stopping criteria
         if i > max_enumerations || time() - start_time > max_time
-            break;
+            break
         end
     end
 
@@ -200,10 +206,11 @@ Finds all the fragments from the `program` defined over the `grammar`.
 The result is a set of the distinct program fragments, generated recursively by iterating over all children. A fragment is any complete subprogram of the original program.
 
 """
-function mine_fragments(grammar::AbstractGrammar, program::AbstractRuleNode)::Set{AbstractRuleNode}
+function mine_fragments(
+        grammar::AbstractGrammar, program::AbstractRuleNode)::Set{AbstractRuleNode}
     fragments = Set{AbstractRuleNode}()
     # Push terminals as they are
-    if !isterminal(grammar, program)
+    if isfilled(program) && !isterminal(grammar, program)
         # Only complete programs count are considered
         if iscomplete(grammar, program)
             push!(fragments, program)
@@ -212,19 +219,27 @@ function mine_fragments(grammar::AbstractGrammar, program::AbstractRuleNode)::Se
             fragments = union(fragments, mine_fragments(grammar, child))
         end
     end
-    fragments
+
+    return fragments
 end
 
 """
     $(TYPEDSIGNATURES)
 
-Finds fragments from the set of programs. Internally uses [`mine_fragments`](@ref) on each element of the set. 
-Returns a set of all fragments found.
+Finds fragments (subprograms) of each program in `programs`.
 """
-function mine_fragments(grammar::AbstractGrammar, programs::Set{AbstractRuleNode})::Set{AbstractRuleNode}
+function mine_fragments(
+        grammar::AbstractGrammar, programs::Set{<:AbstractRuleNode})::Set{AbstractRuleNode}
     fragments = reduce(union, mine_fragments(grammar, p) for p in programs)
-    for program in programs
-        delete!(fragments, program)
-    end
-    fragments
+    fragments = setdiff(fragments, programs) # Don't include the programs themselves in the set of fragments
+
+    return fragments
+end
+
+export
+       frangel,
+       decide_frangel,
+       modify_grammar_frangel!,
+       get_promising_programs
+
 end

test/test_frangel.jl

@@ -1,8 +1,11 @@
 using Herb.HerbGrammar: @cfgrammar, rulenode2expr
 using Herb.HerbSearch: BFSIterator
 using Herb.HerbSpecification: IOExample, Problem
-using Herb.HerbCore: RuleNode
-using Garden: mine_fragments, select_shallowest_fragments, select_smallest_fragments, modify_grammar_frangel!, decide_frangel, NoProgramFoundError
+using Herb.HerbCore: RuleNode, Hole, @rulenode
+using Garden: FrAngel
+using .FrAngel: frangel, mine_fragments, select_shallowest_fragments,
+                select_smallest_fragments, modify_grammar_frangel!, decide_frangel,
+                NoProgramFoundError
 
 @testset verbose=true "FrAngel" begin
     @testset "Integration tests" begin
@@ -20,17 +23,18 @@ using Garden: mine_fragments, select_shallowest_fragments, select_smallest_fragm
             grammar,
             :Start,
             problem;
-            max_depth=4
+            max_depth = 4
         )
 
-        @test rulenode2expr(result,grammar) == 2
+        @test rulenode2expr(result, grammar) == 2
 
         imp_problem = Problem(
             [IOExample{Symbol, Any}(Dict(), 0)]
         )
- 
+
         # A program yielding 0 is impossible to derive from the grammar.
-        @test_throws NoProgramFoundError frangel(BFSIterator, grammar, :Start, imp_problem; max_depth=4)
+        @test_throws NoProgramFoundError frangel(
+            BFSIterator, grammar, :Start, imp_problem; max_depth = 4)
     end
 
     grammar = @cfgrammar begin
@@ -40,41 +44,39 @@ using Garden: mine_fragments, select_shallowest_fragments, select_smallest_fragm
     end
 
     @testset "mine_fragments" begin
-        rn = RuleNode(2,
-            [RuleNode(2,[RuleNode(3),RuleNode(4)]),RuleNode(3)]
-        ) # 2{2{3,4},3}
+        rn = @rulenode 2{2{3, 4}, 3}
+
+        fragments = mine_fragments(grammar, rn)
+        @test rn in fragments
+        @test !((@rulenode 3) in fragments)
+        @test !((@rulenode 3) in fragments)
+        @test (@rulenode 2{3, 4}) in fragments
 
-        fragments = collect(mine_fragments(grammar, rn))
-        @test fragments[1] == rn
-        @test fragments[2] == RuleNode(2,[RuleNode(3),RuleNode(4)])
+        complete = @rulenode 2{3, 4}
+        rn_hole = RuleNode(2, [
+            complete, Hole([0, 0, 1, 1])
+        ])
+        fragments_hole = mine_fragments(grammar, rn_hole)
+        @test !(rn_hole in fragments_hole)
+        @test complete in fragments_hole
     end
 
     @testset "select_shallowest_fragments" begin
-        rn = RuleNode(2, [
-            RuleNode(2, [RuleNode(2, [
-                RuleNode(2, [RuleNode(3), RuleNode(4)]),
-                RuleNode(3)]), 
-                RuleNode(4)]),
-            RuleNode(3)
-        ])
+        rn = @rulenode 2{2{2{2{3, 4}, 3}, 4}, 3}
 
         fragments = mine_fragments(grammar, rn)
 
-        selected_fragments = select_shallowest_fragments(fragments; num_programs=3)
+        selected_fragments = select_shallowest_fragments(fragments; num_programs = 3)
 
-        @test selected_fragments[1] == RuleNode(2, [RuleNode(3), RuleNode(4)])
-        @test selected_fragments[2] == RuleNode(2, [
-                RuleNode(2, [RuleNode(3), RuleNode(4)]), RuleNode(3)]) 
-        @test selected_fragments[3] == RuleNode(2, [RuleNode(2, [
-                RuleNode(2, [RuleNode(3), RuleNode(4)]), RuleNode(3)]), 
-                RuleNode(4)]
-            )
-        end
+        @test selected_fragments[1] == @rulenode 2{3, 4}
+        @test selected_fragments[2] == @rulenode 2{2{3, 4}, 3}
+        @test selected_fragments[3] == @rulenode 2{2{2{3, 4}, 3}, 4}
+    end
 
     @testset "modify_grammar_frangel" begin
-        fragment = RuleNode(2, [RuleNode(3), RuleNode(4)])
+        fragment = @rulenode 2{3, 4}
 
-        modify_grammar_frangel!([fragment], grammar) 
+        modify_grammar_frangel!([fragment], grammar)
         # "Int = Fragment_Int" rule exists
         @test :Fragment_Int in grammar.rules