Replace the runtime randomness guard with a static registration-time check

changelog.d/randomness-guard-module-removed.removed.md

@@ -0,0 +1 @@
+The internal `policyengine_core.simulations.randomness_guard` module (the `forbid_randomness` runtime guard added in 3.27.0) has been removed. Formula-time randomness is now rejected statically at variable registration, and `NonDeterministicFormulaError` now lives in `policyengine_core.errors` (also re-exported from `policyengine_core.variables.formula_randomness`).

changelog.d/static-formula-randomness-check.fixed.md

@@ -0,0 +1 @@
+Formula-time randomness is now rejected by a static check when a variable is registered, instead of a runtime guard that mutated the process-global `numpy.random`. This fixes intermittent, misattributed `NonDeterministicFormulaError`s raised under concurrent simulations, where one thread's running formula collided with another thread's legitimate setup seeding in `Simulation.__init__` (policyengine-core#518). The check fails fast at load, names the offending variable, and additionally catches `from numpy.random import ...` drawing functions and module-scope generators that the runtime guard could not.

policyengine_core/errors/__init__.py

@@ -1,6 +1,7 @@
 from .cycle_error import CycleError
 from .empty_argument_error import EmptyArgumentError
 from .nan_creation_error import NaNCreationError
+from .non_deterministic_formula_error import NonDeterministicFormulaError
 from .parameter_not_found_error import ParameterNotFoundError
 from .parameter_parsing_error import ParameterParsingError
 from .period_mismatch_error import PeriodMismatchError

policyengine_core/errors/non_deterministic_formula_error.py

@@ -0,0 +1,9 @@
+class NonDeterministicFormulaError(RuntimeError):
+    """A variable's formula references a random number generator.
+
+    Rules-engine formulas must be deterministic functions of their inputs.
+    Raised at variable registration by
+    ``policyengine_core.variables.formula_randomness.check_formula_determinism``.
+    """
+
+    pass

policyengine_core/simulations/simulation.py

@@ -14,7 +14,6 @@
 from policyengine_core.entities.entity import Entity
 from policyengine_core.enums import Enum, EnumArray
 from policyengine_core.errors import CycleError, SpiralError
-from policyengine_core.simulations.randomness_guard import forbid_randomness
 from policyengine_core.holders.holder import Holder
 from policyengine_core.periods import Period
 from policyengine_core.periods.config import ETERNITY, MONTH, YEAR
@@ -593,8 +592,9 @@ def calculate(
         self.tracer.record_calculation_start(variable_name, period, self.branch_name)
 
         # No per-variable RNG seeding: formulas may not use randomness at all
-        # (enforced by forbid_randomness in _run_formula), so there is nothing
-        # to make reproducible here.
+        # (enforced statically at variable registration by
+        # check_formula_determinism), so there is nothing to make reproducible
+        # here.
 
         try:
             result = self._calculate(variable_name, period)
@@ -1106,13 +1106,12 @@ def _run_formula(
         parameters_at = self.tax_benefit_system.parameters
 
         # A rules-engine formula must be a pure, deterministic function of its
-        # inputs. Forbid any random number generation while it runs so the same
-        # inputs always produce the same outputs.
-        with forbid_randomness(variable.name):
-            if formula.__code__.co_argcount == 2:
-                array = formula(population, period)
-            else:
-                array = formula(population, period, parameters_at)
+        # inputs. Randomness is forbidden statically at variable registration
+        # (check_formula_determinism), so no runtime guard is needed here.
+        if formula.__code__.co_argcount == 2:
+            array = formula(population, period)
+        else:
+            array = formula(population, period, parameters_at)
 
         return array
 

policyengine_core/variables/formula_randomness.py

@@ -0,0 +1,168 @@
+"""Static detection of formula-time randomness (policyengine-core#518).
+
+A rules-engine formula must be a pure, deterministic function of its inputs.
+Rather than forbidding randomness with a runtime monkeypatch of the
+process-global ``numpy.random`` -- which is not safe under concurrent
+simulations (see #518: the guard installed by one thread's formula leaks onto
+another thread's legitimate setup ``np.random.seed(0)``) -- this module scans a
+formula's bytecode *once, when the variable is constructed* and raises if it
+references numpy's or the standard library's random facilities.
+
+Detection is by **identity**, not by name: a local variable, a string argument,
+or an attribute that merely happens to be spelled ``random`` is never flagged.
+Only genuine *global* and *closure* loads are resolved against the formula's
+namespace (attribute loads such as ``foo.random`` are not), which keeps the
+check free of name-collision false positives.
+
+Because it is static, this also catches forms the old runtime guard explicitly
+could not: ``from numpy.random import default_rng`` (a drawing callable imported
+by name) and a generator hoisted to module scope or captured through a closure
+(``rng = np.random.default_rng(0)``) and used inside the formula.
+"""
+
+from __future__ import annotations
+
+import dis
+import random as _stdlib_random
+from types import ModuleType
+from typing import Optional
+
+import numpy
+
+from policyengine_core.errors import NonDeterministicFormulaError
+
+_RANDOM_MODULES = (numpy.random, _stdlib_random)
+_RNG_INSTANCE_TYPES = (numpy.random.Generator, numpy.random.RandomState)
+
+# Scanning is memoised by code object: each unique formula is inspected once,
+# no matter how many tax-benefit systems instantiate the variable.
+_cache: dict = {}
+
+
+def _is_random_module(obj: object) -> bool:
+    """True if ``obj`` is the ``numpy.random`` or stdlib ``random`` module."""
+    return any(obj is module for module in _RANDOM_MODULES)
+
+
+def _is_random_member(obj: object) -> bool:
+    """True if ``obj`` is a callable / generator that belongs to a random module.
+
+    Catches ``from numpy.random import default_rng`` / ``from random import
+    random`` (imported drawing callables), pre-built ``Generator`` /
+    ``RandomState`` instances hoisted to module scope, and the retired
+    ``commons.formulas.random`` helper (kept as a raiser since #494).
+    """
+    module = getattr(obj, "__module__", None)
+    if isinstance(module, str) and (
+        module == "random" or module.startswith("numpy.random")
+    ):
+        return True
+    if isinstance(obj, _RNG_INSTANCE_TYPES):
+        return True
+    if (
+        module == "policyengine_core.commons.formulas"
+        and getattr(obj, "__name__", None) == "random"
+    ):
+        return True
+    return False
+
+
+def _describe(obj: object) -> str:
+    """A short, human-readable name for the offending reference."""
+    if isinstance(obj, ModuleType):
+        return obj.__name__
+    module = getattr(obj, "__module__", None)
+    name = getattr(obj, "__name__", None)
+    if module and name:
+        return f"{module}.{name}"
+    if isinstance(obj, _RNG_INSTANCE_TYPES):
+        return f"numpy.random.{type(obj).__name__} instance"
+    return repr(obj)
+
+
+def _freevars(formula) -> dict:
+    """Map each of a formula's closure variable names to its current value."""
+    result: dict = {}
+    if formula.__closure__:
+        for name, cell in zip(formula.__code__.co_freevars, formula.__closure__):
+            try:
+                result[name] = cell.cell_contents
+            except ValueError:
+                continue  # empty cell
+    return result
+
+
+def _resolve_load(instruction, global_namespace: dict, freevars: dict) -> object:
+    """Resolve a name-load instruction to the object it loads, or ``None``.
+
+    Only global and closure loads are resolved -- never ``LOAD_ATTR`` -- so an
+    attribute merely spelled ``random`` is not mistaken for the module, even
+    when ``random`` is also imported at module scope.
+    """
+    if instruction.opname in ("LOAD_GLOBAL", "LOAD_NAME"):
+        return global_namespace.get(instruction.argval)
+    if instruction.opname in ("LOAD_DEREF", "LOAD_CLASSDEREF"):
+        return freevars.get(instruction.argval)
+    return None
+
+
+def _referenced_randomness(formula) -> Optional[str]:
+    """Describe the randomness a formula references, or ``None``.
+
+    A single pass over the bytecode: each global/closure load is checked by
+    identity, and a numpy top-level module load followed by a ``.random``
+    attribute access (e.g. ``np.random.seed``) is flagged too.
+    """
+    global_namespace = formula.__globals__
+    freevars = _freevars(formula)
+    previous = None
+    for instruction in dis.get_instructions(formula):
+        loaded = _resolve_load(instruction, global_namespace, freevars)
+        if loaded is not None and (
+            _is_random_module(loaded) or _is_random_member(loaded)
+        ):
+            return _describe(loaded)
+        if (
+            instruction.opname in ("LOAD_ATTR", "LOAD_METHOD")
+            and instruction.argval == "random"
+            and previous is not None
+            and _resolve_load(previous, global_namespace, freevars) is numpy
+        ):
+            return "numpy.random"
+        previous = instruction
+    return None
+
+
+def formula_uses_randomness(formula) -> Optional[str]:
+    """Return a description of the randomness a formula references, or ``None``.
+
+    ``formula`` is a plain function (a ``def formula(...)`` collected from a
+    ``Variable`` subclass). Objects without a code object (e.g. ``None``) return
+    ``None``. Memoised by code object.
+    """
+    code = getattr(formula, "__code__", None)
+    if code is None:
+        return None
+    if code not in _cache:
+        _cache[code] = _referenced_randomness(formula)
+    return _cache[code]
+
+
+def check_formula_determinism(variable) -> None:
+    """Raise ``NonDeterministicFormulaError`` if any of a variable's formulas
+    references randomness.
+
+    Intended to be called from ``Variable.__init__`` alongside
+    ``check_computation_modes`` so a randomness-using formula fails fast, at
+    variable construction, with the offending variable named.
+    """
+    for formula in variable.formulas.values():
+        offending = formula_uses_randomness(formula)
+        if offending is not None:
+            raise NonDeterministicFormulaError(
+                f"The formula for '{variable.name}' references {offending}, but "
+                f"rules-engine formulas must be deterministic functions of their "
+                f"inputs. Remove the random call. If you need a stochastic input, "
+                f"compute it once when building the dataset (with a seeded "
+                f"generator) and store it as an input variable instead."
+            )

policyengine_core/variables/variable.py

@@ -18,6 +18,7 @@
 from policyengine_core.periods import DAY, ETERNITY
 
 from . import config, helpers
+from .formula_randomness import check_formula_determinism
 
 
 class QuantityType:
@@ -325,6 +326,7 @@ def __init__(self, baseline_variable=None):
         self.formulas = self.set_formulas(formulas_attr)
 
         self.check_computation_modes()
+        check_formula_determinism(self)
 
         if unexpected_attrs:
             raise ValueError(