diff --git a/plugins/modelopt/skills/evaluation/SKILL.md b/plugins/modelopt/skills/evaluation/SKILL.md index 07cb120bd0d..93a7ca46d58 100644 --- a/plugins/modelopt/skills/evaluation/SKILL.md +++ b/plugins/modelopt/skills/evaluation/SKILL.md @@ -82,7 +82,7 @@ tasks). If the user asks for GDPVal: ### Step 1 — Prerequisites -Run `nel --version`; if missing, instruct `pip install nemo-evaluator-launcher`. If user has an existing config, skip to Step 8 (optionally review for `???` and quantization flags first). +Run **`"$SKILL_DIR/scripts/nel-check.sh"`** — it asserts the `nel` on PATH is the validated launcher and, when it is not, prints the exact `pip install` command to fix it. Do **not** just check that `nel` exists: a base environment can already carry an older launcher (directly, or via `nemo-evaluator-launcher-internal`, which ships its own launcher version), and scoring on it silently makes the run non-comparable with runs on the validated one. Record the version line it prints with the scores (Step 9). See `references/launcher-version.md` for why it is pinned and how to bump it. If user has an existing config, skip to Step 8 (optionally review for `???` and quantization flags first). **Set up `.env` now (not Step 8).** The working `.env` lives at the **workspace root** — the directory you run `nel` from — matching `modelopttools:eval-config`'s convention; do **not** create it under the skill dir. (NEL does not discover `.env` by path: it reads secrets from the shell env via the `host:` prefix after you `source`, so the location is purely *which file you source* before `nel run`. Keeping the single `.env` at the workspace root avoids a stale duplicate under the symlinked, shared `.agents/` skill tree.) For judge-scored / user-sim tasks (HLE, AA-LCR, Tau2), seed it from the template if absent — the template ships under the skill dir, the working `.env` does not: `[ -f .env ] || cp "$SKILL_DIR/recipes/env.example" .env`. Then try `modelopttools:eval-config` (if available) to fill the judge `model_id`/`url` rows (user adds the secret key). Needed before Step 5, which substitutes those values into task `` placeholders. @@ -407,9 +407,14 @@ export DUMMY_API_KEY=dummy **Step 8.1 — Dry-run** (config validation): ```bash +"$SKILL_DIR/scripts/nel-check.sh" # re-assert the validated launcher before scoring nel run --config --dry-run ``` +Re-run the check here even if Step 1 already passed: the existing-config path skips +Step 1 entirely, and this is the last point before a run that will be scored and +compared. GDPVal instead goes through `nel-gdpval.sh`, which pins the launcher itself. + Fix unresolved `???`, bad Hydra overrides, missing env vars, invalid mounts, image issues, sbatch errors, obvious deployment errors before proceeding. > **Dry-run does NOT validate the image/vLLM version** (image pulled only at deploy). Confirm `image:` ≥ the exact model's `recipes.vllm.ai` minimum (Step 3) before submitting — too-old passes dry-run, then crashes mid-inference. @@ -450,6 +455,8 @@ Remove `limit_samples` overrides; keep canary-validated parallelism. If the cana Before pulling/reporting scores, validate the run. Read `references/run-validation.md` for NEL timeout/resume behavior, completed-run validation, diagnostics, and score harvesting. For a baseline that will be compared with a candidate, also perform its **External Baseline Sanity Check** before a success verdict, then hand the validated runs to `compare-results` for baseline-vs-candidate deltas. +**Report the launcher version with the scores** (`"$SKILL_DIR/scripts/nel-check.sh" --version`, or the line Step 1 printed). It is the harness half of any delta: a baseline and a candidate scored on different launchers are not comparable, and without the version recorded that is undetectable after the fact. See `references/launcher-version.md`. + --- Issues: · diff --git a/plugins/modelopt/skills/evaluation/references/gym-gdpval.md b/plugins/modelopt/skills/evaluation/references/gym-gdpval.md index 83adf10dd7f..fe6b5643761 100644 --- a/plugins/modelopt/skills/evaluation/references/gym-gdpval.md +++ b/plugins/modelopt/skills/evaluation/references/gym-gdpval.md @@ -33,23 +33,17 @@ launcher so a harness change does not become part of the measured model delta. ## Updating the launcher pin -When a newer `nemo-evaluator-launcher` release is available: - -1. Review its release notes for launcher schema, generated Slurm, resume, and export - changes. -2. Update `NEL_GDPVAL_VERSION` in `scripts/nel-gdpval.sh` and the expected spec in - `tests/test_nel_gdpval.py`. -3. Run the focused test and pre-commit checks. Verify `nel-gdpval.sh --version` - reports the candidate version. -4. Dry-run a known GDPVal config and confirm the literal `NEL_INVOCATION_ID` +The pin itself lives in `scripts/nel-validated-version.sh` (shared with the Step 1 +gate), and the general bump procedure is in `references/launcher-version.md`. +GDPVal adds two requirements to it, because this is the one config that forwards +`runtime:NEL_INVOCATION_ID`: + +1. Dry-run a known GDPVal config and confirm the literal `NEL_INVOCATION_ID` assignment still precedes its runtime re-export in every generated `run.sub`. -5. Launch with the candidate version and monitor the first 20–30 minutes for SIF +2. Launch with the candidate version and monitor the first 20–30 minutes for SIF sandbox startup and judge authentication. GDPVal ignores `limit_samples`, so there is no cheap reduced-sample canary. -Only then update the validated version used for scored runs. Do not mix launcher -versions within a baseline-versus-candidate comparison. - ## Where each piece runs | Component | Where | diff --git a/plugins/modelopt/skills/evaluation/references/launcher-version.md b/plugins/modelopt/skills/evaluation/references/launcher-version.md new file mode 100644 index 00000000000..7c1d6b1108f --- /dev/null +++ b/plugins/modelopt/skills/evaluation/references/launcher-version.md @@ -0,0 +1,66 @@ +# Launcher version — the validated `nemo-evaluator-launcher` pin + +Shared reference for the **0.2.x path** (Steps 1–9, including GDPVal). nel-next +(`nemo-evaluator` 0.4.x) is a different package and pins separately — see +`references/nel-next.md`. + +The validated version lives in one place, `scripts/nel-validated-version.sh`, and +is consumed by `scripts/nel-check.sh` (Step 1 gate) and `scripts/nel-gdpval.sh` +(GDPVal's hard pin). Bump that one file, not the call sites. + +## Why the version is pinned + +**Comparability.** This skill exists to produce baseline-vs-candidate deltas. +Running the two legs on different launchers folds a harness change into the +measured model delta — the same reasoning `nel-next.sh` already applies to its +0.4.x git SHA. Keep a comparison pair on one launcher, and record the version +with the scores. + +**A known failure, on GDPVal specifically.** Launchers older than the pin emit +`export NEL_INVOCATION_ID="${NEL_INVOCATION_ID}"` into the generated `run.sub` +without assigning it first, so the job exits with `NEL_INVOCATION_ID: unbound +variable` under `set -u` before the evaluation client starts. This fires only for +configs that forward `runtime:NEL_INVOCATION_ID`, which today is GDPVal alone — +so it is a GDPVal correctness requirement, while comparability applies to every +scored task. Details and the dry-run check: `references/gym-gdpval.md`. + +## Presence is not the same as version + +Step 1 used to accept any `nel` already on PATH. That is how a GDPVal run picked +up a stale 0.2.4 from the base environment and failed. Note that +`nemo-evaluator-launcher-internal` ships its **own** launcher version and can +supply an older `nel` even when it is itself newer — `nel --version` prints both +rows, and only the `nemo_evaluator_launcher` row governs generated-Slurm and +schema behavior: + +```text +nemo_evaluator_launcher: 0.2.4 <- this row gates +nemo_evaluator_launcher_internal: 0.3.174+20260609 +``` + +Run the gate rather than eyeballing it: + +```bash +"$SKILL_DIR/scripts/nel-check.sh" # exits non-zero with the pip command to fix it +``` + +`NEL_ALLOW_UNVALIDATED=1` downgrades a mismatch to a warning and marks the output +`(UNVALIDATED)` — dev/canary only, never for scored or compared runs. GDPVal has +no escape hatch: it runs through `nel-gdpval.sh`, which hard-pins the launcher. + +## Updating the pin + +When a newer `nemo-evaluator-launcher` release is available: + +1. Review its release notes for launcher schema, generated Slurm, resume, and + export changes. +2. Update `NEL_VALIDATED_VERSION` in `scripts/nel-validated-version.sh`, and the + expected version in `tests/test_nel_check.py` / `tests/test_nel_gdpval.py`. +3. Run the focused tests and pre-commit checks, and confirm `nel-check.sh + --version` and `nel-gdpval.sh --version` both report the candidate. +4. Re-validate on a real run before adopting it for scored work — for GDPVal, + follow the extra dry-run and canary steps in `references/gym-gdpval.md` + (`limit_samples` is inert there, so there is no cheap reduced-sample canary). +5. Do not mix launcher versions within a baseline-versus-candidate comparison. If + the baseline was scored on the old pin, either keep the candidate there too or + re-run the baseline on the new one. diff --git a/plugins/modelopt/skills/evaluation/references/run-validation.md b/plugins/modelopt/skills/evaluation/references/run-validation.md index 43dbd8116a5..b24f2c064c5 100644 --- a/plugins/modelopt/skills/evaluation/references/run-validation.md +++ b/plugins/modelopt/skills/evaluation/references/run-validation.md @@ -31,9 +31,15 @@ single-model run: 6. If reasoning traces are present, confirm they are parsed/stripped/ignored before scoring consistently. Check for parser errors, unmatched reasoning delimiters, `finish_reason: length`, reasoning text leaked into answers, answers stripped with the reasoning, or reasoning disabled when the config intended it to be active. Report the run-validation summary before any score: log scan status, sample -accounting, reasoning/answer parsing status, and any errors or warnings found. -If any validation item fails, either rerun/fix it or label the result as -incomplete or invalid. +accounting, reasoning/answer parsing status, the launcher version the run was +scored on, and any errors or warnings found. If any validation item fails, either +rerun/fix it or label the result as incomplete or invalid. + +Record the launcher version (`"$SKILL_DIR/scripts/nel-check.sh" --version`) +alongside the scores, and confirm a baseline and its candidate were scored on the +same one before handing them to `compare-results` — a launcher difference is a +harness change inside the delta, and it is undetectable after the fact if nobody +wrote the version down. See `references/launcher-version.md`. ## External Baseline Sanity Check diff --git a/plugins/modelopt/skills/evaluation/scripts/nel-check.sh b/plugins/modelopt/skills/evaluation/scripts/nel-check.sh new file mode 100755 index 00000000000..94353a4efa3 --- /dev/null +++ b/plugins/modelopt/skills/evaluation/scripts/nel-check.sh @@ -0,0 +1,88 @@ +#!/usr/bin/env bash +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# nel-check.sh — assert the `nel` on PATH is the validated launcher (Step 1). +# +# Presence is not enough: a base environment can already carry an older +# `nemo-evaluator-launcher` (directly, or via `nemo-evaluator-launcher-internal`, +# which ships its own launcher version). Running a baseline and a candidate on +# different launchers folds a harness change into the measured model delta, so +# this fails loudly instead of silently scoring on whatever is installed. +# +# Usage: +# nel-check.sh # assert PATH `nel` == validated version +# nel-check.sh --version # print the validated version +# nel-check.sh --spec # print the pip spec to install +# +# Set NEL_ALLOW_UNVALIDATED=1 to downgrade the mismatch to a warning (dev/canary +# only — never for scored runs). GDPVal has no such escape hatch: it goes through +# nel-gdpval.sh, which hard-pins the launcher. See references/launcher-version.md. + +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +# shellcheck source=./nel-validated-version.sh +source "$SCRIPT_DIR/nel-validated-version.sh" + +case "${1:-}" in + -h|--help) awk '/^# nel-check\.sh/{p=1} /^set /{p=0} p' "$0" | sed 's/^# \{0,1\}//'; exit 0 ;; + --version) echo "$NEL_VALIDATED_VERSION"; exit 0 ;; + --spec) echo "$NEL_VALIDATED_SPEC"; exit 0 ;; +esac + +_install_hint() { + echo " pip install '${NEL_VALIDATED_SPEC}'" >&2 + echo "Then re-run this check. If a stale launcher keeps winning, it is likely" >&2 + echo "pulled in by 'nemo-evaluator-launcher-internal' — uninstall or pin that too." >&2 +} + +command -v nel >/dev/null 2>&1 || { + echo "ERROR: 'nel' not found on PATH. Install the validated launcher:" >&2 + _install_hint + exit 1 +} + +# `nel --version` prints the version table on stdout and log lines on stderr. +# The table lists several packages; `nemo_evaluator_launcher` is the one that +# determines generated-Slurm and schema behavior. +version_table="$(nel --version 2>/dev/null || true)" +found="$(awk -F': ' '$1 == "nemo_evaluator_launcher" { print $2; exit }' <<<"$version_table")" + +if [[ -z "$found" ]]; then + echo "ERROR: could not read 'nemo_evaluator_launcher' from 'nel --version'." >&2 + echo "Got:" >&2 + echo "$version_table" >&2 + _install_hint + exit 1 +fi + +if [[ "$found" != "$NEL_VALIDATED_VERSION" ]]; then + if [[ "${NEL_ALLOW_UNVALIDATED:-}" == "1" ]]; then + echo "WARNING: nel ${found} is NOT the validated ${NEL_VALIDATED_VERSION}." >&2 + echo "WARNING: NEL_ALLOW_UNVALIDATED=1 — dev/canary only. Do not report these" >&2 + echo "WARNING: scores, and never compare them against a ${NEL_VALIDATED_VERSION} baseline." >&2 + echo "nemo_evaluator_launcher: ${found} (UNVALIDATED)" + exit 0 + fi + echo "ERROR: 'nel' on PATH is ${found}, but the validated launcher is ${NEL_VALIDATED_VERSION}." >&2 + echo "Scoring on a different launcher makes the run non-comparable with runs on" >&2 + echo "${NEL_VALIDATED_VERSION}. Install the validated launcher:" >&2 + _install_hint + exit 1 +fi + +# Record this line with the scores (Step 9) — it is the harness half of the delta. +echo "nemo_evaluator_launcher: ${found} (validated)" diff --git a/plugins/modelopt/skills/evaluation/scripts/nel-gdpval.sh b/plugins/modelopt/skills/evaluation/scripts/nel-gdpval.sh index 64d8fe2c3ee..37af7320be4 100755 --- a/plugins/modelopt/skills/evaluation/scripts/nel-gdpval.sh +++ b/plugins/modelopt/skills/evaluation/scripts/nel-gdpval.sh @@ -16,9 +16,14 @@ set -euo pipefail -# Exact known-good pin; follow references/gym-gdpval.md before bumping it. -readonly NEL_GDPVAL_VERSION="0.2.6" -readonly NEL_GDPVAL_SPEC="nemo-evaluator-launcher[all]==${NEL_GDPVAL_VERSION}" +# Exact known-good pin, taken from the shared constant (never from the +# environment, so a stale `.env` value cannot select a different launcher). +# Follow references/launcher-version.md before bumping it. +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +# shellcheck source=./nel-validated-version.sh +source "$SCRIPT_DIR/nel-validated-version.sh" +readonly NEL_GDPVAL_VERSION="$NEL_VALIDATED_VERSION" +readonly NEL_GDPVAL_SPEC="$NEL_VALIDATED_SPEC" case "${1:-}" in -h|--help) diff --git a/plugins/modelopt/skills/evaluation/scripts/nel-validated-version.sh b/plugins/modelopt/skills/evaluation/scripts/nel-validated-version.sh new file mode 100644 index 00000000000..fc0647561c8 --- /dev/null +++ b/plugins/modelopt/skills/evaluation/scripts/nel-validated-version.sh @@ -0,0 +1,26 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# Single source of truth for the validated `nemo-evaluator-launcher` (the 0.2.x +# path — Steps 1-9 and GDPVal). Sourced by `nel-check.sh` and `nel-gdpval.sh` so +# one edit bumps both; see references/launcher-version.md before changing it. +# +# Assigned unconditionally, never from the environment: a stale ambient value +# (e.g. sourced from `.env`) must not be able to select a different launcher. +# nel-next (`nemo-evaluator` 0.4.x) is a different package and pins separately in +# nel-next.sh. + +NEL_VALIDATED_VERSION="0.2.6" +NEL_VALIDATED_SPEC="nemo-evaluator-launcher[all]==${NEL_VALIDATED_VERSION}" diff --git a/plugins/modelopt/skills/evaluation/tests/test_nel_check.py b/plugins/modelopt/skills/evaluation/tests/test_nel_check.py new file mode 100644 index 00000000000..c41c9465ae1 --- /dev/null +++ b/plugins/modelopt/skills/evaluation/tests/test_nel_check.py @@ -0,0 +1,130 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import os +import subprocess +from pathlib import Path + +import pytest + +SCRIPTS = Path(__file__).parents[1] / "scripts" +CHECK = SCRIPTS / "nel-check.sh" +GDPVAL = SCRIPTS / "nel-gdpval.sh" +VERSION_FILE = SCRIPTS / "nel-validated-version.sh" + +VALIDATED = "0.2.6" + + +def _run(args, env=None, **kwargs): + return subprocess.run([str(CHECK), *args], capture_output=True, text=True, env=env, **kwargs) + + +def _env_with_stub_nel(tmp_path, version_table): + """Put a stub `nel` printing `version_table` on stdout (logs on stderr, as the real CLI does).""" + nel = tmp_path / "nel" + nel.write_text( + "#!/usr/bin/env bash\n" + 'echo "[I] Centralized logging configured" >&2\n' + f"cat <<'EOF'\n{version_table}\nEOF\n" + ) + nel.chmod(0o755) + env = os.environ.copy() + # Prepend so the stub wins over any launcher already installed in the base env. + env["PATH"] = f"{tmp_path}:{env['PATH']}" + env.pop("NEL_ALLOW_UNVALIDATED", None) + return env + + +def test_reports_validated_version(): + assert _run(["--version"]).stdout.strip() == VALIDATED + + +def test_reports_pip_spec(): + assert _run(["--spec"]).stdout.strip() == f"nemo-evaluator-launcher[all]=={VALIDATED}" + + +def test_accepts_validated_launcher(tmp_path): + env = _env_with_stub_nel(tmp_path, f"nemo_evaluator_launcher: {VALIDATED}") + result = _run([], env=env) + assert result.returncode == 0 + assert f"nemo_evaluator_launcher: {VALIDATED} (validated)" in result.stdout + + +def test_rejects_stale_launcher(tmp_path): + """The failure that motivated the pin: a base env already carrying an older launcher.""" + env = _env_with_stub_nel(tmp_path, "nemo_evaluator_launcher: 0.2.4") + result = _run([], env=env) + assert result.returncode == 1 + assert "0.2.4" in result.stderr + assert f"pip install 'nemo-evaluator-launcher[all]=={VALIDATED}'" in result.stderr + + +def test_ignores_internal_package_version(tmp_path): + """`nemo-evaluator-launcher-internal` prints its own row; only the launcher row gates.""" + env = _env_with_stub_nel( + tmp_path, + f"nemo_evaluator_launcher: {VALIDATED}\nnemo_evaluator_launcher_internal: 0.3.174+20260609", + ) + result = _run([], env=env) + assert result.returncode == 0, result.stderr + + +def test_rejects_stale_launcher_behind_internal_package(tmp_path): + """An internal package at 0.3.x must not mask a stale 0.2.4 launcher.""" + env = _env_with_stub_nel( + tmp_path, + "nemo_evaluator_launcher: 0.2.4\nnemo_evaluator_launcher_internal: 0.3.174+20260609", + ) + assert _run([], env=env).returncode == 1 + + +def test_missing_launcher_is_actionable(tmp_path): + env = os.environ.copy() + env["PATH"] = "/usr/bin:/bin" + result = _run([], env=env) + assert result.returncode == 1 + assert "not found on PATH" in result.stderr + assert f"pip install 'nemo-evaluator-launcher[all]=={VALIDATED}'" in result.stderr + + +def test_unparseable_version_output_fails_closed(tmp_path): + env = _env_with_stub_nel(tmp_path, "some unexpected output") + result = _run([], env=env) + assert result.returncode == 1 + assert "could not read" in result.stderr + + +def test_escape_hatch_warns_and_marks_result_unvalidated(tmp_path): + env = _env_with_stub_nel(tmp_path, "nemo_evaluator_launcher: 0.2.4") + env["NEL_ALLOW_UNVALIDATED"] = "1" + result = _run([], env=env) + assert result.returncode == 0 + assert "(UNVALIDATED)" in result.stdout + assert "dev/canary only" in result.stderr + + +@pytest.mark.parametrize("script", [CHECK, GDPVAL]) +def test_scripts_share_one_validated_version(script): + """Both entry points must resolve the same pin, so a bump cannot drift.""" + resolved = subprocess.run( + ["bash", "-c", f'source "{VERSION_FILE}"; echo "$NEL_VALIDATED_VERSION"'], + capture_output=True, + text=True, + check=True, + ).stdout.strip() + assert resolved == VALIDATED + assert VALIDATED not in script.read_text(), ( + f"{script.name} hard-codes the version; source nel-validated-version.sh instead" + )