Make derivation, enumeration and pruning reproducible - #95
Merged
Conversation
added 5 commits
August 20, 2026 08:07
No experiment built on this framework could be repeated, not even inside one process: eight calls to sample_tree with random.Random(0) produced six different terms. The cause sits in SolutionSpace.resolution, the routine behind depth_first_resolution, breadth_first_resolution and sample_tree. It collected the goals of a derivation step in a set[Goal], and Goal defines neither __eq__ nor __hash__. The set therefore never deduplicated anything -- it only scattered the goals over their object addresses and handed them back in an allocation-dependent order, which the seeded shuffle in sample_tree then permuted. Collecting them in a list removes no capability and makes a derivation depend on the grammar alone. A list rather than value equality on Goal, for three measured reasons. Of 862246 goals observed across a maze benchmark, a symbolic regression and this test suite, not one was equal by value to an earlier one, so there is nothing to deduplicate. A set with a value-based hash would not fix the order either, only relocate it: over eight processes with different PYTHONHASHSEEDs it produced eight different results where the list produces one. And a canonical key is O(n log n) on a path that is otherwise O(n), which cost 73 to 75 percent on depth_first_resolution. Next to that, both push strategies computed sorted(new_goals, key=...) and discarded the result, so the intended ordering never took effect. Assigning it is not enough: extendleft inserts in reverse, so pushing the sorted list directly would make depth-first search expand the goal with the most subgoals first, the opposite of the intent. Depth-first now pushes reversed(ordered); breadth-first, which appends and reads from the front, does not. The initial goals were also collected by prepending, and since sorted is stable that broke ties among equally wide rules in reverse rule order at the root of a derivation while every deeper level broke them in rule order; they are appended now. Both resolutions consequently return a different sequence wherever the result is cut short. A complete enumeration of a finite grammar yields the same terms as before; under max_count the selection changes.
Reading the rules of a non-terminal went through defaultdict.__getitem__, which inserts. Merely looking at a non-terminal therefore added it to the grammar, and nonterminals() handed out the live keys() view, so the set of non-terminals could grow underneath a caller that was still iterating it. Running any search over a grammar that refers to a non-terminal without rules triggered both. Reads inside the class now go through a non-mutating helper, and nonterminals() returns a snapshot in insertion order. add_rule still writes through the defaultdict, which is where that behaviour belongs. __getitem__ is public, and for an unknown non-terminal it now raises KeyError instead of inventing an empty deque: a deque that is not stored would swallow an append without a word, and get() already covers the "maybe present" case. For a known non-terminal it keeps returning the stored deque, so appending to that still adds a rule. Because nonterminals() is a copy now, membership against it would have become linear in the size of the grammar. SolutionSpace grows a __contains__ so that "nt in space" stays a dict lookup; the four internal guards use it, and external callers have the same replacement for "nt in space.nonterminals()".
Both resolution docstrings claimed leftmost goal selection. The code takes the deepest open subgoals and, among those, the leftmost. Only the documentation changes here. Whether true leftmost selection would be the right semantics is a question about the inhabitation algorithm itself and is not decided in this commit.
enumerate_trees was irreproducible independently of the derivation order: over thirty fresh processes, enumerate_trees(start, max_count=20) returned thirty different orders and twenty-eight different sets of terms. Pinning PYTHONHASHSEED did not help, because CoSy combinators are usually plain function objects, so the hash of a Tree over them falls back to the address of one particular run. Three collections were responsible, all plain sets whose iteration order is observable: the terms a rule generates, the terms already known per non-terminal, and the working set of non-terminals the loop draws from. They are backed by a dict now, whose keys are a set that keeps the order in which they were added. A total order on Tree would have been the alternative; it is not needed for this and a naive implementation was 6.3 times slower. Under max_count this changes which terms come out, not only the order they come out in -- from whatever the allocator produced to the same ones in every process. A complete enumeration returns the same terms as before. _generate_new_trees returns that ordered set instead of a plain set, so the two interrupt-and-resume tests accumulate with update() to keep their local sets sets. This commit stands on its own: nothing before it depends on it.
prune() drove its walk over the ground types with plain sets, so the order in which it discovered them -- and with it the key order of the grammar it returns and the order of that grammar's nonterminals() -- depended on PYTHONHASHSEED. On a two-non-terminal grammar it reported ['C', 'S'] under seeds 0 and 1 and ['S', 'C'] under 42, 7 and 123. The three collections are dicts now, whose keys are a set that keeps the order in which they were added, and the queue is drained oldest first so that the traversal follows discovery order. What prune() returns no longer depends on the interpreter that ran it. This commit stands on its own: nothing before it depends on 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: 686a815 | Previous: be71b5d | Ratio |
|---|---|---|---|
benchmarks/test_benchmark_maximal_elements.py::test_benchmark_maximal_elements |
9.684730395303463 iter/sec (stddev: 0.0018974316285660538) |
9.608281351562356 iter/sec (stddev: 0.010017085420616013) |
0.99 |
benchmarks/test_benchmark_maze.py::test_benchmark_maze |
3.84008059327672 iter/sec (stddev: 0.015859703370300314) |
3.9235166334572096 iter/sec (stddev: 0.018797551923491352) |
1.02 |
benchmarks/test_benchmark_maze_contains.py::test_benchmark_maze_contains |
3.586262333539007 iter/sec (stddev: 0.018942522756175008) |
3.5391214334611916 iter/sec (stddev: 0.025877608823890046) |
0.99 |
benchmarks/test_benchmark_maze_loopfree.py::test_benchmark_maze_loopfree |
3.8333355642068483 iter/sec (stddev: 0.015397253477173386) |
3.8477221717914887 iter/sec (stddev: 0.02072136799063595) |
1.00 |
This comment was automatically generated by workflow using github-action-benchmark.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #95 +/- ##
===========================================
+ Coverage 73.02% 74.22% +1.19%
===========================================
Files 39 41 +2
Lines 2944 3135 +191
Branches 494 500 +6
===========================================
+ Hits 2150 2327 +177
- Misses 686 703 +17
+ Partials 108 105 -3
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:
|
mrhaandi
approved these changes
Aug 20, 2026
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
3 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:
Nothing this framework produces could be reproduced, not even inside a single process. A dozen
lines are enough to show it on
develop:Eight draws, one seed, one process. On
developthis printed 6, 7, 8, 8 and 7 over fiverepetitions; on this branch it prints 1 every time.
The place to look is
SolutionSpace.resolution, notenumerate_trees.enumerate_treesandprunewere irreproducible for a second, independent reason, and are fixedhere too.
Cause:
Four independent things, all of them an unordered container whose iteration order is observable.
The goals of a derivation step.
resolutioncollected them in aset[Goal], andGoaldefines neither
__eq__nor__hash__. The set therefore never deduplicated anything -- it onlyscattered the goals over their object addresses and handed them back in allocation order.
sample_treethen shuffled that order with the seeded generator, so the seed permuted a list thatwas already random.
The sort that was thrown away. Both push strategies called
sorted(new_goals, key=lambda g: len(g.subgoals))and discarded the result. The intended orderingnever took effect, which is why the derivation order was left entirely to the set above.
Reading created what it read. Rule lookups went through
defaultdict.__getitem__, whichinserts. Merely looking at a non-terminal added it to the grammar, and
nonterminals()handed outthe live
keys()view, so the set of non-terminals could grow underneath a caller that wasiterating it.
Three sets in
enumerate_treesand three inprune. Inenumerate_trees: the terms a rulegenerates, the terms already known per non-terminal, and the working set of non-terminals the loop
draws from. In
prune: the ground types, the queue, and the inverse grammar. PinningPYTHONHASHSEEDdoes not repair the first group, because CoSy combinators are usually plainfunction objects, so
Tree._hashfalls back to the address of one particular run.What changes:
resolutionderives its goals into a list, and the sort takes effect. A list removes nocapability, because the set never removed a duplicate (measured below). Assigning the sorted
list is not enough on its own:
extendleftinserts in reverse, so pushing it directly wouldmake depth-first search expand the goal with the most subgoals first, the opposite of the
intent. Depth-first now pushes
reversed(ordered); breadth-first, which appends and reads fromthe front, does not. That asymmetry is in the code as a comment rather than left for the next
reader. The initial goals were also collected by prepending, and since
sortedis stable thatbroke ties among equally wide rules in reverse rule order at the root of a derivation while
every deeper level broke them in rule order; they are appended now, so ties resolve the same way
everywhere.
non-mutating helper,
nonterminals()returns a snapshot, andadd_rulestill writes through thedefaultdict, which is where that behavior belongs.
__getitem__raisesKeyErrorfor anunknown non-terminal instead of inventing an empty deque.
SolutionSpacegains a__contains__so that membership stays a dict lookup.
the code takes the deepest open subgoals and, among those, the leftmost. Documentation only. This
deliberately leaves open whether true leftmost selection would be the right semantics -- that is
a question about the inhabitation algorithm and is not decided here.
enumerate_treesbecomes reproducible. The three sets are backed by adict, whose keys area set that keeps the order in which they were added. No new dependency, and no total order on
Tree: that is the obvious alternative, it is not needed for reproducibility, and a naive one(size, then root name, then children) made the enumeration measured below 15 times slower --
2980 ms against 195 ms -- because the priority queue compares whole terms on every insertion.
prunereturns a grammar in a stable order. Same treatment for its three collections, andthe queue is drained oldest-first, so the traversal follows the discovery order.