Skip to content

supplied_input and supplied_input_periods count a value cached over a supplied input as supplied #2307

Open

Description

@MaxGhenis

What is wrong

supplied_input and supplied_input_periods in policyengine_uk/utils/supplied_inputs.py treat a value cached over a supplied input as supplied.

Run on main at 6d89338 with policyengine-core 3.32.27 (the locked version) and 3.33.2:

import numpy as np
from policyengine_core.periods import period
from policyengine_uk import Simulation
from policyengine_uk.utils.supplied_inputs import supplied_input, supplied_input_periods

situation = {
    "people": {"person": {"age": {2025: 40}}},
    "benunits": {"benunit": {"members": ["person"]}},
    "households": {"household": {"members": ["person"]}},
}
sim = Simulation(situation=situation)
sim.set_input("bus_fare_spending", 2025, [100.0])
holder = sim.get_holder("bus_fare_spending")
holder.put_in_cache(np.array([999.0]), period(2025))
household = sim.populations["household"]
print(holder._stores_user_input(period(2025), "default"))  # False
print(supplied_input(household, "bus_fare_spending", period(2025)))  # [999.]
print(supplied_input_periods(household, "bus_fare_spending"))  # [2025]

supplied_input should return None and supplied_input_periods should return []. On 3.33.2, core's own Simulation._get_recorded_input_array("bus_fare_spending", period(2025)) returns None.

Why

Both helpers check that core recorded the input (Simulation._user_input_keys) and that Holder._get_array_from_storage(period, branch) returns something. That 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_cache replaces the stored value and core takes that tier off the holder's record of where the input is (Holder._user_input_storage), but it keeps the _user_input_keys entry.

policyengine-core decides per tier: Holder._stores_user_input counts a tier only if the holder records the input there and the tier holds a value that is not derived. So the helpers get two cases wrong:

  1. A value cached over the input in the same tier is returned as supplied (the reproduction above).
  2. An input stored on disk with a value cached over it in memory is read from memory, so the cached value is returned instead of the input.

Does it change results today?

From reading core 3.32.27 and 3.33.2, I think not. The engine stores calculated values with put_in_cache(..., derived=True), which stores nothing over an input that get_array reads. The defect needs a direct put_in_cache with its default derived=False, or Holder._set. No policyengine-uk code does that. A code search of the PolicyEngine organisation finds direct put_in_cache calls only in US code. The PR will check this with a test.

Fix

PR #2265 adds stored_supplied_value(holder, period, branch_name), which reads a supplied value only from a tier that the holder records it in, as Holder._stores_user_input does. Both helpers should use it (or the same tier rule) and keep the order supplied_input uses now: the nearest branch first. The neutralised-variable behaviour of supplied_input stays.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions