Skip to content

Fix silent weight-save failure in non-verbose calibrate branch#348

Merged
MaxGhenis merged 2 commits intomainfrom
fix/calibrate-nonverbose-save
Apr 17, 2026
Merged

Fix silent weight-save failure in non-verbose calibrate branch#348
MaxGhenis merged 2 commits intomainfrom
fix/calibrate-nonverbose-save

Conversation

@MaxGhenis
Copy link
Copy Markdown
Contributor

Summary

  • calibrate_local_areas had its if epoch % 10 == 0 save block indented outside the for epoch in range(epochs): loop in the non-verbose branch of policyengine_uk_data/utils/calibrate.py.
  • That block therefore ran once after the loop with epoch = epochs - 1. For the default epochs=512 that is epoch=511, and 511 % 10 != 0, so the weights h5 file was silently never written on local runs.
  • CI only exercises the verbose branch (where the save block is correctly inside the loop), so this bug never surfaced there.

This PR:

  1. Re-indents the save block inside the non-verbose loop so it behaves identically to the verbose branch.
  2. Adds policyengine_uk_data/tests/test_calibrate_save.py with a regression test that drives calibrate_local_areas with tiny toy inputs and epochs=5 (so epoch=4 % 10 != 0 — a value that fails on the broken code but passes with the fix).

No changes to call sites or public API.

Test plan

  • uv run pytest policyengine_uk_data/tests/test_calibrate_save.py -x passes with the fix.
  • Reverting the calibrate.py change and rerunning the test reliably fails with "calibrate_local_areas did not write the weight file".

Reported as finding U1 in an external bug hunt on policyengine-uk-data.

@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 772028a into main Apr 17, 2026
3 checks passed
@MaxGhenis MaxGhenis deleted the fix/calibrate-nonverbose-save 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