Remove SolutionSpace.sample_tree, which the samplers replaced - #109
Merged
Conversation
sample_tree drew one tree top-down under an optional depth bound and returned None where nothing came. The samplers of cosy.search draw the same way through a resolution query, and since the genetic operators moved to them, no caller of sample_tree is left in the framework. What remains is a second way to draw from a solution space that nothing asks for. The three tests that drew through it draw through DepthBoundedRandomSampler over a generator query or a residual query instead, and their names follow. test_a_sample_reaches_more_than_the_nullary_clause_of_the_start_symbol goes with no replacement. It guarded a defect of the frontier sample_tree drew from, and the property it asserted is asserted more sharply in tests/test_samplers.py, where test_every_completion_within_the_bound_is_drawn_by_some_seed requires set equality with the reachable terms on a space whose start symbol has a nullary clause. The skeleton test gains the assertion its sibling already carries, that the seeds reach more than one term. Without it the subset direction stays true where the draw stops varying. The seeded terms of test_a_seeded_sample_repeats differ from the ones sample_tree produced. The sampler draws its randomness from the clause order rather than from the goals of a step, and the property the test needs from the engine is the same one either way. sample_tree was the only code under src/cosy/core that used the random module, so the S311 exemption for that path goes with it.
![tudo-seal-workflows[bot]](https://avatars.githubusercontent.com/in/2312552?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Benchmark CoSy
Details
| Benchmark suite | Current: 44e36bd | Previous: be71b5d | Ratio |
|---|---|---|---|
benchmarks/test_benchmark_maximal_elements.py::test_benchmark_maximal_elements |
10.273612373811034 iter/sec (stddev: 0.0004668522031856945) |
9.608281351562356 iter/sec (stddev: 0.010017085420616013) |
0.94 |
benchmarks/test_benchmark_maze.py::test_benchmark_maze |
4.352696216559452 iter/sec (stddev: 0.0236595215939288) |
3.9235166334572096 iter/sec (stddev: 0.018797551923491352) |
0.90 |
benchmarks/test_benchmark_maze_contains.py::test_benchmark_maze_contains |
3.9859519905236427 iter/sec (stddev: 0.024390729940967856) |
3.5391214334611916 iter/sec (stddev: 0.025877608823890046) |
0.89 |
benchmarks/test_benchmark_maze_loopfree.py::test_benchmark_maze_loopfree |
4.363107244067764 iter/sec (stddev: 0.01772350745971044) |
3.8477221717914887 iter/sec (stddev: 0.02072136799063595) |
0.88 |
This comment was automatically generated by workflow using github-action-benchmark.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #109 +/- ##
===========================================
- Coverage 88.77% 88.76% -0.01%
===========================================
Files 69 69
Lines 7456 7435 -21
Branches 829 829
===========================================
- Hits 6619 6600 -19
+ Misses 741 739 -2
Partials 96 96
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
2 participants
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:
SolutionSpace.sample_treeis the predecessor of the samplers. It draws one tree top-down under anoptional depth bound and returns None where nothing came. The samplers of
cosy.searchdraw thesame way through a resolution query, and since the genetic operators moved to them in #108, no
caller of
sample_treeis left in the framework. What remains is a second way to draw from asolution space that nothing asks for.
Changes:
sample_treeis removed with no replacement. A caller poses a resolution query and draws througha sampler of
cosy.search. (BREAKING)DepthBoundedRandomSamplerover a generatorquery or a residual query instead, and their names follow.
test_a_sample_reaches_more_than_the_nullary_clause_of_the_start_symbolis dropped. It guarded adefect of the frontier
sample_treedrew from, andtest_every_completion_within_the_bound_is_drawn_by_some_seedasserts the same property moresharply, as set equality with the reachable terms on a space whose start symbol has a nullary
clause.
one term. Without it the subset direction stays true where the draw stops varying.
sample_treewas the only code undersrc/cosy/corethat used therandommodule, so theS311exemption for that path goes with it.
test_a_seeded_sample_repeatsdiffer from the onessample_treeproduced. The sampler draws its randomness from the clause order rather than from the goals of a
step, and the property the test needs from the engine is the same one either way.