Compare and render a term without a descent that follows its depth - #105
Merged
Conversation
![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: 7856ed8 | Previous: be71b5d | Ratio |
|---|---|---|---|
benchmarks/test_benchmark_maximal_elements.py::test_benchmark_maximal_elements |
9.937766624688578 iter/sec (stddev: 0.0005041658476680435) |
9.608281351562356 iter/sec (stddev: 0.010017085420616013) |
0.97 |
benchmarks/test_benchmark_maze.py::test_benchmark_maze |
3.9918314131882298 iter/sec (stddev: 0.020639774010039528) |
3.9235166334572096 iter/sec (stddev: 0.018797551923491352) |
0.98 |
benchmarks/test_benchmark_maze_contains.py::test_benchmark_maze_contains |
3.6246320652301436 iter/sec (stddev: 0.0224579446751456) |
3.5391214334611916 iter/sec (stddev: 0.025877608823890046) |
0.98 |
benchmarks/test_benchmark_maze_loopfree.py::test_benchmark_maze_loopfree |
4.022406817175069 iter/sec (stddev: 0.016062203100244418) |
3.8477221717914887 iter/sec (stddev: 0.02072136799063595) |
0.96 |
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 #105 +/- ##
===========================================
+ Coverage 83.64% 84.03% +0.39%
===========================================
Files 55 55
Lines 4866 4986 +120
Branches 600 616 +16
===========================================
+ Hits 4070 4190 +120
Misses 699 699
Partials 97 97
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
reviewed
Aug 25, 2026
| # terms, every ``dict.fromkeys`` and the fitness cache of the evolutionary algorithms. The value | ||
| # is a compromise: small enough for the descent to fit in a call stack that is already in use, | ||
| # large enough that ordinary terms take it. | ||
| _SHALLOW_ENOUGH = 64 |
Contributor
There was a problem hiding this comment.
"shallow" means "depth" is small. the magic constant is used for "size". maybe "_SMALL" is a better name?
Comparing the children of two nodes compared two tuples, and comparing tuples compares their elements, so `Tree.__eq__` descended one level per level of the term. `Tree.__str__` descended the same way through `__rec_to_str__`. That bounded the depth a term could have at a fixed fraction of the interpreter's recursion limit: two separately built copies of a chain overflowed at a third of the limit on CPython 3.11 and at nearly all of it from 3.12 on, where the separate C recursion limit binds. Every dict and every set keyed on terms carried the same bound, because a lookup whose hash matches ends in a comparison: the fitness cache of the evolutionary algorithms is keyed on terms, and so is the deduplication of a population. Both operations now carry what they still have to visit on an explicit stack, and subterms of at most `_SMALL_ENOUGH` positions are compared and rendered as before. `size` counts positions and is `1 + sum(child.size)`, so a subterm of at most n positions is at most n levels deep, and the descent below the threshold is bounded by the threshold rather than by the term. That is a property of the definition, not of a measurement. The comparison asks the same questions in the same order as before and stops at the first position where the two terms disagree, and the rendering writes the same brackets. The descent is kept below the threshold because it is the tuple comparison, which runs in C, where an explicit stack allocates a tuple and a list slot per node in Python, and `__eq__` carries every set of terms, every `dict.fromkeys` and the fitness cache. Converting it as well cost 3.7 times as much on a dict lookup when measured. What that would buy is a smaller constant, not a wider class of terms: below the threshold this answers what the descent answered before, because below the threshold it is that descent, and no caller can compare less than it could before. One answer changes. A term is equal to itself even where the label at its root is not equal to itself, because identity is checked first. A term rooted in a NaN used to be unequal to itself while the same NaN one level down was not, since the tuple comparison settled that pair on identity before looking at it. Both answers are the reflexive one now, which is what a dict and a set gave all along. Two terms built apart around a NaN stay unequal. Costs, from one machine and one build with both trees measured in the same run: comparing is about 9 % more expensive on the shapes a search produces, a dict lookup over 200 separately built keys taking 375 against 343 microseconds and a set of 400 terms 385 against 355. Comparing a term against itself is twice as fast, and rendering is unchanged at 813 against 819 microseconds over 200 terms. The two cases that pay more are the ones that could not be computed at all before: a chain of 300 nodes compares in 114 against 47 microseconds and renders in 166 against 119. The benchmark suite moves within 1 % in both directions. Thirteen tests. Four work at twice the recursion limit, on equality, a dict lookup, rendering, and a comparison that has to reach the bottom of such a term, which is the statement about the old bound that does not depend on which interpreter runs it. Four fix what the loop has to notice by itself and a chain cannot show, because a chain has one child per node: differing arity, children compared in place, a shared subterm that settles nothing, and the label of a node the loop descends into. Four fix what a term renders as, against a reference renderer written independently of `__str__`, on terms on both sides of the threshold. One fixes that equality is reflexive whatever the labels are. The test in `tests/test_kernels.py` that records this defect with `pytest.raises(RecursionError)` is replaced by one asserting the answer the kernel now gives, which is the answer it already gives for two identical objects.
FelixLaarmann
force-pushed
the
bugfix/iterative-tree-comparison
branch
from
August 25, 2026 11:13
0524dc1 to
7856ed8
Compare
mrhaandi
approved these changes
Aug 25, 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
Tree.__eq__compared the children of two nodes as tuples, and comparing tuples compares theirelements, so a comparison descended one level per level of the term.
Tree.__str__descended thesame way through
__rec_to_str__. That bounded the depth a term could have at a fixed fraction ofthe interpreter's recursion limit: two separately built copies of a chain overflowed at a third of
the limit on CPython 3.11 and at nearly all of it from 3.12 on. Every
dictand everysetkeyedon terms carried the same bound, because a lookup whose hash matches ends in a comparison: the
fitness cache of the evolutionary algorithms is keyed on terms, and so is the deduplication of a
population.
Changes
__eq__carries the pairs still to be compared on an explicit stack, asking the same questionsin the same order and stopping at the first position where the two terms disagree.
__str__carries what it still has to visit the same way, once to descend into a node and onceto join what its children left behind. The brackets it writes are unchanged.
_SHALLOW_ENOUGHpositions keep the comparison and the renderer they had.__reduce__said that pickling is the one place in this class where depth isa limit. It now names equality and rendering among the iterative operations, which makes it true.