Stop emitting rules a predicate already forbids - #97
Closed
FelixLaarmann wants to merge 1 commit into
Closed
FelixLaarmann wants to merge 1 commit into
FelixLaarmann wants to merge 1 commit into
Conversation
`Synthesizer.construct_solution_space_rules` built rules that can never be applied. A combinator without a term parameter produces rules whose only named arguments are constants: the non-terminal arguments it takes come from arrows in its suffix and carry no name. The substitution such a rule's predicates are applied to therefore consists of the literal values alone -- it is exactly the rule's `literal_substitution`, and it is fully known while the rule is being built. A predicate that is false on it forbids the rule for good, yet the rule was constructed and added to the solution space anyway. In a space over ten literal values with the predicate `d > 7`, eight of the ten rules for `Digit` were of this kind. Do not construct them. The check runs once per enumerated literal assignment, before any subquery is computed for it, and only where the answer is already fixed. A combinator with a term parameter keeps all of its rules: its predicates speak about terms that do not exist at this point. Three things followed from keeping those rules. `prune` considered the affected non-terminals productive. It reasons purely structurally, over `RHSRule.non_terminals`; predicates take no part in it. A rule without non-terminal arguments is a base case for `prune`, whether or not its predicate admits it. Depth-first resolution failed to terminate where it could have. Take `Digit -> zero` with a predicate that rejects the only literal value, plus `Digit -> wrap(Digit)`: `Digit` has no term at all, but the search descends into `wrap(wrap(...))` without end. Once the rejected rule is not created, `Digit` is unproductive, `prune` removes it, and the search stops immediately. Every count taken over the rules counted terms that do not exist. The rules that disappear were never derivable, so `enumerate_trees` and `contains_tree` answer as before. The resolution answers differently in one shape: for a rule whose non-terminal arguments are all anonymous it used to ignore the predicates entirely and could return a forbidden term. It now agrees with `enumerate_trees`.
Contributor
There was a problem hiding this comment.
Benchmark CoSy
Details
| Benchmark suite | Current: 852fd0f | Previous: be71b5d | Ratio |
|---|---|---|---|
benchmarks/test_benchmark_maximal_elements.py::test_benchmark_maximal_elements |
9.232243867255239 iter/sec (stddev: 0.0022244478848171415) |
9.608281351562356 iter/sec (stddev: 0.010017085420616013) |
1.04 |
benchmarks/test_benchmark_maze.py::test_benchmark_maze |
3.9591295007486313 iter/sec (stddev: 0.012015487926278896) |
3.9235166334572096 iter/sec (stddev: 0.018797551923491352) |
0.99 |
benchmarks/test_benchmark_maze_contains.py::test_benchmark_maze_contains |
3.6654905439301566 iter/sec (stddev: 0.014693018045307234) |
3.5391214334611916 iter/sec (stddev: 0.025877608823890046) |
0.97 |
benchmarks/test_benchmark_maze_loopfree.py::test_benchmark_maze_loopfree |
3.802635910400676 iter/sec (stddev: 0.014009371245450463) |
3.8477221717914887 iter/sec (stddev: 0.02072136799063595) |
1.01 |
This comment was automatically generated by workflow using github-action-benchmark.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #97 +/- ##
===========================================
+ Coverage 73.02% 74.96% +1.93%
===========================================
Files 39 41 +2
Lines 2944 3151 +207
Branches 494 500 +6
===========================================
+ Hits 2150 2362 +212
+ Misses 686 677 -9
- Partials 108 112 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem:
Synthesizer.construct_solution_space_rulesbuilds rules that can never be applied, and puts theminto the
SolutionSpaceanyway. Ten literal values and one predicate are enough to show it ondevelop:Eight of the ten rules are unsatisfiable:
dis fixed to a rejected value, and no later choice canchange that.
enumerate_treesknows it -- it applies the predicate and returns two terms -- but theeight rules stay in the space, and everything that reads the space rather than the terms is misled
by them.
prunetreats such non-terminals as productive. It computes productivity purely structurally,over
RHSRule.non_terminals; predicates take no part in the algorithm at all. A rule with nonon-terminal argument is a base case for
prune, whether or not its predicate admits it. Add aconsumer and the lie propagates:
Digithas no term.prunekeeps it, and keepsStartwith it.Depth-first resolution fails to terminate where it could have. Replace the consumer by a
recursive rule --
Digit -> zerowhose predicate rejects the only literal value, plusDigit -> wrap(Digit).Digitstill has no term, andprunestill keeps it, so the pruned spaceis no help.
developanswerswrap (digit 0), a term its own repository forbids andcontains_treerejects. With the predicate check inGoal(the parallel pull request, see below)the wrong answer is gone but the search descends into
wrap(wrap(...))without end. If therejected rule is never created,
Digitis unproductive andprune()removes it, so a search overthe pruned space returns the empty result immediately.
How much of that reaches a caller who does not prune depends on whether the non-terminal keeps any
rule at all. If every one of its rules is rejected, it does not appear in the generated space to
begin with, and the search ends at once without pruning -- that is the case in the example above,
where
space.nonterminals()no longer listsDigit. If it keeps a recursive rule, as here, itstays in the space and an unpruned depth-first search still descends forever. That is correct for a
semi-decidable search on a query with no answer, and
construct_solution_spacedoes not prune onits own --
prune()is the caller's decision, and this change is what finally makes it an informedone.
Every count taken over the rules counts terms that do not exist. Rule counts, non-terminal
counts and anything derived from them are off by whatever the predicates forbid -- eight rules out
of ten in the example above.
Cause:
The rules are constructed without ever consulting the predicates.
construct_solution_space_rulescarries
specification_info.term_predicatesthrough to theRHSRuleit yields and hands thedecision to whoever reads the space later. For most rules that is the only thing it can do. For one
class of rules it is not, and that class is the one above.
The description of rules in
SolutionSpacefixes what a predicate can see:The synthesizer names exactly those non-terminal arguments that a component declares with
argument(...), that is, theTermParameters in its prefix. The non-terminal arguments it derivesfrom arrows inside the suffix stay unnamed. A combinator without a term parameter therefore
produces rules whose named part consists of constants alone, and the substitution its predicates
are applied to is exactly
RHSRule.literal_substitution-- fully determined by the instantiation,before a single argument type has been looked at.
enumerate_treesconfirms it from the otherside: its
specific_substitutionis the named non-terminal arguments unioned withrule.literal_substitution, which collapses to the literal substitution when there are no namedarguments.
What changes:
One guard in
construct_solution_space_rules, once per enumerated literal assignment, before anysubquery is computed for it:
No term parameter means no named non-terminal argument, which means the predicate substitution is
the literal substitution and nothing later can add to it. If a predicate is false on it, no rule is
constructed for this instantiation -- for any arity of the combinator type and any minimal cover.