diff --git a/src/cosy/core/tree.py b/src/cosy/core/tree.py index 99eb40a..5b65d5e 100644 --- a/src/cosy/core/tree.py +++ b/src/cosy/core/tree.py @@ -9,7 +9,7 @@ # Uniqueness is guaranteed by python's set (instead of list) data structure. from collections import deque from collections.abc import Callable, Hashable, Sequence -from functools import partial +from functools import lru_cache, partial from inspect import Parameter, _empty, _ParameterKind, signature from typing import Any, Generic, TypeVar @@ -18,6 +18,75 @@ Path = tuple[int, ...] +# ``inspect.signature`` is called in ``interpret`` once per occurrence of a combinator, at about +# 5.4 microseconds per call. Memoization reduces this to once per combinator. +# +# What the bound must hold is the number of combinators evaluated together, which is at most the +# size of a component repository: 3 to 7 in the examples here, 4 in the benchmarks, and 24 to 49 +# in the largest practical scenarios. An LRU that no longer holds that set falls to a zero hit +# rate at once rather than declining, because every lookup evicts the entry the next one asks for. +# Measured over 200 combinators, ``maxsize=128`` runs about ninety times slower than +# ``maxsize=1024``. +# +# The cache key is the combinator object itself, and a function or a lambda hashes by identity. An +# algebra rebuilt for every evaluation therefore presents new keys for the same signatures, and no +# lookup hits across calls. The bound is what keeps such a caller from filling the cache with +# callables it will never ask for again, together with whatever those callables close over. +# Measured on that pattern, an unbounded memo grew to 200000 entries over 50000 evaluations and +# became slower than the bounded one, since its table keeps being rebuilt. 1024 sits well above +# the largest working set named above and well below the size at which retention costs anything. +# See ``test_a_fresh_algebra_per_evaluation_does_not_grow_the_memo``. +# +# The cache holds static metadata about a combinator. Every combinator is still called on every +# evaluation, so an interpretation may have side effects and may answer differently each time. See +# ``tests/test_interpretation_semantics.py``. +@lru_cache(maxsize=1024) +def _parameters_cached(combinator: Callable[..., Any]) -> tuple[Parameter, ...]: + """Return the parameters of a callable, from a bounded memo. + + Args: + combinator (Callable[..., Any]): The callable to inspect. + + Returns: + tuple[Parameter, ...]: Its parameters, in declaration order. A tuple rather than a list, + because the memo hands out the object it stored. + + Raises: + ValueError: If ``combinator`` exposes no signature. ``interpret`` turns this into a + ``TypeError`` naming the combinator. + TypeError: If ``combinator`` cannot be a memo key, raised by ``lru_cache`` before this body + runs, or if ``signature`` cannot inspect it. ``_parameters_of`` tells the two apart. + """ + return tuple(signature(combinator).parameters.values()) + + +def _parameters_of(combinator: Callable[..., Any]) -> tuple[Parameter, ...]: + """Return the parameters of a callable, through the memo wherever that is possible. + + Args: + combinator (Callable[..., Any]): The callable to inspect. + + Returns: + tuple[Parameter, ...]: Its parameters, in declaration order. + + Raises: + ValueError: If ``combinator`` exposes no signature, see ``_parameters_cached``. + TypeError: If ``signature`` cannot inspect ``combinator``, for instance because it carries + a ``__signature__`` that is not a signature. The other ``TypeError``, the one an + unhashable combinator raises on the memo key, is handled below rather than reported. + """ + try: + return _parameters_cached(combinator) + except TypeError: + # Two failures arrive as ``TypeError``, and retrying without the memo tells them apart. An + # unhashable combinator fails on the cache key before the body runs, the common case being + # a value object that defines ``__eq__`` and so has no ``__hash__``. A combinator that + # ``signature`` itself rejects raises again here and reaches the caller, as it did before + # the memo existed. Checking hashability up front would hash every combinator a second + # time on the hot path, for a case that does not occur in practice. + return tuple(signature(combinator).parameters.values()) + + class Tree(Generic[T]): """Please only use immutably.""" @@ -27,12 +96,6 @@ class Tree(Generic[T]): _hash: int _positions: frozenset[Path] | None = None _leaf_positions: frozenset[Path] | None = None - # tuple[interpretation id, reference to interpretation dict (avoid GC, see test), cached interpretation result] - # Breaks for non-deterministic interpretations. The entry belongs to the node, and a node is - # shared by every term built around it, so an interpretation dict that is changed in place -- - # same object, same id, different contents -- now reports its stale value through every one of - # those terms rather than only through the one the node was first evaluated in. - _interpreted: tuple[int, dict[T, Any] | None, Any] | None = None def __init__(self, root: T, children: Sequence["Tree[T]"] = ()) -> None: """_summary_. @@ -116,42 +179,32 @@ def __copy__(self) -> "Tree[T]": """ return Tree(root=self.root, children=self.children) - # Writing is recursive -- three ``save()`` levels per node here, four with the default - # protocol -- and on Python 3.10 and 3.11 that recursion is bounded by the interpreter's - # recursion limit: measured from inside a pytest run, a chain of 318 nodes is the deepest term - # that can be written, against 238 before. From 3.12 on the separate C recursion limit binds - # instead, at about 3325. Terms do grow deeper than that -- which is why ``subtree_at``, - # ``_walk`` and ``interpret`` are iterative -- so pickling is the one place in this class where - # depth is still a limit; the reduction below at least raises the ceiling by a third. + # Writing is recursive, three ``save()`` levels per node here and four with the default + # protocol. On Python 3.10 and 3.11 that recursion is bounded by the interpreter's recursion + # limit: measured from inside a pytest run, a chain of 318 nodes is the deepest term that can + # be written, against 238 before. From 3.12 on the separate C recursion limit binds instead, + # at about 3325. Terms do grow deeper than that, which is why ``subtree_at``, ``_walk`` and + # ``interpret`` are iterative, so pickling is the one place in this class where depth is still + # a limit. The reduction below at least raises the ceiling by a third. def __reduce__(self) -> tuple[type["Tree[T]"], tuple[T, tuple["Tree[T]", ...]]]: """Reconstruct through the constructor instead of through the instance dictionary. - Everything ``__init__`` computes -- ``size`` and ``_hash`` -- and everything filled on - demand -- the two position sets and the interpretation result -- follows from ``root`` and - ``children``, so none of it has to be written. The default protocol writes the instance - dictionary, and therefore writes all of it, together with the ``__orig_class__`` that - ``Tree[str](...)`` leaves on an instance. ``__copy__`` and ``replace_subtree_at`` already - build their results out of ``root`` and ``children`` alone; this makes the third way of - producing a node agree with them. + Everything ``__init__`` computes (``size`` and ``_hash``) and everything filled on demand + (the two position sets) follows from ``root`` and ``children``, so none of it has to be + written. The default protocol writes the instance dictionary, and therefore writes all of + it, together with the ``__orig_class__`` that ``Tree[str](...)`` leaves on an instance. + ``__copy__`` and ``replace_subtree_at`` already build their results out of ``root`` and + ``children`` alone. This makes the third way of producing a node agree with them. ``_hash`` is what makes this more than a question of size. It is ``hash((root, children))``, and hashing a string is randomized per process, so a transported ``_hash`` is the writing process's answer to a question the reading process would answer differently. A term read back from a file then compares *equal* to the same term built here and hashes *differently*: a ``set`` keeps both of them, and a ``dict`` - deduplicates neither -- measured by writing under ``PYTHONHASHSEED=1`` and reading under + deduplicates neither, measured by writing under ``PYTHONHASHSEED=1`` and reading under ``PYTHONHASHSEED=2``. Recomputing on load is what makes a term that arrived by stream interchangeable with one built in place. - The interpretation cache is worse than wasteful. Its entry is - ``(id(interpretation), interpretation, result)``, and it holds the second field precisely - so that the address in the first stays taken and the key stays meaningful. Loading - rebuilds the interpretation at a different address, so the key names an object that no - longer exists anywhere; should the interpreter hand that address to something else, a - later ``interpret`` is answered with the result of a foreign interpretation. Carrying the - entry also drags every callable of the interpretation into the stream, which makes a - single node unpicklable as soon as one of them is a lambda. - The position sets are the plain case: a second encoding of a structure the stream already carries. A term of 2047 nodes with them filled at the root writes 19486 bytes this way instead of 116805, and writing it costs 0.67 milliseconds instead of 1.32 (CPython 3.13, @@ -160,7 +213,7 @@ def __reduce__(self) -> tuple[type["Tree[T]"], tuple[T, tuple["Tree[T]", ...]]]: unchanged. ``copy.deepcopy`` reduces through here as well, so a deep copy now starts with cold caches - too; ``copy.copy`` continues to go through ``__copy__``. The class is taken from + too. ``copy.copy`` continues to go through ``__copy__``. The class is taken from ``self.__class__`` rather than named outright, because loading must not change what an object is. @@ -177,11 +230,12 @@ def __setstate__(self, state: dict[str, Any]) -> None: Only a stream written before ``__reduce__`` existed reaches this method, since a reduction that hands back constructor arguments produces no state at all. Such a stream carries the derived fields, and adopting them is what the reduction above avoids producing: the - ``_hash`` in it was computed under the writing process's hash seed, and the interpretation - cache in it is keyed on an address in a process that has ended. Taking ``root`` and - ``children`` and computing the rest here makes that fix reach terms that were written - before it existed -- which matters because the terms anyone keeps on disk are the - expensive ones -- and leaves the caches cold, where they belong. + ``_hash`` in it was computed under the writing process's hash seed. A stream old enough + carries an interpretation result as well, which no longer has a field to be read into and + is dropped here with the rest. Taking ``root`` and ``children`` and computing the rest + here makes that fix reach terms that were written before it existed, which matters because + the terms anyone keeps on disk are the expensive ones, and leaves the position sets cold, + where they belong. Args: state (dict[str, Any]): The instance dictionary of the node as it was written. @@ -205,11 +259,6 @@ def interpret(self, interpretation: dict[T, Any] | None = None) -> Any: TypeError: _description_ """ - # if interpretation hasn't changed, skip interpreting, use cache - evaluated = self._interpreted - if evaluated is not None and evaluated[0] == id(interpretation): - return evaluated[2] - terms: deque[Tree[T]] = deque((self,)) combinators: deque[tuple[T, int]] = deque() # decompose terms @@ -222,14 +271,14 @@ def interpret(self, interpretation: dict[T, Any] | None = None) -> Any: # apply/call decomposed terms while combinators: (c, n) = combinators.pop() - parameters_of_c: Sequence[Parameter] = [] current_combinator: partial[Any] | T | Callable[..., Any] = ( c if interpretation is None or c not in interpretation else interpretation[c] ) + parameters_of_current_combinator: Sequence[Parameter] = [] if callable(current_combinator): try: - parameters_of_c = list(signature(current_combinator).parameters.values()) + parameters_of_current_combinator = _parameters_of(current_combinator) except ValueError as exc: msg = ( f"Interpretation of combinator {c} does not expose a signature. " @@ -237,7 +286,7 @@ def interpret(self, interpretation: dict[T, Any] | None = None) -> Any: ) raise TypeError(msg) from exc - if n == 0 and len(parameters_of_c) == 0: + if n == 0 and len(parameters_of_current_combinator) == 0: current_combinator = current_combinator() arguments = deque(results.pop() for _ in range(n)) @@ -252,11 +301,11 @@ def interpret(self, interpretation: dict[T, Any] | None = None) -> Any: use_partial = False - simple_arity = len(list(filter(lambda x: x.default == _empty, parameters_of_c))) - default_arity = len(list(filter(lambda x: x.default != _empty, parameters_of_c))) + simple_arity = len(list(filter(lambda x: x.default == _empty, parameters_of_current_combinator))) + default_arity = len(list(filter(lambda x: x.default != _empty, parameters_of_current_combinator))) # if any parameter is marked as var_args, we need to use all available arguments - pop_all = any(x.kind == _ParameterKind.VAR_POSITIONAL for x in parameters_of_c) + pop_all = any(x.kind == _ParameterKind.VAR_POSITIONAL for x in parameters_of_current_combinator) # If a var_args parameter is found, we need to subtract it from the normal parameters. # Note: python does only allow one parameter in the form of *arg @@ -293,16 +342,12 @@ def interpret(self, interpretation: dict[T, Any] | None = None) -> Any: current_combinator = current_combinator(*fixed_parameters, *var_parameters, *default_parameters) results.append(current_combinator) - result = results.pop() - - # no cache hit (first seen or interpretation changed), overwrite cache - self._interpreted = (id(interpretation), interpretation, result) - return result + return results.pop() def _walk(self) -> tuple[frozenset[Path], frozenset[Path]]: """Fill both position caches in one traversal. - The leaves are read off the walk -- a node without children is a leaf -- rather than + The leaves are read off the walk, a node without children being a leaf, rather than filtered out of the position set afterwards. Filtering compares every position against every other, which is quadratic: a term of 32767 nodes took 84 seconds, and resolving a term at a position asks for the leaves of that term on every call. @@ -331,8 +376,8 @@ def positions(self) -> frozenset[Path]: The cached set is handed out as it is, frozen rather than copied. It belongs to the node, and a node is shared by every term built around it, so a caller who mutated what it got back would change what every one of those terms reports. Freezing makes that impossible - instead of merely inadvisable, and it costs nothing per call -- copying the set would be - linear in the size of the term on every read. + instead of merely inadvisable, and it costs nothing per call, where copying the set would + be linear in the size of the term on every read. Returns: frozenset[Path]: The positions of every node. @@ -362,7 +407,7 @@ def subtree_at(self, pos: Path) -> "Tree[T]": The node itself, not a copy of it. The class is immutable, so sharing is safe, and it is what the rest of the class assumes: ``replace_subtree_at`` shares every node off the path it rebuilds. Copying on the way back up made reading a position cost a copy of - everything below it -- reading every position of a 2047-node term took 94206 copies. + everything below it. Reading every position of a 2047-node term took 94206 copies. Iterative rather than recursive: terms grow to hundreds of nodes, and a chain that deep overflows a recursive descent. @@ -388,14 +433,14 @@ def replace_subtree_at(self, pos: Path, tree: "Tree[T]") -> "Tree[T]": """Return a copy of this tree with the subtree at the given position replaced. Neither this tree nor the replacement is modified. The result shares every node that is not - on the path from the root to ``pos`` with ``self``, and shares ``tree`` itself; only the + on the path from the root to ``pos`` with ``self``, and shares ``tree`` itself. Only the nodes along that path are rebuilt. Rebuilding through ``__init__`` rather than mutating in place is what keeps ``size`` and ``_hash`` correct. Both are computed once at construction, so an in-place replacement left - every ancestor of the replacement point reporting the values it had *before* the change -- - which broke the equality/hash contract of the class and, through it, every cache and every - ``set`` keyed on trees. + every ancestor of the replacement point reporting the values it had *before* the change. + That broke the equality and hash contract of the class and, through it, every cache and + every ``set`` keyed on trees. Args: pos (Path): Position of the subtree to replace, as a tuple of child indices. @@ -420,7 +465,7 @@ def replace_subtree_at(self, pos: Path, tree: "Tree[T]") -> "Tree[T]": current = current.children[index] path_nodes.append(current) - # rebuild bottom-up; everything off the path is shared rather than copied + # rebuild bottom-up, everything off the path is shared rather than copied replacement = tree for depth in range(len(pos) - 1, -1, -1): parent = path_nodes[depth] diff --git a/tests/test_interpretation_semantics.py b/tests/test_interpretation_semantics.py new file mode 100644 index 0000000..bf47b75 --- /dev/null +++ b/tests/test_interpretation_semantics.py @@ -0,0 +1,234 @@ +"""What ``interpret`` promises a combinator: that it is called, every time. + +An interpretation may have side effects and may answer differently on every call. The evaluation +of a term is therefore not something a term can remember: a memo over results turns the second +evaluation of a term into a replay of the first, which is a different function, not a faster one. + +These tests state that promise so that it survives the next attempt to cache. A node used to keep +``(id(interpretation), interpretation, result)`` and answer from it, which broke every case below. +The memo that replaced it holds the *parameters* of a combinator. Those follow from the callable +and not from its arguments, so it leaves all of them intact. +""" + +import math +import pickle +import random +from typing import Any + +import pytest + +from cosy.core.tree import Tree + + +def test_a_combinator_with_a_side_effect_runs_on_every_evaluation() -> None: + """Two evaluations of one term are two evaluations, not one and a replay. + + This is the case a result memo cannot serve: the effect *is* the point of the call, and a + cache that skips the call skips the effect. + """ + log: list[str] = [] + + def record() -> str: + """Append to the log and report what was appended. + + Returns: + str: The entry just written. + """ + log.append(f"call {len(log)}") + return log[-1] + + term = Tree(record, ()) + + first = term.interpret(None) + second = term.interpret(None) + + assert log == ["call 0", "call 1"] + assert (first, second) == ("call 0", "call 1") + + +def test_a_non_deterministic_interpretation_answers_afresh_every_time() -> None: + """An interpretation that draws is asked again, not remembered. + + Fitness in an evolutionary run is an interpretation, and a fitness that averages a noisy + measurement has to be able to disagree with itself. + """ + rng = random.Random(20260821) + + def draw() -> float: + """Draw the next number. + + Returns: + float: The draw. + """ + return rng.random() + + term = Tree(draw, ()) + + assert term.interpret(None) != term.interpret(None) + + +def test_a_non_deterministic_value_below_the_root_is_drawn_afresh() -> None: + """The promise holds inside the term, not only at the node that was asked. + + A memo at the root alone would still let the term as a whole answer twice with one draw. + """ + rng = random.Random(20260821) + + def draw() -> float: + """Draw the next number. + + Returns: + float: The draw. + """ + return rng.random() + + def keep(value: float) -> float: + """Pass a value through unchanged. + + Args: + value (float): The interpreted child. + + Returns: + float: The same value. + """ + return value + + term: Tree[Any] = Tree(keep, (Tree(draw, ()),)) + + assert term.interpret(None) != term.interpret(None) + + +def test_a_shared_node_answers_afresh_in_every_term_that_holds_it() -> None: + """Sharing must not turn one evaluation into an answer for terms built elsewhere. + + Immutability lets one node object sit in many terms, so an entry stored *on a node* is read by + every term around it. The same subterm in two individuals of a population would report the + evaluation of whichever was measured first. + """ + log: list[str] = [] + + def record() -> int: + """Count the call. + + Returns: + int: The number of calls so far. + """ + log.append("x") + return len(log) + + def left(value: int) -> str: + """Wrap a value on the left. + + Args: + value (int): The interpreted child. + + Returns: + str: The rendering. + """ + return f"L{value}" + + def right(value: int) -> str: + """Wrap a value on the right. + + Args: + value (int): The interpreted child. + + Returns: + str: The rendering. + """ + return f"R{value}" + + shared = Tree(record, ()) + + assert Tree[Any](left, (shared,)).interpret(None) == "L1" + assert Tree[Any](right, (shared,)).interpret(None) == "R2" + assert len(log) == 2 + + +def test_a_combinator_runs_once_per_occurrence() -> None: + """A term of n nodes over one combinator is n calls. + + The parameter memo asks for a combinator's signature once. It must not also make the + combinator itself be applied once. + """ + depth = 200 + log: list[int] = [] + + def leaf() -> int: + """Start the count. + + Returns: + int: Zero. + """ + return 0 + + def step(value: int) -> int: + """Count one application. + + Args: + value (int): The interpreted child. + + Returns: + int: One more than the child. + """ + log.append(value) + return value + 1 + + term: Tree[Any] = Tree(leaf, ()) + for _ in range(depth): + term = Tree(step, (term,)) + + assert term.interpret(None) == depth + assert len(log) == depth + + +def test_an_interpretation_changed_in_place_takes_effect_immediately() -> None: + """The same dictionary with different contents is a different interpretation. + + Keying a result on ``id(interpretation)`` reported the old value here, silently: the address + is unchanged, so the entry looked valid while the meaning of every symbol in it had moved. + """ + term = Tree("f", (Tree("x", ()),)) + interpretation: dict[str, Any] = {"f": lambda value: value * 2, "x": lambda: 3} + + assert term.interpret(interpretation) == 6 + + interpretation["f"] = lambda value: value * 100 + + assert term.interpret(interpretation) == 300 + + +def test_two_interpretations_of_one_term_do_not_shadow_each_other() -> None: + """A term evaluated under two algebras answers under each of them.""" + term = Tree("f", (Tree("x", ()),)) + doubling: dict[str, Any] = {"f": lambda value: value * 2, "x": lambda: 3} + squaring: dict[str, Any] = {"f": lambda value: value * value, "x": lambda: 3} + + assert [term.interpret(doubling), term.interpret(squaring)] == [6, 9] + assert [term.interpret(doubling), term.interpret(squaring)] == [6, 9] + + +def test_an_evaluated_term_still_pickles_and_carries_no_result() -> None: + """Evaluating a term must not make it unpicklable, whatever the algebra was built from. + + Terms are pickled to move a population between processes, and an algebra assembled from + lambdas is the ordinary case, so a node that kept its interpretation could not travel. + """ + term = Tree("f", (Tree("x", ()),)) + interpretation: dict[str, Any] = {"f": lambda value: value * 2, "x": lambda: 3} + + assert term.interpret(interpretation) == 6 + restored = pickle.loads(pickle.dumps(term)) + + assert restored == term + assert restored.interpret(interpretation) == 6 + + +def test_a_combinator_that_cannot_be_interpreted_is_reported_every_time() -> None: + """A failure is reported on every evaluation, never remembered and never turned into a value.""" + term = Tree("b", (Tree("leaf", ()),)) + interpretation: dict[str, Any] = {"b": math.log, "leaf": lambda: 1.0} + + for _ in range(2): + with pytest.raises(TypeError, match="does not expose a signature"): + term.interpret(interpretation) diff --git a/tests/test_tree_interpretation_caching.py b/tests/test_tree_interpretation_caching.py deleted file mode 100644 index 2a33997..0000000 --- a/tests/test_tree_interpretation_caching.py +++ /dev/null @@ -1,71 +0,0 @@ -"""_summary_.""" - -import gc -import weakref -from typing import Any - -import pytest - -from cosy.core.tree import Tree - - -@pytest.fixture -def term() -> Tree: - """_summary_. - - Returns: - Tree: _description_ - """ - return Tree("f", (Tree("x", ()),)) - - -def test_cache_separates_interpretations(term: Tree) -> None: - """_summary_.""" - doubling: dict[str, Any] = {"f": lambda value: value * 2, "x": lambda: 3} - squaring: dict[str, Any] = {"f": lambda value: value * value, "x": lambda: 3} - assert term.interpret(doubling) == 6 - assert term.interpret(squaring) == 9 - assert term.interpret(doubling) == 6 - assert term.interpret(squaring) == 9 - - -def test_repeated_interpretation_hits_cache(term: Tree) -> None: - """_summary_.""" - boxing: dict[str, Any] = {"f": lambda value: [value], "x": lambda: 3} - first = term.interpret(boxing) - second = term.interpret(boxing) - # same object: result was not recomputed - assert first is second - - -def test_cache_neither_grows_or_expires(term: Tree) -> None: - """_summary_.""" - - # exists to be weakly referenced - class Interpretation(dict): - pass - - graveyard: list = [] - for _ in range(100): - interpretation = Interpretation({"f": lambda value: value + 1, "x": lambda: 3}) - graveyard.append(weakref.ref(interpretation)) - assert term.interpret(interpretation) == 4 - # cache remains as only possible anchor - del interpretation - gc.collect() - - assert sum(1 for reference in graveyard if reference() is not None) == 1 - assert graveyard[-1]() is not None - - -def test_indirect_interpretation_is_cached() -> None: - """_summary_.""" - calls: list = [] - - def symbol() -> int: - calls.append(len(calls)) - return calls[-1] - - term = Tree(symbol, ()) - assert term.interpret(None) == 0 - assert term.interpret(None) == 0 diff --git a/tests/test_tree_invariants.py b/tests/test_tree_invariants.py index 988fa0b..bfdd444 100644 --- a/tests/test_tree_invariants.py +++ b/tests/test_tree_invariants.py @@ -11,9 +11,8 @@ implementation does. The second is about the cached fields specifically: a replacement has to recompute them. The third asks the same question about pickling, which is the other way a node can be asked what belongs to it: the derived fields must stay out of the stream. ``_hash`` because a -hash computed under another process's seed breaks the very contract the first group fixes, the -interpretation cache because its key stops naming anything once it has travelled, and the position -sets because carrying them is waste. +hash computed under another process's seed breaks the very contract the first group fixes, and the +position sets because carrying them is waste. """ import os @@ -441,22 +440,21 @@ class grows next. def test_the_interpretation_is_left_behind_and_never_blocks_pickling() -> None: - """The interpretation does not travel with the term, and neither does its cached result. - - It used to, and that was a bug in both directions. Outward: the cache entry held the - interpretation, the interpretation held a lambda, and a single interpreted node therefore - failed to pickle at all -- while terms are pickled to move a population between processes and - an algebra assembled from lambdas is the ordinary case. Inward: the entry is keyed on - ``id(interpretation)``, and a round trip rebuilds that dictionary elsewhere, so the key stops - naming what the entry holds -- and a freed address is free to be handed out again, to an - interpretation the cached result has nothing to do with. + """The interpretation does not travel with the term. + + A node used to keep the result of its last evaluation, together with the interpretation that + produced it, and that was a bug in both directions. Outward: the interpretation held a lambda, + and a single interpreted node therefore failed to pickle at all. Terms are pickled to move a + population between processes, and an algebra assembled from lambdas is the ordinary case. + Inward: the entry was keyed on ``id(interpretation)``, and a round trip rebuilds that + dictionary elsewhere, so the key stopped naming what the entry held. A node holds no result + at all now, and this test keeps it that way. """ tree = Tree("add", (Tree("one"), Tree("two"))) assert tree.interpret({"add": _sum_of, "one": _one, "two": _two}) == 3 blob = pickle.dumps(tree) assert b"_sum_of" not in blob - assert pickle.loads(blob)._interpreted is None # noqa: SLF001 with_lambdas = Tree("add", (Tree("one"), Tree("two"))) assert with_lambdas.interpret({"add": lambda left, right: left + right, "one": lambda: 1, "two": lambda: 2}) == 3 @@ -567,4 +565,3 @@ def test_a_stream_that_still_carries_an_instance_dictionary_is_rebuilt(monkeypat assert back.size == fresh.size assert back._positions is None # noqa: SLF001 assert back._leaf_positions is None # noqa: SLF001 - assert back._interpreted is None # noqa: SLF001 diff --git a/tests/test_tree_performance.py b/tests/test_tree_performance.py index f2020ed..397cd43 100644 --- a/tests/test_tree_performance.py +++ b/tests/test_tree_performance.py @@ -12,6 +12,9 @@ * ``leaf_positions`` filtered the position set against itself, which is quadratic in the number of nodes -- and the leaves of a term are asked for once per query against it. +``interpret`` had a fourth cost: it asked ``inspect.signature`` for the parameters of every +combinator *occurrence*, and that call dominated the evaluation of a term. + The tests below count operations instead of measuring time. A counted operation says the same thing on a loaded machine as on an idle one, whereas a wall-clock bound would only say how busy the machine running the suite happens to be. Where the claim is about growth rather than about a @@ -19,15 +22,19 @@ every call the interpreter makes and so measures work done rather than time taken. """ +import math import sys from collections.abc import Callable from copy import copy +from dataclasses import dataclass +from functools import lru_cache +from inspect import Signature, signature from types import FrameType from typing import Any import pytest -from cosy.core.tree import Path, Tree +from cosy.core.tree import Path, Tree, _parameters_cached, _parameters_of def chain(depth: int) -> Tree[str]: @@ -45,6 +52,23 @@ def chain(depth: int) -> Tree[str]: return node +def cycling_chain(labels: tuple[str, ...], depth: int) -> Tree[str]: + """Build a unary chain whose labels cycle through ``labels``. + + Args: + labels (tuple[str, ...]): The labels to cycle through, innermost first. + depth (int): The number of unary nodes above the leaf. + + Returns: + Tree[str]: The chain. Neighboring nodes carry different labels, so evaluating it asks for + the combinators in rotation rather than in blocks. + """ + node: Tree[str] = Tree("leaf", ()) + for index in range(depth): + node = Tree(labels[index % len(labels)], (node,)) + return node + + def shared_layers(depth: int) -> Tree[str]: """Build a complete binary tree of the given depth out of one node object per level. @@ -172,10 +196,7 @@ def test_replace_subtree_at_shares_everything_off_the_path() -> None: """Only the nodes between the root and the replaced position are new. This is the contract the docstring states, and the reason a replacement is cheap: an offspring - assembled by crossover costs the depth of the crossover point, not the size of the parent. It - is also what keeps the per-node interpretation cache useful across a generation -- a rebuilt - node starts out with an empty one, so a copying implementation would make every individual - look new to ``interpret`` even where nothing about it changed. + assembled by crossover costs the depth of the crossover point, not the size of the parent. """ tree = Tree("f", (Tree("g", (Tree("x"), Tree("w"))), Tree("y"))) replacement = Tree("z") @@ -302,3 +323,216 @@ def test_a_shared_node_reports_one_set_to_every_term_that_holds_it() -> None: with pytest.raises(AttributeError): subtree.positions().add((42,)) # type: ignore[attr-defined] assert (42,) not in grafted.children[1].positions() + + +# --------------------------------------------------------------------------- +# interpret: one signature per combinator, not one per occurrence +# --------------------------------------------------------------------------- + + +def counting_signatures(monkeypatch: pytest.MonkeyPatch) -> list[Any]: + """Record every callable whose signature is asked for, against an empty memo. + + The memo is module state that the whole process shares, so a test that read the shipped one + would depend on what ran before it, and one that filled it would leave its closures behind. + + The bound is read off the shipped memo rather than repeated here, so the test still asks about + what is shipped while the memo the rest of the process shares is neither emptied nor left + holding this test's closures. + + Args: + monkeypatch (pytest.MonkeyPatch): Used to stand an empty memo in for the shared one. + + Returns: + list[Any]: The list the recorded callables are appended to. + """ + calls: list[Any] = [] + + def recording(obj: Any) -> Signature: + """Record the object and delegate. + + Args: + obj (Any): The callable whose signature is asked for. + + Returns: + Signature: The signature of ``obj``. + """ + calls.append(obj) + return signature(obj) + + monkeypatch.setattr("cosy.core.tree.signature", recording) + monkeypatch.setattr("cosy.core.tree._parameters_cached", empty_memo()) + return calls + + +def empty_memo(): + """Return an empty memo of the shipped size, wrapping the shipped function. + + Returns: + Any: The memo, ready to stand in for ``_parameters_cached``. + """ + return lru_cache(maxsize=_parameters_cached.cache_parameters()["maxsize"])(_parameters_cached.__wrapped__) + + +def test_interpret_asks_once_per_combinator_even_when_they_alternate(monkeypatch: pytest.MonkeyPatch) -> None: + """A signature is asked for once per combinator, not once per node that carries it. + + A chain over a single combinator is served by a memo of any size at all, so it says nothing. + Here the labels rotate, which is the shape a real term has: every step asks for a different + combinator and comes back to the first one five steps later. + + Args: + monkeypatch (pytest.MonkeyPatch): The pytest monkeypatch fixture. + """ + calls = counting_signatures(monkeypatch) + + labels = ("a", "b", "c", "d", "e") + interpretation: dict[str, Any] = { + label: (lambda value, step=step: value + step) for step, label in enumerate(labels, start=1) + } + interpretation["leaf"] = lambda: 0 + + assert cycling_chain(labels, 300).interpret(interpretation) == 900 + + assert len(calls) == len(interpretation), ( + f"asked for {len(calls)} signatures over {len(interpretation)} combinators" + ) + + +def test_a_whole_algebra_of_repository_size_stays_in_the_memo(monkeypatch: pytest.MonkeyPatch) -> None: + """The bound holds a whole algebra, so a round trip through one keeps hitting. + + An LRU one entry short of its working set evicts exactly the entry the next lookup asks for, + so it does not degrade gradually but all at once. That is what the bound is generous for. + Sixty-five combinators is well past the algebras in use here. + + Args: + monkeypatch (pytest.MonkeyPatch): The pytest monkeypatch fixture. + """ + calls = counting_signatures(monkeypatch) + + labels = tuple(f"c{index}" for index in range(64)) + algebra: dict[str, Any] = {label: (lambda value: value + 1) for label in labels} + algebra["leaf"] = lambda: 0 + + assert cycling_chain(labels, 3 * len(labels)).interpret(algebra) == 3 * len(labels) + + assert len(calls) == len(algebra) + + +def test_a_fresh_algebra_per_evaluation_does_not_grow_the_memo(monkeypatch: pytest.MonkeyPatch) -> None: + """A caller that rebuilds its algebra for every evaluation must not grow the memo without end. + + The pattern is the ordinary one: an algebra assembled inside the call that evaluates the term, + so every set of callables is used once and is unreachable afterwards. Unbounded, the memo + would keep all of them, together with whatever they close over. Bounded, it holds what the + last evaluations needed and forgets the rest. + + Args: + monkeypatch (pytest.MonkeyPatch): The pytest monkeypatch fixture. + """ + memo = empty_memo() + monkeypatch.setattr("cosy.core.tree._parameters_cached", memo) + bound = memo.cache_parameters()["maxsize"] + + tree: Tree[str] = Tree("leaf", ()) + for _ in range(20): + tree = Tree("step", (tree,)) + for _ in range(2 * bound): + assert tree.interpret({"leaf": lambda: 0, "step": lambda value: value + 1}) == 20 + + assert memo.cache_info().currsize <= bound + + +def test_a_combinator_without_parameters_is_remembered_like_any_other(monkeypatch: pytest.MonkeyPatch) -> None: + """A nullary combinator's answer is the empty tuple, and an empty answer is still an answer. + + Leaves are most of the nodes of a term, and their combinators take no arguments. A memo that + tested its stored answer for truth rather than for presence would miss on every one of them + while looking fully effective on the inner nodes. + + Args: + monkeypatch (pytest.MonkeyPatch): The pytest monkeypatch fixture. + """ + calls = counting_signatures(monkeypatch) + + interpretation: dict[str, Any] = {"pair": lambda left, right: left + right, "leaf": lambda: 1} + tree: Tree[str] = Tree("leaf", ()) + for _ in range(50): + tree = Tree("pair", (tree, Tree("leaf", ()))) + + assert tree.interpret(interpretation) == 51 + + assert len(calls) == 2, f"asked for {len(calls)} signatures over 2 combinators" + + +@dataclass +class UncacheableAdd: + """A combinator that cannot key the memo. + + A dataclass keeps the default ``__eq__``, which sets ``__hash__`` to ``None``. A combinator + written as a value object rather than as a function ends up unhashable that way, without its + author ever thinking about caching. + """ + + def __call__(self, left: int, right: int) -> int: + """Add two numbers. + + Args: + left (int): The first summand. + right (int): The second summand. + + Returns: + int: Their sum. + """ + return left + right + + +def test_interpret_accepts_a_combinator_that_cannot_be_cached() -> None: + """An unhashable combinator is inspected directly instead of being rejected. + + This is the one failure mode the memo introduces: before it existed every callable worked, and + a combinator that cannot be a dictionary key must not start raising ``TypeError`` from inside + the cache. The memo is skipped for it, not consulted and not blamed. + """ + combinator = UncacheableAdd() + assert type(combinator).__hash__ is None + + tree = Tree("add", (Tree("one"), Tree("two"))) + assert tree.interpret({"add": combinator, "one": lambda: 1, "two": lambda: 2}) == 3 + + +def test_interpret_still_reports_a_combinator_without_a_signature() -> None: + """A built-in without an introspectable signature is still a hard error. + + The memo must not turn the ``TypeError`` of ``interpret`` into a silent success or into an + error raised by the memo itself: a failure is reported, never replaced by a value. + """ + tree = Tree("b", (Tree("leaf", ()),)) + with pytest.raises(TypeError, match="does not expose a signature"): + tree.interpret({"b": math.log, "leaf": lambda: 1.0}) + + +def test_the_memo_hands_out_something_a_caller_cannot_corrupt() -> None: + """The stored answer is a tuple, because the memo hands out the object it stored. + + A caller that appended to a list it received would change what every later evaluation of that + combinator reads. + """ + + def combinator(left: int, right: int) -> int: + """Add two numbers. + + Args: + left (int): The first summand. + right (int): The second summand. + + Returns: + int: Their sum. + """ + return left + right + + parameters = _parameters_of(combinator) + + assert isinstance(parameters, tuple) + assert _parameters_of(combinator) is parameters