fix: make calibration staleness detection content-based - #37
Merged
Merged
Conversation
`tools/calibrate --check` asked Snakemake whether a dry run would do anything, which is an mtime question. The five steps write into one directory that they all read from, so re-running any step marked its successors stale even when the artefacts came back byte-identical, and a `git checkout` marked everything stale. It never converged. A step is stale when re-running it would change its artefacts, which depends only on inputs from outside the artefact set. Each set now carries a `fingerprint.yaml` hashing exactly those: the external leaves of the step's Snakemake DAG, the rule files and scripts that DAG runs, the step config (YAML round-tripped, so comments do not count), and the artefacts the step wrote. The DAG is rebuilt on each check, so dependencies a rule change adds or drops are picked up immediately. `--record` re-stamps a set without solving, for code changes that provably cannot move the artefacts. Two correctness fixes fall out of building the DAG honestly: - The solve-time calibration artefacts were declared as inputs of `solve_model` but not of `calibrate_deviation_penalty`, whose Broyden iteration runs the same solve in-process. Both now share `calibration_artefact_inputs`. - `health_required` built a full deepcopy of the configuration for every configured scenario. On `gsa.yaml` (16384 generated samples) that was ~35 s of the ~43 s spent constructing any DAG; it is now ~4 s, which brings `--check` on the `gbd-anchored` set from 3m15s down to 32 s.
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.
Problem
tools/calibrate --checkasked Snakemake whether a dry run would do anything,which is an mtime question. The five calibration steps write into one directory
that they all read from, so re-running any step marked its successors stale even
when the artefacts came back byte-identical, and a
git checkoutmarkedeverything stale. It never converged, and the guidance in the docs had degraded
to "treat
[STALE]as worth investigating, don't chase it to all-green".It also had a false negative:
calibrate_deviation_penaltyruns its solvesin-process, so it never declared the calibration artefacts those solves read.
Approach
A step is stale when re-running it would change its artefacts, which depends
only on inputs from outside the artefact set. Each set now carries a
fingerprint.yamlhashing exactly those:manually downloaded data, the
build_modelpackage),edits do not register,
back is reported rather than silently trusted.
The DAG is rebuilt on each check through the Snakemake Python API, so a
dependency that a rule or code change adds or drops is picked up straight away.
Because the fingerprint ignores the artefact set's own contents, re-running one
step never marks its successors stale.
New
tools/calibrate --recordre-stamps a set without solving, for code changesthat provably cannot move the artefacts.
Fixes that fell out of building the DAG honestly
Solve-time calibration artefacts (feed corrections, exogenous feed and forage,
food-demand multipliers, the calibrated deviation penalty) were declared as
inputs of
solve_modelbut not ofcalibrate_deviation_penalty. Both nowshare
calibration_artefact_inputs.health_requiredbuilt a full deepcopy of the configuration for everyconfigured scenario. On
gsa.yaml(16384 generated samples) that was ~35 s ofthe ~43 s spent constructing any DAG -- so it slowed down every Snakemake
invocation against a generated ensemble, not just the check. Reading the
overrides directly brings Snakefile parsing to ~4 s at an identical 848-job
DAG.
Verification
Timings on this workstation,
gbd-anchoredset:--check3m15s -> 32 s;defaultset ~19 s.Behaviour matrix, all confirmed correct:
touchevery artefact + a curated inputcost.yamlslack_marginal_cost7.5 -> 7.6costcosttemp()downloadspixi run -e dev pytest715 passed;test-integrationpasses;ruffand theREUSE hook clean. New
tests/test_calibration_fingerprint.pycovers thecomparison logic and the YAML/directory hashing.