mirror of
https://github.com/NVIDIA/Model-Optimizer.git
synced 2026-10-26 06:54:04 +08:00
Zhiyu
53ccec614f
### What does this PR do? Type of change: Bug fix Pins GDPVal evaluation commands to the currently validated `nemo-evaluator-launcher==0.2.6` through a dedicated `uvx` wrapper and updates the GDPVal skill, recipe, example, and reference documentation to use it. The reference also documents how to validate and adopt a future launcher release without mixing harness versions in baseline-versus-candidate comparisons. Older launchers generate a runtime re-export for `NEL_INVOCATION_ID` without first assigning the launcher-created invocation ID. The resulting Slurm script exits under `set -u` with `NEL_INVOCATION_ID: unbound variable` before the evaluation client starts. Launcher 0.2.6 writes the stable invocation ID into the script before the re-export, including for walltime-resume chains. ### Usage ```bash "$SKILL_DIR/scripts/nel-gdpval.sh" --version "$SKILL_DIR/scripts/nel-gdpval.sh" run --config gdpval.yaml --dry-run "$SKILL_DIR/scripts/nel-gdpval.sh" run --config gdpval.yaml ``` ### Testing - `bash -n plugins/modelopt/skills/evaluation/scripts/nel-gdpval.sh` - Verified the wrapper reports `nemo_evaluator_launcher: 0.2.6`. - Ran a real GDPVal dry-run and verified the generated script assigns `NEL_INVOCATION_ID="<invocation-id>"` before the runtime re-export. - Added a hermetic shell-wrapper test with a stub `uvx`; a conflicting ambient conflicting ambient version/spec values still invoke the validated 0.2.6 package. - `python -m pytest plugins/modelopt/skills/evaluation/ -o addopts="" -p no:cacheprovider -v` - Ran pre-commit on all six changed files. ### Before your PR is "*Ready for review*" Make sure you read and follow [Contributor guidelines](https://github.com/NVIDIA/Model-Optimizer/blob/main/CONTRIBUTING.md) and your commits are signed (`git commit -s -S`). Make sure you read and follow the [Security Best Practices](https://github.com/NVIDIA/Model-Optimizer/blob/main/SECURITY.md#security-coding-practices-for-contributors). - Is this change backward compatible?: ✅ - If you copied code from any other sources or added a new PIP dependency, did you follow guidance in `CONTRIBUTING.md`: N/A - Did you write any new necessary tests?: ✅ — added a hermetic wrapper test that stubs `uvx` and verifies the exact package spec despite a conflicting environment value. - Did you update [Changelog](https://github.com/NVIDIA/Model-Optimizer/blob/main/CHANGELOG.rst)?: N/A - Did you get Claude approval on this PR?: N/A ### Additional Information The fix preserves a stable GDPVal cache namespace across Slurm walltime resumes; using `SLURM_JOB_ID` would not. The failing run reused an existing base-environment `nel` 0.2.4 because the pre-existing prerequisite checked only whether `nel` was installed. The skill already identified 0.2.6 as the default launcher for Steps 1–9 and GDPVal. Version 0.2.6 is also the latest public `nemo-evaluator-launcher` release, so there is no newer public launcher version to validate. A dedicated `uvx` wrapper keeps the fix narrow and deterministic: - `nel-next.sh` manages the separate `nemo-evaluator` 0.4.x package, CLI, and schema; generalizing it would couple incompatible workflows. - A skill-level lock/project adds state for a single exact package invocation, while `uvx --from ...==0.2.6` already provides an isolated cached environment. - A version preflight would only reject the stale shared environment or require mutating it; the wrapper guarantees the known-good launcher on every GDPVal invocation without changing the existing `nel`. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added a GDPVal launcher wrapper that consistently uses the validated launcher version and forwards commands. - Added help output and checks for required runtime tooling. - **Documentation** - Clarified GDPVal launch, dry-run, reproducibility, resumed-run, and invocation ID guidance. - Documented version requirements and known sample limitations. - Updated evaluation examples and task instructions with the new launch procedure. - **Tests** - Added coverage verifying the pinned launcher version and argument forwarding. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Zhiyu Cheng <zhiyuc@nvidia.com>