Replace the runtime randomness guard with a static registration-time check#519
Merged
Merged
Conversation
Add policyengine_core/variables/formula_randomness.py: a bytecode-based, identity-driven detector that flags formula-time references to numpy.random and the stdlib random module (module refs, drawing callables imported by name, pre-built Generator/RandomState instances hoisted to module scope, and the retired commons.formulas.random helper), plus check_formula_determinism() for use at variable registration. Detection resolves only genuine global/closure loads (never LOAD_ATTR), so an attribute or local merely spelled 'random' is not flagged. It is memoised by code object. This commit only adds the detector and its unit tests; it is not yet wired into Variable.__init__, so it has no effect on the rest of the system. Closes two forms the runtime guard could not catch: 'from numpy.random import default_rng' and a module-scope generator used inside a formula. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Integration tests that a variable whose formula references randomness is rejected at registration (system.add_variables instantiates it), naming the variable, and that a deterministic variable still registers and computes. Marked strict xfail until the check is wired into Variable.__init__ in the next commit (they currently do not raise, so an XPASS will force the un-xfail). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Call check_formula_determinism(self) right after check_computation_modes in Variable.__init__, so a formula that references randomness is rejected when the variable is registered with a tax-benefit system, naming the variable. Un-xfails the registration tests. Trims test_randomness_guard.py: its detection and former known-gap cases are now enforced statically at registration (covered by test_variable_formula_determinism.py), leaving only the runtime-guard mechanics that still apply until the guard is removed. Full tests/core green (669 passed): every country_template variable and test fixture is deterministic under the wired check. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The runtime guard mutated the shared numpy.random module while a formula ran, which is unsafe under concurrent simulations: the guard installed by one thread's formula leaked onto another thread's legitimate setup np.random.seed(0) (Simulation.__init__), producing intermittent, misattributed NonDeterministicFormulaErrors (cliff-watch#38) and a deploy break (policyengine-household-api#1575). Enforcement now lives in the static registration check added in the previous commits. Drop forbid_randomness from _run_formula, reduce randomness_guard.py to a shim that re-exports NonDeterministicFormulaError (from its new home in variables/formula_randomness.py) for import back-compat, and delete the runtime-guard-only tests. Full tests/core green (666 passed). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Several threads each repeatedly build a default simulation (each build calls the legitimate setup np.random.seed(0)) and compute a formula, synchronized by a barrier to maximise overlap. It asserts no spurious NonDeterministicFormulaError. Verified: 6/6 green against the static check; 6/6 red against the reinstated runtime guard. On the fixed code nothing patches numpy.random, so the error is not merely improbable but impossible -- the test is deterministically green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Detector (formula_randomness.py): - Collapse the two disassembly passes into one _referenced_randomness() pass sharing a _resolve_load() helper; resolving the numpy base via any load type (not just LOAD_GLOBAL) also closes the closure-captured-numpy gap. - Hoist the Generator/RandomState tuple to _RNG_INSTANCE_TYPES and reuse the computed __module__ in _is_random_member (was fetched twice). Tests: - Unit: closures over a generator, a drawing function, and the numpy module (the newly-closed gap), plus a deterministic-closure negative and a back-compat test for the legacy randomness_guard import path. - Integration: randomness in a later dated formula (formula_2020) is rejected, and a reform's update_variable introducing randomness is rejected on apply. - Fix a stale docstring that still described the registration tests as xfail. Full tests/core green (674 passed). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Delete policyengine_core/simulations/randomness_guard.py entirely (it was only a back-compat re-export after the runtime guard was removed). Give NonDeterministicFormulaError a permanent home in policyengine_core.errors, matching the one-class-per-file errors convention; formula_randomness.py imports and re-exports it, so it stays importable from where it is raised. Update the exception-location test to assert the errors package is the canonical home, and add a 'removed' changelog fragment documenting the dropped import path. Full tests/core green (674 passed); ruff clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
1 participant
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.
Fixes #518
Summary
Replaces the runtime formula-time randomness guard — which mutated the process-global
numpy.randomand is unsafe under concurrent simulations — with a static check that runs when a variable is registered.The old guard (
forbid_randomness, added in #500) monkeypatched every public callable ofnumpy.randomand stdlibrandomwhile a formula ran. Because that mutation is process-global, one thread's in-formula guard collided with another thread's legitimate setup seeding —Simulation.__init__'snp.random.seed(0)— raising intermittent, misattributedNonDeterministicFormulaErrors under concurrent load (field repro: PolicyEngine/cliff-watch#38; deploy break: PolicyEngine/policyengine-household-api#1575).What changed
policyengine_core/variables/formula_randomness.py: a bytecode-based, identity-driven detector (formula_uses_randomness) pluscheck_formula_determinism(variable).Variable.__init__(right aftercheck_computation_modes), so a randomness-using formula fails fast at registration, naming the offending variable.Simulation._run_formulaand deletedsimulations/randomness_guard.pyentirely.NonDeterministicFormulaErrornow lives inpolicyengine_core.errors(and is re-exported fromvariables/formula_randomness.py, where it is raised).Why static is the right fix
Global module patching cannot be made correct for "forbid during formulas, allow during setup" when both happen concurrently in one process — name resolution for
np.random.xgoes through a shared module object, so any interception is process-wide. The static check has no shared mutable state, so it is thread- and async-safe by construction, deterministic, and fails at load rather than intermittently in production. It also catches two forms the runtime guard documented as gaps:from numpy.random import ...drawing callables, and pre-built or closure-captured generators.Behaviour change / adoption note
Detection is stricter than the guard it replaces (it closes the two gaps above), so a downstream formula that referenced randomness only through one of those gaps would now fail at system construction rather than slipping through. Verified this is a non-issue for the current production models:
random.Variablesubclasses includingcontrib/(1,747 classes / 1,120 formula methods) → 0 hits. UK'snp.randomlives only in scenario / dynamics / data-construction helpers, never in a variable formula, so the check (which only scansvariable.formulas) does not touch it.Any future formula that reintroduces randomness now fails fast at load — the intended safety property.
Tests
tests/core/variables/test_formula_randomness.py): every detection form (numpy/stdlib module refs, by-name imports, module-scope and closure-captured generators, the retiredcommons.formulas.randomhelper), negatives (deterministic numpy, an attribute/local/string spelledrandom), and the exception's canonicalpolicyengine_core.errorshome.tests/core/variables/test_variable_formula_determinism.py): rejection at registration, a dated formula (formula_2020), and the reformupdate_variablepath; a deterministic variable still registers and computes.tests/core/simulations/test_formula_randomness_concurrency.py): reproduces the Thread-safety: forbid_randomness mutates process-global numpy.random, causing intermittent false NonDeterministicFormulaError under concurrent simulations #518 race (concurrent build + calculate). Green on this branch; verified red against the reinstated runtime guard.tests/coregreen (674 passed);ruff format --check .andruff check .clean.🤖 Generated with Claude Code