Repository navigation
Fix resume mixing evaluator versions in PAIBench motion smoothness (evaluator identity fingerprint) - #375
Open
JIN-strong wants to merge 5 commits into
Open
Fix resume mixing evaluator versions in PAIBench motion smoothness (evaluator identity fingerprint)#375JIN-strong wants to merge 5 commits into
JIN-strong wants to merge 5 commits into
Conversation
Part of NVIDIA#361. Adds pure helpers to fingerprint the effective scorer source (git HEAD + worktree state), checked-in patches, and the environment freeze, with soft degradation when git or freeze are absent. Signed-off-by: JIN-strong <[email protected]>
Resume now hard-fails when the effective scorer (git source, applied patches, environment freeze) differs from the manifest, so merged scores can no longer mix evaluator versions. Fixes NVIDIA#361. Signed-off-by: JIN-strong <[email protected]>
Verifies evaluator identity reports unavailable/missing and resume still works under the degraded baseline, and skips git-dependent tests where git is absent. Signed-off-by: JIN-strong <[email protected]>
Signed-off-by: JIN-strong <[email protected]>
- legacy RUNNER_VERSION=2 manifests now raise a dedicated message ('legacy manifest without evaluator identity') instead of a misleading 'restore the previous evaluator' error, since no identity was recorded and the evaluator itself may be unchanged; the evaluator-identity mismatch path is preserved for when the key exists but differs.
- hash_patches now folds filename and length into the digest so different patch sets cannot collide (e.g. {a:AB,b:C} vs {a:A,b:BC}).
- add a legacy-manifest rejection test and a collision-regression test.
- README: add git status diagnostic hint for unexpected resume failures.
Signed-off-by: JIN-strong <[email protected]>
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.
What
The PAIBench-G motion-smoothness runner (
run_motion_smoothness_sharded.py) recorded onlyevaluator_root(a path) in its resume manifest. After the scorer checkout, its compatibility patches, or the scoring environment change, resuming silently mixed scores computed by different evaluator versions into one aggregated result.This PR fingerprints the effective evaluator into the resume manifest and hard-fails resume when it changes:
build_evaluator_identity()computes an identity from the scorer git HEAD plus agit status --porcelainhash (captures in-place edits and untracked files), content hashes of the checked-in compatibility patches, and thescorer_environment.freeze.txthash.evaluator_identitykey andRUNNER_VERSIONbumps 2 → 3.prepare_manifest()hard-fails with an actionable message when the previous identity differs. Legacy (pre-evaluator_identity) manifests get a dedicated message pointing to a new output directory.status: "unavailable"with a warning, and a missing freeze or patches directory recordsstatus: "missing", so resume protection degrades to the previous path+mtime baseline instead of crashing.Why
Resuming a sharded run after a scorer upgrade could merge scores from the old and new evaluator versions without any signal, producing results that no single evaluator version had actually computed.
Fixes #361.
Validation
python -m pytest test_run_motion_smoothness_sharded.py -qinevaluation/cosmos3/generator/paibench_g→ 14 passed, covering fingerprint determinism, rejection of changed patches or a changed scorer source, legacy-manifest rejection, and the degradation paths. Git-dependent tests skip cleanly wheregitis absent.python -m py_compile run_motion_smoothness_sharded.py→ clean.grep -n "evaluator-root" run_with_cosmos_framework.ipynb→ only the existing--evaluator-root "$PAIBENCH_SCORER_DIR"invocation; no new CLI arguments are required (patches_dirdefaults from__file__).