Repository navigation
Conversation
supplied_input and supplied_input_periods checked only that core recorded the input and that Holder._get_array_from_storage returned something. That reads memory before disk and does not ask whether the value is the input, so a value cached over a supplied input counted as supplied, and an input on disk under a value cached in memory was read from memory. Both now use the tier rule of stored_supplied_value (Holder's _stores_user_input), keep the nearest-branch-first order, and keep the neutralised-variable behaviour. Fixes #2307. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Example tests for a value cached over an input in memory and on disk, an input on disk under a value cached in memory, branch precedence and a neutralised variable. A Hypothesis model of inputs set, values cached, arrays deleted and calculations run over a three-branch chain and both tiers checks the helpers against the model, against each other, and against policyengine-core's Holder._stores_user_input and (from 3.33.1) Simulation._get_recorded_input_array. A last test calculates with every registered variable supplied and checks each input is still supplied, so normal calculation never hit the defect. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
On policyengine-core 3.33.2 and later, the property test also checks that the periods the helper lists are the periods core lists. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Oct 11, 2026
This branch has not been deployed
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
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 #2307
Stacked on #2265. #2265 adds
stored_supplied_value, and this PR builds on it rather than adding a second copy. The PR targets #2265's branch (fix-subsample-2254), so the diff shows only this change. When #2265 merges and its branch is deleted, GitHub moves this PR tomain. CI runs only for PRs intomain, so it runs then.What was wrong
supplied_inputandsupplied_input_periodstreated a value cached over a supplied input as supplied. Reproduced onmainat 6d89338 with policyengine-core 3.32.27 (the locked version) and 3.33.2. The reproduction is in #2307: afterset_inputof 100 andput_in_cacheof 999 for the same period,supplied_inputreturned[999.]andsupplied_input_periodsreturned[2025]. Core'sHolder._stores_user_inputreturnedFalse, and on 3.33.2Simulation._get_recorded_input_arrayreturnedNone.Why
Both helpers checked only two things: that core recorded the input (
Simulation._user_input_keys), and thatHolder._get_array_from_storagereturned something. That method reads memory first, then disk, and does not ask whether the value it finds is the input. The record outlives a value cached over the input.put_in_cachestores over the input and takes that tier off the holder's record of where the input is (Holder._user_input_storage), but the_user_input_keysentry stays. So there were two wrong answers:What changes
supplied_inputreads throughstored_supplied_value(from Give UK simulations a working subsample #2265). That reads a supplied value only from a tier that the holder records the input in and that holds a value the engine did not calculate. This is the ruleHolder._stores_user_inputuses.supplied_input_periodsuses the same rule. It finds the tier without reading the value, which may be on disk. To share that step,stored_supplied_valuenow calls a new private_supplied_value_storage. Whatstored_supplied_valuereturns is unchanged.supplied_inputstill looks at this branch first, then each branch it was made from, then the default branch. If a branch's input is gone, it reads the next one, as core's_get_recorded_input_arraydoes.SUPPLIED_INPUT_VARIABLESand its check.drop_missing_supplied_inputs. It forgets a record whose value is gone from both tiers, the population-rebuild case (policyengine-core#605). It keeps a record whose input a cached value has replaced. That is harmless now: the helpers ignore such a record, and so does core's input export (_stores_user_input).Does this change results?
Not in normal calculation. The defect shows only when code stores a value with
Holder.put_in_cacheand its defaultderived=False, or withHolder._set.Simulationstores every value it calculates withput_in_cache(..., derived=True).put_in_cachewithderived=Truestores nothing when the valueget_arraywould read for that period is an input.set_inputand its period helpers, and the input replay in_invalidate_all_caches.put_in_cachenorHolder._set.put_in_cachecalls only in US code: one crfb-tob-impacts worker and two policyengine-us tests.test_calculating_leaves_every_supplied_value_suppliedexecutes this claim. It supplies every variable inSUPPLIED_INPUT_VARIABLESfor two years, on the default branch and on a branch, and calculates ten outputs over five years on both. Then it checks that each input is still supplied and unchanged, and that core still counts every recorded input as stored. This test also passes on the code before the fix.What the callers ask:
utils/bus_fares.py(has_bus_journeys) asks whether any bus journeys were supplied.person_bus_fare_spendingasks for the supplied householdbus_fare_spendingand the supplied journey counts.ni_class_4_trading_lossasks for the suppliedtrading_lossof the year.ni_class_4_losses_brought_forwardasks for the years with a supplied loss or balance, and for the supplied balance.None of these answers changes unless a value has been cached over the input by one of the routes above.
Invariants
Stated in
policyengine_uk/tests/test_supplied_inputs.py. The model covers every sequence of these steps: inputs set, values cached (calculated or not), arrays deleted and calculations run. Each step can act on a branch, on the branch it was made from or on the default branch, in memory or on disk.supplied_inputreturns the input on the nearest branch that still stores one for the period, reading memory before disk. Otherwise it returnsNone. An input stays stored until it is deleted, or until another input or a cached value is stored over it in the same tier. A value cached in memory over an input on disk leaves that input. A calculated value never replaces an input.supplied_inputreturns a value for a period exactly whensupplied_input_periodslists that period, and the list runs from the earliest period.Holder._stores_user_inputdoes.supplied_inputreturns whatSimulation._get_recorded_input_arrayreturns.supplied_input_periodslists the periods thatSimulation._get_set_input_periodslists.Intended: a neutralised variable whose input is still stored reads as its default.
Tests
policyengine_uk/tests/test_supplied_inputs.pyhas 15 tests:set_inputfirst drops cached values that may depend on the input, so an input can go to disk where the model expected memory. The model now records the tier each value is actually stored in, and the case is pinned as an explicit example.--basetemp:test_bus_fare_reforms.py,test_input_records.py,test_ni_class_4_profits_properties.pyandtest_subsample.pygave 103 passed and 1 skipped. The skip is Give UK simulations a working subsample #2265's 3.33-only differential.bus_journeys,person_bus_fare_spending,loss_relief,ni_class_2andni_class_4_profits.uvx ruff format --check .anduvx ruff check .pass with ruff 0.17.0, the latest release.main. On 2026-10-11, the Test job of Give UK simulations a working subsample #2265 failed only intest_dataframe_dataset.py::test_an_exported_frame_loads_back_and_exports_itself, which fails onmaintoo, and then the runner was shut down.axiom: n/a: simulation infrastructure, no policy rule change
🤖 Generated with Claude Code