Copy only set parameters for YAML inputs, share the reform-free tree, and run test batches in forked workers

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.

changelog.d/lazy-parameter-node-at-instant.changed.md

@@ -0,0 +1 @@
+Resolve `ParameterNodeAtInstant` children on first read instead of building the whole parameter tree for every instant, which cost policyengine-us 16 to 33 MB and several seconds per instant on every tax-benefit system.

changelog.d/modify-parameters-cached-instants.fixed.md

@@ -0,0 +1 @@
+`modify_parameters` drops the values a parameter tree has cached at each instant once its modifier has run, so a modifier's in-place changes to values, brackets or children are read even where the system had already read those parameters.

changelog.d/parameter-cache-invalidation.fixed.md

@@ -0,0 +1 @@
+`Parameter.update` on a bracket of a freshly loaded scale and `ParameterNode.add_child` clear the cached parameters of every node above them, `ParameterNode.clear_instant_caches` clears them after direct edits to values, brackets or children, and a tax-benefit system clears them once its parameter operations have run.

changelog.d/parameters-at-instant-cache-follows-updates.fixed.md

@@ -0,0 +1 @@
+`TaxBenefitSystem.get_parameters_at_instant` returns the parameters as they are after a `Parameter.update`, instead of the node it had cached before the change.

changelog.d/reform-modify-parameters-own-tree.changed.md

@@ -0,0 +1 @@
+`Reform.modify_parameters` no longer deep-copies the baseline's parameter tree. The modifier changes the reform's own copy, as it already did for a reform applied with `apply_reform_set`, so changes made earlier in `apply` are kept and building such a reform skips a full copy of the tree.

changelog.d/test-runner-perf.added.md

@@ -0,0 +1 @@
+The YAML test runner sets a case's dotted parameter inputs on a reform that copies only those parameters' paths instead of the whole tree, builds reform systems on the shared reform-free system so each holds one copy of the parameter tree, adds a `--read-only-arrays` option that fails a formula writing into an array it read, and `policyengine_core.tools.forked_test_runner.run_batches_in_forks` runs test batches in children forked from one process that built the tax-benefit system.

docs/engineering/skills/testing.md

@@ -36,3 +36,50 @@ 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` runs on a system built for
+  that combination on the reform-free system: a reform copies the parameter
+  tree before it changes it, so each holds one copy of the tree. The runner
+  caches the `--reform-cache-size` most recently used of them (default 2) plus
+  the reform-free system, and drops one before building another when full.
+  Do not add a cache to the runner that grows with the number of cases or
+  combinations.
+- A case that sets a parameter with a dotted input key runs on a system of its
+  own (`_ParameterInputReform`) that copies only the nodes on the paths to the
+  parameters it sets (`policyengine_core.parameters.path_copy`) and shares the
+  rest of the tree, with its cached at-instant values, with the system its
+  `reforms` select. Nothing may change a shared node in place.
+- 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).
+
+`policyengine_core.tools.forked_test_runner.run_batches_in_forks` runs batches
+in children forked from a process that has already built the system, so a
+batch skips the import and build, and the children share the parent's system
+in memory until they write to it. `tests/core/tools/test_runner/test_forked_test_runner.py`
+covers its exit codes, worker limit and timeout.
+
+`--read-only-arrays` (option `read_only_arrays`) stores every array read-only
+for the run, so a formula that writes into an array it read from another
+variable fails at that line (`test_read_only_arrays.py`).

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/data_storage/in_memory_storage.py

@@ -85,6 +85,14 @@ class InMemoryStorage:
     # holder per branch for inputs most branches never change.
     _inputs: Union[Set[str], FrozenSet[str]] = _NO_INPUTS
     is_eternal: bool
+    # Whether every storage stores and hands out read-only views of numpy
+    # arrays, so that code writing into an array it read (``x += y`` on the
+    # result of ``tax_unit("x", period)``) raises at that line instead of
+    # changing ``x`` for every later reader. Off by default; the YAML test
+    # runner turns it on for a run with the ``read_only_arrays`` option. The
+    # caller's own array stays writeable: only the stored view is protected.
+    # Masked arrays are left as they are, since a view shares their mask.
+    read_only_arrays: bool = False
 
     def __init__(self, is_eternal: bool):
         self._arrays = {}
@@ -182,6 +190,8 @@ def get(self, period: Period, branch_name: str = "default") -> ArrayLike:
             # First read of an array shared by ``clone``: from here on the
             # caller may write into it, so it needs to be this storage's own.
             values = values.copy()
+            if self.read_only_arrays:
+                values = _read_only_view(values)
             self._arrays[key] = values
             self._stop_sharing(key)
         return values
@@ -304,6 +314,8 @@ def put(
                 "string form is lossy (see policyengine-core#526)."
             )
         key = f"{branch_name}:{period}"
+        if self.read_only_arrays and _can_share(value):
+            value = _read_only_view(value)
         self._arrays[key] = value
         self._stop_sharing(key)
         if derived:

policyengine_core/parameters/parameter_node.py

@@ -182,6 +182,8 @@ def add_child(self, name: str, child: Union["ParameterNode", Parameter]):
         self.children[name] = child
         setattr(self, name, child)
         child.parent = self
+        # Nodes at instant cached here and above were built without it.
+        self.clear_parent_cache()
 
     def __repr__(self) -> str:
         result = os.linesep.join(
@@ -214,31 +216,60 @@ 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
 
     def clear_parent_cache(self):
-        self._at_instant_cache.clear()
-        if self.parent is not None:
-            self.parent.clear_parent_cache()
+        """Clear the nodes at instant cached by this node and every node above
+        it (scales cache nothing and are passed through).
+
+        Walks up without recursing and stops at a node it has already
+        cleared: while a node is loaded, a child named ``parent`` (as
+        policyengine-us's ``gov.hhs.medicaid.eligibility.categories.parent``)
+        replaces its ``parent`` attribute and points back at it until the node
+        is attached to its own parent.
+        """
+        node, cleared = self, set()
+        while node is not None and id(node) not in cleared:
+            cleared.add(id(node))
+            cache = getattr(node, "_at_instant_cache", None)
+            if cache is not None:
+                cache.clear()
+            node = getattr(node, "parent", None)
 
     def mark_as_modified(self):
         self.modified = True
         if self.parent is not None:
             self.parent.mark_as_modified()
 
+    def clear_instant_caches(self) -> None:
+        """Drop the nodes at instant cached by this node, every node below it
+        and every node above it.
+
+        ``Parameter.update`` and ``add_child`` clear what they change. Call
+        this after changing ``values_list``, a scale's ``brackets`` or
+        ``children`` directly, so that nodes at instant requested afterwards
+        read the change.
+        """
+        self.clear_parent_cache()
+        for descendant in self.get_descendants():
+            if isinstance(descendant, ParameterNode):
+                descendant._at_instant_cache.clear()
+
     def get_child(self, path: str) -> "ParameterNode":
         node = self
         for name in path.split("."):