Stop the YAML test runner keeping every case's simulation and reform system

changelog.d/fix-test-runner-memory.fixed.md

@@ -0,0 +1 @@
+Stop the YAML test runner keeping per-case state alive for the whole run: finished cases drop runner-owned simulation references (also when the simulation failed to build) and any tracer, the reform-system cache keeps the 2 most recently used (`--reform-cache-size`), and neither a traced parameter tree nor the tax-benefit system's cache of it at each instant (`get_parameters_at_instant`) keeps tracer-bound wrappers. A miss on the reform-free system now builds with automatic collection paused too, and the full-collection limit counts each baseline's reform-free system plus its cached reform systems.

docs/engineering/skills/testing.md

@@ -36,3 +36,33 @@ broader behavioral change, run the relevant focused tests and formatting check.
 - Prefer small synthetic fixtures for regression tests.
 - When fixing a bug, add a regression test that fails without the fix and passes
   with it unless the change is documentation-only.
+
+## YAML suites and memory
+
+`policyengine-core test <paths> -c <country_package>` runs every case in one
+process. Its memory is bounded by design, and tests must keep it that way:
+
+- A finished case releases its simulation (`YamlItem.teardown`). pytest keeps
+  every collected item until the session ends, so anything stored on the item
+  lives for the whole run.
+- A case that names `reforms` or `extensions`, or sets a parameter with a
+  dotted input key, runs on a system built for that combination: a full copy
+  of the baseline system. The runner caches the `--reform-cache-size` most
+  recently used of them (default 2) plus the reform-free system. Do not add a
+  cache to the runner that grows with the number of cases or combinations.
+- Each system still caches its parameter tree at every distinct instant its
+  cases read (`ParameterNode.get_at_instant`,
+  `TaxBenefitSystem.get_parameters_at_instant`), so memory grows with the
+  number of distinct dates a suite reads, not with the number of cases.
+  Neither cache may hold a tracer: a read of a traced tree is wrapped for that
+  read only.
+- `tests/core/tools/test_runner/test_runner_memory.py` holds the regression
+  tests: a few hundred cases in one process must not grow traced memory or
+  the number of live simulations and systems, and traced cases at distinct
+  dates must leave no tracer or recorded result alive.
+
+Run one large suite at a time. A country package's whole YAML tree in one
+process still holds the baseline system, the reform-free copy and the cached
+reform systems at once; use the country package's own batch runner where it
+has one (policyengine-us: `policyengine_us/tests/test_batched.py` and the
+`make test-yaml-*` targets).

docs/python_api/tools.md

@@ -8,6 +8,9 @@ The `policyengine_core.tools` module contains miscellaneous utility functions, i
 .. autofunction:: policyengine_core.tools.test_runner.run_tests
 ```
 
+The same runner backs `policyengine-core test <paths> -c <country_package>`,
+where `--reform-cache-size N` sets `reform_cache_size`.
+
 ## dump_simulation
 
 ```{eval-rst}

policyengine_core/parameters/parameter_node.py

@@ -215,17 +215,19 @@ def clone(self) -> "ParameterNode":
         return clone
 
     def _get_at_instant(self, instant: Instant) -> ParameterNodeAtInstant:
-        if instant in self._at_instant_cache:
-            return self._at_instant_cache[instant]
-        node_at_instant = ParameterNodeAtInstant(self.name, self, instant)
+        node_at_instant = self._at_instant_cache.get(instant)
+        if node_at_instant is None:
+            node_at_instant = ParameterNodeAtInstant(self.name, self, instant)
+            self._at_instant_cache[instant] = node_at_instant
         if self.trace:
-            at_instant = TracingParameterNodeAtInstant(
+            # Wrapped on each read and never cached: a cached wrapper kept the
+            # tracer of whichever simulation first read this instant alive for
+            # the life of the parameters, and recorded later reads into it,
+            # while a node cached before tracing was turned on went untraced.
+            return TracingParameterNodeAtInstant(
                 node_at_instant, self.tracer, self.branch_name
             )
-        else:
-            at_instant = node_at_instant
-        self._at_instant_cache[instant] = at_instant
-        return at_instant
+        return node_at_instant
 
     def attach_to_parent(self, parent: "ParameterNode"):
         self.parent = parent

policyengine_core/scripts/policyengine_command.py

@@ -9,6 +9,13 @@
 """
 
 
+def non_negative_int(value: str) -> int:
+    number = int(value)
+    if number < 0:
+        raise argparse.ArgumentTypeError(f"must be 0 or more, got {number}")
+    return number
+
+
 def get_parser():
     parser = argparse.ArgumentParser()
 
@@ -91,6 +98,12 @@ def build_test_parser(parser):
             default=None,
             help="variables to ignore. If specified, do not test the given variables.",
         )
+        parser.add_argument(
+            "--reform-cache-size",
+            type=non_negative_int,
+            default=None,
+            help="how many reform systems (built for tests with reforms, extensions or parameter inputs) to keep cached; each is a full copy of the tax-benefit system. Defaults to 2; 0 caches none.",
+        )
 
         return parser
 

policyengine_core/scripts/run_test.py

@@ -30,6 +30,7 @@ def main(parser):
         "only_variables": args.only_variables,
         "ignore_variables": args.ignore_variables,
         "visualize": args.visualize,
+        "reform_cache_size": args.reform_cache_size,
     }
 
     paths = [os.path.abspath(path) for path in args.path]

policyengine_core/taxbenefitsystems/tax_benefit_system.py

@@ -54,6 +54,7 @@
 )
 from policyengine_core.periods import Instant, Period
 from policyengine_core.populations import GroupPopulation, Population
+from policyengine_core.tracers import TracingParameterNodeAtInstant
 from policyengine_core.variables import Variable
 from policyengine_core.variables.variable import NUMERIC_VALUE_TYPES
 
@@ -659,10 +660,23 @@ def get_parameters_at_instant(self, instant: Instant) -> ParameterNodeAtInstant:
                 )
             )
 
+        parameters = self.parameters
         parameters_at_instant = self._parameters_at_instant_cache.get(instant)
-        if parameters_at_instant is None and self.parameters is not None:
-            parameters_at_instant = self.parameters.get_at_instant(str(instant))
+        if parameters_at_instant is None and parameters is not None:
+            parameters_at_instant = parameters.get_at_instant(str(instant))
+            if isinstance(parameters_at_instant, TracingParameterNodeAtInstant):
+                parameters_at_instant = parameters_at_instant.parameter_node_at_instant
             self._parameters_at_instant_cache[instant] = parameters_at_instant
+        if parameters_at_instant is not None and getattr(parameters, "trace", False):
+            # Wrapped on each read and never cached, as the tree's own reads
+            # are (``ParameterNode._get_at_instant``): a cached wrapper kept
+            # the tracer of whichever simulation first read this instant
+            # alive for the life of the system, and recorded later reads into
+            # it, while a node cached before tracing was turned on went
+            # untraced.
+            return TracingParameterNodeAtInstant(
+                parameters_at_instant, parameters.tracer, parameters.branch_name
+            )
         return parameters_at_instant
 
     def get_package_metadata(self) -> dict: