Skip to content

Use shared WEEKS_IN_YEAR and local RNG in consumption imputation#353

Merged
MaxGhenis merged 2 commits intomainfrom
fix/consumption-weeks-rng
Apr 17, 2026
Merged

Use shared WEEKS_IN_YEAR and local RNG in consumption imputation#353
MaxGhenis merged 2 commits intomainfrom
fix/consumption-weeks-rng

Conversation

@MaxGhenis
Copy link
Copy Markdown
Contributor

Summary

datasets/imputations/consumption.py had two sub-bugs (finding U6):

  1. Weekly-to-annual conversion used a bare * 52 while the sibling FRS loader uses WEEKS_IN_YEAR = 365.25 / 7 ≈ 52.1786. That ~0.34% gap systematically underestimated annualised LCFS consumption relative to FRS income and skewed VAT/energy imputation targets.
  2. Two np.random.seed(42) calls mutated the process-wide RNG state, so any unrelated numpy random consumer that ran after consumption imputation silently changed behaviour.

This PR:

  • Promotes WEEKS_IN_YEAR to module scope in datasets/frs.py (with a docstring) so siblings can import one canonical value, and swaps consumption.py to use it for both the household and person annualisation passes.
  • Replaces both np.random.seed(42) calls with rng = np.random.default_rng(42) and routes the draws through the local RNG, keeping the seed at 42 for bit-for-bit fingerprint compatibility.
  • Adds test_consumption_weeks_rng.py asserting the imported constant matches, the module source no longer contains np.random.seed(, and re-importing consumption does not perturb the global RNG state.

Test plan

  • uv run pytest policyengine_uk_data/tests/test_consumption_weeks_rng.py -q passes.

Finding U6 from the bug hunt.

@MaxGhenis
Copy link
Copy Markdown
Contributor Author

Self-review: APPROVE. See /tmp/bug-hunt/reviews/uk-data-reviews.md for detail. (GitHub blocks self-approve, so leaving as comment.)

@MaxGhenis MaxGhenis merged commit 959d47f into main Apr 17, 2026
3 checks passed
@MaxGhenis MaxGhenis deleted the fix/consumption-weeks-rng branch April 17, 2026 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant