fix: break the calibration chain's dependency on its last step - #41
Merged
Merged
Conversation
The feed, food_waste, food_demand and cost steps all read `deviation_penalty.yaml`, which the stability step writes at the end of the chain. The chain is a strict forward pass, so those four were fit against whatever vintage happened to be on disk, and `tools/calibrate --check` reported them stale after every complete run. None of them needs the file. feed, food_waste and food_demand pin production to actuals via `use_actual_production` and the enforced baselines, so the deviation penalty has nothing to act on; they now disable it. The cost step drives production stability through hard bounds, and the solve resolves the `"calibrated"` sentinel only for an enabled L1 penalty -- the input declaration in `calibration_artefact_inputs` now mirrors that gate instead of keying off the sentinel alone, which drops the dependency there without a config change. The dependency was inert: regenerating both tracked sets against the current model reproduces every artefact byte-identically, and `--check` now settles all-green after a full chain. Also re-stamps both sets after #36 and #38. Neither moved the artefacts -- verified by a full re-run, not by inspection.
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.
The question this started from
Are calibration provenance and fingerprints still valid after #36 and #38?
Provenance: yes. #38 updated both stamps itself, and both sets match the
current config.
Artefacts: yes, and verified rather than assumed. #38 is 61 files and
-1430 lines, touching
build_model.py,build_model/biomass.py, the GBD prepscripts and
prepare_faostat_animal_production.py. Too large to call inert byreading, so I regenerated both tracked sets from scratch. Every artefact came
back byte-identical -- the calibrated L1 costs to the last digit:
Fingerprints: stale, and now refreshed by that re-run.
What the re-run exposed
Even immediately after a complete chain,
--checkreported the first foursteps stale:
deviation_penalty.yamlis written bystability, the last step, and readby the first four. That is a lower-triangle violation of the same kind as the
feed/food-waste one fixed earlier: the chain is a strict forward pass, so those
four were fit against whatever vintage happened to be on disk.
None of them actually needs the file:
feed,food_waste,food_demandpin production to actuals(
use_actual_productionplus the enforced baselines), so the deviationpenalty has nothing to act on. They now set
deviation_penalty.enabled: false, and the solve gates sentinel resolution on that flag.costdrives production stability through hard bounds. The solve resolvesthe
"calibrated"sentinel only when the penalty is enabled andpenalty_mode == "l1", butcalibration_artefact_inputsdeclared the inputwhenever the sentinel merely appeared. Tightening the declaration to mirror
the solve's own gate drops the dependency with no config change.
The subtle part: a step does not have to enable a successor's calibration to
depend on it. Inheriting
l1_cost: "calibrated"fromdefault.yamlis enough.Noted in the calibration skill for whoever adds the next calibrated key.
Verification
changes, so the fix is inert too.
tools/calibrate --checkis all-green on both sets with matchingprovenance -- the first time the chain has settled.
pixi run -e dev pytest727 passed; ruff, snakefmt and REUSE hooks clean.Supersedes #39, which re-stamped against pre-#38 code.