[skill] evaluation: add GDPVal (NeMo Gym Stirrup agent) support - #2039
[skill] evaluation: add GDPVal (NeMo Gym Stirrup agent) support#2039cjluo-nv wants to merge 2 commits into
Conversation
GDPVal is an agentic AA benchmark: the Stirrup agent produces office/PDF deliverables in a per-task Apptainer code-exec sandbox, then a judge panel scores them. It runs on the 0.2.6 launcher as a `nemo_gym` task, but is standalone (one gym eval per config) and mechanically unlike the `aa/` nemo-skills tasks, so it gets its own branch in the skill. - recipes/tasks/aa_gym/gdpval.md: task recipe (standalone rule, scoring modes, canary, score extraction). - references/gym-gdpval.md: SIF sandbox, `_gym_prepare` machinery, deploy sizing, rubric-vs-comparison scoring, MLflow deliverables trap, failure modes, and the SIF<->Gym-version rebuild coupling. - recipes/examples/gym_gdpval/: self-contained SLURM + vLLM template plus the co-located `_gym_prepare.yaml` Hydra include. - scripts/gdpval-sif.sh: build-if-absent / reuse-if-present Apptainer SIF helper. Builds on the target cluster only (never copies across clusters), flock-guarded and atomic, driven by $GDPVAL_SIF_DIR. - SKILL.md / quantization-benchmarks.md: GDPVal is part of the AA suite but a different harness, so it is generated as a companion standalone config and never merged into the `aa/` task list. - env.example: TAVILY_API_KEY (agent web search) and GDPVAL_SIF_DIR. Validated end-to-end on an aarch64 GB300 cluster: deploy -> SIF build+exec -> gym head server -> 220 rollouts + deliverables -> judge scoring. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Chenjie Luo <chenjiel@nvidia.com>
📝 WalkthroughWalkthroughAdds a GDPVal SIF builder and a standalone NeMo Gym evaluation path. The change includes runtime preparation, sandbox and judge configuration, environment examples, staged execution guidance, and separate GDPVal handling in AA quantization recommendations. ChangesGDPVal evaluation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant nel
participant vLLM
participant nemo_gym
participant GDPValJudge
nel->>vLLM: Deploy reasoning-enabled policy
nel->>nemo_gym: Start pinned GDPVal task
nemo_gym->>vLLM: Collect policy rollouts
nemo_gym->>GDPValJudge: Submit outputs for scoring
GDPValJudge-->>nemo_gym: Return scores and deliverables
nemo_gym-->>nel: Export results to MLflow
Suggested reviewers: 🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2039 +/- ##
==========================================
- Coverage 75.81% 74.16% -1.65%
==========================================
Files 518 519 +1
Lines 58574 61696 +3122
==========================================
+ Hits 44406 45759 +1353
- Misses 14168 15937 +1769
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Warning
CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.
Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.agents/scripts/gdpval-sif.sh:
- Around line 45-46: Update GDPVAL_GYM_COMMIT and GDPVAL_SIF_NAME defaults in
.agents/scripts/gdpval-sif.sh to dd41196f620f2af99947d776cbe5da9439d2a08d and
python-3.13.gdpval.sif. In
.agents/skills/evaluation/recipes/examples/gym_gdpval/example_gym_gdpval.yaml:30-40,
no direct change is needed once the script defaults are corrected. Update the
documented SIF path to python-3.13.gdpval.sif in
.agents/skills/evaluation/SKILL.md:64-70,
.agents/skills/evaluation/recipes/tasks/aa_gym/gdpval.md:23-28, and
.agents/skills/evaluation/references/gym-gdpval.md:22-29.
- Around line 108-123: Update both Apptainer build invocations in the fakeroot
fallback flow to include the --force option, ensuring the commands using
"$APPTAINER_BIN" overwrite an existing "$tmp" without prompting.
In @.agents/skills/evaluation/recipes/examples/gym_gdpval/_gym_prepare.yaml:
- Around line 27-70: Update the final PYTHONPATH export in gym_prepare to
preserve any inherited $PYTHONPATH while prepending /opt/Gym and the main venv
site-packages. Use a shell conditional or equivalent that references $PYTHONPATH
without ${...} syntax, and keep the existing path ordering for Gym and its
dependencies.
In
@.agents/skills/evaluation/recipes/examples/gym_gdpval/example_gym_gdpval.yaml:
- Around line 166-175: Update the pre_cmd installation flow to detect the
container architecture and install an architecture-compatible Apptainer runtime,
avoiding the hardcoded amd64 1.4.2 package on aarch64 where no matching release
exists. Ensure installation failure is not masked by the current &&/fallback
behavior: verify Apptainer is available after installation and explicitly
terminate pre_cmd if it is unavailable, preserving the existing squashfuse setup
only after Apptainer succeeds.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 963909ee-d867-411c-a876-056e093a3662
📒 Files selected for processing (8)
.agents/scripts/gdpval-sif.sh.agents/skills/evaluation/SKILL.md.agents/skills/evaluation/recipes/env.example.agents/skills/evaluation/recipes/examples/gym_gdpval/_gym_prepare.yaml.agents/skills/evaluation/recipes/examples/gym_gdpval/example_gym_gdpval.yaml.agents/skills/evaluation/recipes/tasks/aa_gym/gdpval.md.agents/skills/evaluation/references/gym-gdpval.md.agents/skills/evaluation/references/quantization-benchmarks.md
| rm -f "$tmp" "$def_local" | ||
| # apptainer build cannot take a remote def URL as its source — fetch the def to a | ||
| # local file first, then build from it. | ||
| if command -v curl >/dev/null 2>&1; then curl -fsSL "$def_url" -o "$def_local" | ||
| else wget -qO "$def_local" "$def_url"; fi | ||
| [ -s "$def_local" ] || { rm -f "$def_local"; _die "failed to download def from $def_url"; } | ||
| # Prefer --fakeroot (needs an /etc/subuid entry for the build user); fall back to an | ||
| # unprivileged build where fakeroot is unavailable. | ||
| if "$APPTAINER_BIN" build --fakeroot "$tmp" "$def_local"; then | ||
| : | ||
| elif "$APPTAINER_BIN" build "$tmp" "$def_local"; then | ||
| _log "built without --fakeroot (unprivileged mode)" | ||
| else | ||
| rm -f "$tmp" "$def_local" | ||
| _die "apptainer build failed (see output above)." | ||
| fi |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
sed -n '80,135p' .agents/scripts/gdpval-sif.sh
printf '\n--- build command references ---\n'
rg -n --hidden -S 'apptainer|singularity|--fakeroot|--force|gdpval-sif' .agents .github README* 2>/dev/null | head -200
printf '\n--- tool availability and versions (without running repository code) ---\n'
command -v apptainer && apptainer --version || true
command -v singularity && singularity --version || trueRepository: NVIDIA/Model-Optimizer
Length of output: 10198
🌐 Web query:
Apptainer 1.4.2 build existing output file --force overwrite behavior documentation source
💡 Result:
In Apptainer 1.4.2, the --force (or -F) flag is used with the build command to overwrite an existing container image file if it already exists [1]. When you provide the --force option, Apptainer will automatically delete and overwrite the existing image file without displaying the standard interactive confirmation prompt that would otherwise appear [2][3]. Note that this behavior differs from the --update flag, which is specifically used when building into existing sandbox containers; the --update option allows you to skip the definition file's header and add or modify sections within that existing sandbox container rather than overwriting the entire image [4][5].
Citations:
- 1: https://apptainer.org/docs/user/1.4/cli/apptainer_build.html
- 2: https://apptainer.org/docs/user/latest/build_a_container.html
- 3: https://github.com/apptainer/apptainer-userdocs/blob/1.4/build_a_container.rst
- 4: https://apptainer.org/docs/user/main/build_a_container.html
- 5: https://github.com/apptainer/apptainer-userdocs/blob/main/build_a_container.rst
🌐 Web query:
SingularityCE build existing destination file --force overwrite behavior documentation source
💡 Result:
In SingularityCE, the --force (or -F) flag is used with the singularity build command to overwrite an existing container image file if it already exists [1][2]. When this flag is provided, SingularityCE will delete and replace the existing image without prompting the user for interactive confirmation [3][4]. If you attempt to build an image to a destination where a file already exists without using the --force flag, the command will typically fail or prompt for confirmation, depending on the context and version. For sandbox (directory-based) containers, there is also an --update option [5][6]. While --force is used for overwriting files entirely, --update allows you to build into an existing sandbox container by skipping the definition file's header and only building the sections specified in the definition file [5][6]. Note that --update is only valid for sandbox containers [5][6].
Citations:
- 1: https://docs.sylabs.io/guides/latest/user-guide/cli/singularity_build.html
- 2: https://docs.sylabs.io/guides/main/user-guide/cli/singularity_build.html
- 3: https://docs.sylabs.io/guides/4.3/user-guide/build_a_container.html
- 4: https://docs.sylabs.io/guides/main/user-guide/build_a_container.html
- 5: https://docs.sylabs.io/guides/latest/user-guide/build_a_container.html
- 6: https://docs.sylabs.io/guides/4.2/user-guide/build_a_container.html
🌐 Web query:
site:github.com/apptainer/apptainer build failed cleanup output image partial file build.go --force
💡 Result:
There is no native Apptainer or Singularity build command flag --force that is documented to specifically "cleanup output image" or resolve "partial file" errors in build.go [1][2][3]. Errors related to failed cleanups or partial files during an Apptainer/Singularity build are typically environmental or related to how the build process interacts with the host filesystem. If you are experiencing a build failure, consider the following standard troubleshooting steps: 1. Use a local filesystem for temporary directories: Ensure your APPTAINER_TMPDIR (or TMPDIR) is not set to an NFS-mounted location, as this frequently causes stale file handles and cleanup failures [4][5]. 2. Check directory permissions and flags: Ensure the temporary directory is fully writable and does not have restrictive sticky bits or group permission settings that might interfere with container build processes [5]. 3. Debug the failure: Use the --debug flag with your build command (e.g., apptainer --debug build...) to identify the specific stage where the cleanup or file operation fails [6][7][8]. 4. Consider clearing the build cache: If you suspect a corrupted cache, you can often clear it or disable it temporarily using the --disable-cache flag during your build [7]. If you encountered a specific error message involving "cleanup output image" or "partial file" in your logs, it may be a known issue linked to specific versions of Apptainer or the underlying Go container libraries [6][8][9]. Searching the Apptainer GitHub repository for the specific error string from your debug output is the most reliable way to find patches or workarounds relevant to your version [2][6][9].
Citations:
- 1: apptainer/apptainer@326f41e
- 2: Unprivileged encrypted image build shows timeout messages apptainer/apptainer#2529
- 3: Create a temporary directory at build time for build artifacts apptainer/apptainer#490
- 4: Test section during build process triggers stale NFS file handle apptainer/apptainer#2594
- 5: apptainer build fails if APPTAINER_TMPDIR points to directory with secondary group sticky bit writable apptainer/apptainer#3013
- 6: Frequent cleanup error message apptainer/apptainer#974
- 7: apptainer build gives "Process killed" apptainer/apptainer#2650
- 8: Apptainer build command fails if home directory contains an @ character apptainer/apptainer#2976
- 9: apptainer build error: 'link: unpriv.link: unpriv.wrap target: no such file or directory' apptainer/apptainer#2841
🌐 Web query:
site:github.com/sylabs/singularity source build failed cleanup destination file force
💡 Result:
The "source build failed" error related to cleanup and destination files typically arises in Singularity (now Apptainer) builds when the tool detects an existing file or directory at the target path and requires explicit permission to overwrite it [1][2]. The software prevents accidental data loss by checking if the destination already exists [1]. If it is a non-empty directory or an existing file, the build will fail or pause to request confirmation unless the --force (or -F) flag is used [1][3]. Key points regarding this behavior: 1. Flag usage: If you encounter an error stating that the target exists or cannot be overwritten, add the --force flag to your command (e.g., singularity build --force [options] [destination] [definition]) [1][3]. This tells the build process to proceed with deleting or overwriting the existing destination [1]. 2. Interactive prompts: In non-automated environments, the tool may prompt you to confirm the deletion of an existing build target [1]. In automated/non-interactive environments (where stdin is not a terminal), the build will fail immediately with a message instructing you to use --force if you intend to overwrite the target [1]. 3. Sandbox permission issues: If the build fails because it cannot clean up a previous sandbox (often due to root ownership), ensure you have the appropriate permissions (e.g., using sudo or --fakeroot) [4]. Simply using --force may not resolve errors caused by lack of filesystem-level write permissions [4]. 4. Build cleanup: Apptainer/Singularity is designed to clean up temporary build bundles automatically upon completion or failure [5][6]. If an error occurs during the build, the internal cleanup routine attempts to remove temporary root filesystems and directories unless the --no-cleanup flag was used [5][6]. If you are seeing a specific error like failed to create symlink or permission denied, this often indicates the build engine is struggling to modify an existing directory structure that was likely created during a previous run with different permissions [4][7]. In such cases, manually removing the destination directory with appropriate permissions (e.g., sudo rm -rf ) before re-running the build is the most reliable solution [4].
Citations:
- 1: https://github.com/sylabs/singularity/blob/v4.5.0/cmd/internal/cli/build.go
- 2: sylabs/singularity@30dc5c4
- 3: https://github.com/sylabs/singularity/blob/v4.5.0/cmd/internal/cli/singularity.go
- 4: delete sandbox sylabs/singularity#2938
- 5: https://github.com/sylabs/singularity/blob/master/internal/pkg/build/build.go
- 6: https://github.com/sylabs/singularity/blob/v4.5.0/internal/pkg/runtime/engine/singularity/cleanup_linux.go
- 7: Error while building from image: "failed to create symlink /image/rootfs/var/tmp" sylabs/singularity#3084
🏁 Script executed:
python3 - <<'PY'
import json, urllib.request
urls = {
"apptainer_build": "https://raw.githubusercontent.com/apptainer/apptainer/main/cmd/internal/cli/action/build.go",
"singularity_build": "https://raw.githubusercontent.com/sylabs/singularity/main/cmd/internal/cli/action/build.go",
}
for name, url in urls.items():
print(f"--- {name}: {url} ---")
try:
text = urllib.request.urlopen(url, timeout=20).read().decode()
except Exception as exc:
print(type(exc).__name__, exc)
continue
lines = text.splitlines()
for i, line in enumerate(lines, 1):
low = line.lower()
if any(term in low for term in ("force", "remove", "cleanup", "output", "destination")):
start, end = max(1, i-3), min(len(lines), i+5)
print(f"[{start}-{end}]")
print("\n".join(f"{j}: {lines[j-1]}" for j in range(start, end+1)))
PYRepository: NVIDIA/Model-Optimizer
Length of output: 683
🏁 Script executed:
python3 - <<'PY'
import re, urllib.request, ssl
urls = {
"apptainer-1.4.2-build": "https://raw.githubusercontent.com/apptainer/apptainer/v1.4.2/cmd/internal/cli/action/build.go",
"apptainer-1.4.2-build_internal": "https://raw.githubusercontent.com/apptainer/apptainer/v1.4.2/internal/pkg/build/build.go",
"singularity-4.5-build": "https://raw.githubusercontent.com/sylabs/singularity/v4.5.0/cmd/internal/cli/build.go",
"singularity-4.5-build_internal": "https://raw.githubusercontent.com/sylabs/singularity/v4.5.0/internal/pkg/build/build.go",
}
ctx = ssl._create_unverified_context()
for name, url in urls.items():
print(f"--- {name} ---")
try:
text = urllib.request.urlopen(url, context=ctx, timeout=20).read().decode()
except Exception as exc:
print(type(exc).__name__, exc)
continue
lines = text.splitlines()
hits = [i for i, line in enumerate(lines) if re.search(r'force|exist|remove|cleanup|output|destination|overwrite', line, re.I)]
shown = set()
for i in hits:
start, end = max(0, i-3), min(len(lines), i+5)
key = (start, end)
if key in shown:
continue
shown.add(key)
print(f"[{start+1}-{end}]")
print("\n".join(f"{j+1}: {lines[j]}" for j in range(start, end)))
PYRepository: NVIDIA/Model-Optimizer
Length of output: 20119
🏁 Script executed:
python3 - <<'PY'
import json, urllib.request, ssl
ctx = ssl._create_unverified_context()
for repo, ref in [
("apptainer/apptainer", "v1.4.2"),
("sylabs/singularity", "v4.5.0"),
]:
url = f"https://api.github.com/repos/{repo}/git/trees/{ref}?recursive=1"
print(f"--- {repo}@{ref} candidate paths ---")
try:
data = json.load(urllib.request.urlopen(url, context=ctx, timeout=30))
except Exception as exc:
print(type(exc).__name__, exc)
continue
for item in data.get("tree", []):
p = item.get("path", "")
if p.endswith(".go") and any(x in p.lower() for x in ("build", "assembler")):
print(p)
PYRepository: NVIDIA/Model-Optimizer
Length of output: 353
🏁 Script executed:
python3 - <<'PY'
import io, re, tarfile, urllib.request, ssl
ctx = ssl._create_unverified_context()
archives = [
("apptainer-1.4.2", "https://codeload.github.com/apptainer/apptainer/tar.gz/refs/tags/v1.4.2"),
("singularity-4.5.0", "https://codeload.github.com/sylabs/singularity/tar.gz/refs/tags/v4.5.0"),
]
patterns = ("build", "assembler", "sif")
for name, url in archives:
print(f"--- {name} ---")
try:
raw = urllib.request.urlopen(url, context=ctx, timeout=60).read()
tf = tarfile.open(fileobj=io.BytesIO(raw), mode="r:gz")
except Exception as exc:
print(type(exc).__name__, exc)
continue
members = [m for m in tf.getmembers()
if m.isfile() and m.name.endswith(".go")
and any(p in m.name.lower() for p in patterns)]
for m in members:
text = tf.extractfile(m).read().decode("utf-8", "replace")
if re.search(r"forceOverwrite|checkBuildTarget|SIFAssembler|os\.Create|os\.Remove|RemoveAll|Dest", text):
print(f"\n### {m.name}")
lines = text.splitlines()
hits = [i for i, line in enumerate(lines)
if re.search(r"forceOverwrite|checkBuildTarget|SIFAssembler|os\.Create|os\.Remove|RemoveAll|Dest|existing", line, re.I)]
shown = set()
for i in hits:
start, end = max(0, i-2), min(len(lines), i+4)
if (start, end) in shown:
continue
shown.add((start, end))
print(f"[{start+1}-{end}]")
print("\n".join(f"{j+1}: {lines[j]}" for j in range(start, end)))
PYRepository: NVIDIA/Model-Optimizer
Length of output: 50379
🏁 Script executed:
python3 - <<'PY'
import io, tarfile, urllib.request, ssl
ctx = ssl._create_unverified_context()
raw = urllib.request.urlopen(
"https://codeload.github.com/apptainer/apptainer/tar.gz/refs/tags/v1.4.2",
context=ctx, timeout=60).read()
tf = tarfile.open(fileobj=io.BytesIO(raw), mode="r:gz")
wanted = {
"cmd/internal/cli/build.go": [(444, 510)],
"internal/pkg/build/assemblers/sif.go": [(120, 155), (159, 310)],
}
prefix = "apptainer-1.4.2/"
for path, ranges in wanted.items():
member = tf.getmember(prefix + path)
lines = tf.extractfile(member).read().decode().splitlines()
print(f"--- {path} ---")
for start, end in ranges:
print(f"[{start}-{end}]")
print("\n".join(f"{i}: {lines[i-1]}" for i in range(start, min(end, len(lines))+1)))
PYRepository: NVIDIA/Model-Optimizer
Length of output: 9327
Pass --force to both build attempts
If the --fakeroot build leaves $tmp, Apptainer 1.4.2 and SingularityCE reject the fallback in non-interactive mode or prompt in interactive mode. Pass --force to both commands.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.agents/scripts/gdpval-sif.sh around lines 108 - 123, Update both Apptainer
build invocations in the fakeroot fallback flow to include the --force option,
ensuring the commands using "$APPTAINER_BIN" overwrite an existing "$tmp"
without prompting.
| gym_prepare: | ||
| # Common command preamble shared by every benchmark: activate the baked Gym | ||
| # venv, optionally checkout the install_on_the_fly pin (only when /opt/Gym is a | ||
| # git repo; baked images aren't), then repair the image's incomplete per-server | ||
| # venvs. The repair: (1) rewrites each component requirements.txt — drops the | ||
| # editable "-e nemo-gym[dev]" line (it forces a ray>=2.55.1 re-resolve vs the | ||
| # image's pinned ray 2.49.2, breaking venv-less servers like arena_judge) and | ||
| # ensures ray==2.49.2 + tqdm; (2) installs the fixed requirements into each | ||
| # baked (skeleton) sub-venv; (3) fronts the main venv on PYTHONPATH so nemo_gym | ||
| # + framework deps resolve for server processes. | ||
| # NOTE: avoid bash ${VAR} here — OmegaConf parses ${...}; $(...) / $r / $v are fine. | ||
| # NOTE: every step is guarded (|| true / -q grep) so it is safe under set -e. | ||
| prepare: |- | ||
| set -ex | ||
| cd /opt/Gym | ||
| export UV_CACHE_DIR=/opt/cache/uv | ||
| source .venv/bin/activate | ||
| # install_on_the_fly: checkout the pin when /opt/Gym is a git repo; some | ||
| # images bake Gym at a fixed version (not a git repo) — use it as-is. | ||
| if [ -d .git ]; then | ||
| git remote add oss_pin "{{config.params.extra.nemo_gym.install_on_the_fly.url}}" 2>/dev/null || true | ||
| git fetch oss_pin | ||
| git checkout "{{config.params.extra.nemo_gym.install_on_the_fly.commit}}" | ||
| echo "=== NeMo Gym commit ===" && git rev-parse HEAD | ||
| else | ||
| echo "=== /opt/Gym is not a git repo; using baked-in Gym version ===" | ||
| fi | ||
| for r in /opt/Gym/responses_api_models/*/requirements.txt \ | ||
| /opt/Gym/responses_api_agents/*/requirements.txt \ | ||
| /opt/Gym/resources_servers/*/requirements.txt; do | ||
| [ -f "$r" ] || continue | ||
| grep -vE '^[[:space:]]*-e ' "$r" > "$r.fixed" || true | ||
| grep -qiE '^ray([<>=[]|$)' "$r.fixed" 2>/dev/null || echo 'ray==2.49.2' >> "$r.fixed" | ||
| grep -qiE '^tqdm' "$r.fixed" 2>/dev/null || echo 'tqdm' >> "$r.fixed" | ||
| mv "$r.fixed" "$r" 2>/dev/null || true | ||
| done | ||
| for v in /opt/Gym/responses_api_models/*/.venv \ | ||
| /opt/Gym/responses_api_agents/*/.venv \ | ||
| /opt/Gym/resources_servers/*/.venv; do | ||
| [ -d "$v" ] || continue | ||
| d="$(dirname "$v")" | ||
| [ -f "$d/requirements.txt" ] && uv pip install --python "$v/bin/python" -q -r "$d/requirements.txt" || true | ||
| done | ||
| export PYTHONPATH="/opt/Gym:$(/opt/Gym/.venv/bin/python -c 'import site; print(site.getsitepackages()[0])')" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
file=".agents/skills/evaluation/recipes/examples/gym_gdpval/_gym_prepare.yaml"
printf '%s\n' '--- target file ---'
cat -n "$file" | sed -n '1,120p'
printf '%s\n' '--- PYTHONPATH references ---'
rg -n -C 3 'PYTHONPATH|gym_prepare|install_on_the_fly' .agents/skills/evaluation/recipes .agents/skills/evaluation 2>/dev/null | sed -n '1,240p'
printf '%s\n' '--- repository status ---'
git status --shortRepository: NVIDIA/Model-Optimizer
Length of output: 34741
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- all repository PYTHONPATH references ---'
rg -n -i 'PYTHONPATH' . | sed -n '1,240p'
printf '%s\n' '--- relevant Gym evaluation files ---'
fd -t f . .agents/skills/evaluation | rg '(^|/)(gdpval|gym|slurm|execution|container|def|env)' | sed -n '1,240p'
printf '%s\n' '--- environment/container command context ---'
rg -n -C 5 'GDPVAL_CONTAINER_PATH|apptainer|singularity|--env|env ' .agents/skills/evaluation .agents/scripts 2>/dev/null | sed -n '1,320p'Repository: NVIDIA/Model-Optimizer
Length of output: 33926
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- Gym GDPVal execution and environment configuration ---'
cat -n .agents/skills/evaluation/recipes/examples/gym_gdpval/example_gym_gdpval.yaml | sed -n '45,100p;185,270p'
printf '%s\n' '--- GDPVal reference container details ---'
cat -n .agents/skills/evaluation/references/gym-gdpval.md | sed -n '1,180p'
printf '%s\n' '--- SLURM environment guidance ---'
cat -n .agents/skills/evaluation/references/slurm.md | sed -n '1,180p'Repository: NVIDIA/Model-Optimizer
Length of output: 22790
🏁 Script executed:
#!/bin/bash
set -eu
cat -n .agents/skills/evaluation/recipes/examples/gym_gdpval/example_gym_gdpval.yaml | sed -n '95,190p'
printf '%s\n' '--- exact PYTHONPATH assignment semantics ---'
python3 - <<'PY'
import os
old = os.environ.get("PYTHONPATH")
gym = "/opt/Gym:/opt/Gym/.venv/lib/python3.13/site-packages"
os.environ["PYTHONPATH"] = gym
print({"inherited": old, "assigned": os.environ["PYTHONPATH"], "inherited_preserved": old is None or old in os.environ["PYTHONPATH"]})
PYRepository: NVIDIA/Model-Optimizer
Length of output: 5672
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
import subprocess
assignment = 'export PYTHONPATH="/opt/Gym:$(printf %s /opt/Gym/.venv/lib/python3.13/site-packages)"'
for inherited in ["/host/component", "/host/one:/host/two", None]:
env = {} if inherited is None else {"PYTHONPATH": inherited}
result = subprocess.run(
["bash", "-c", assignment + '; printf "%s" "$PYTHONPATH"'],
env=env,
text=True,
capture_output=True,
check=True,
)
print(f"inherited={inherited!r} -> assigned={result.stdout!r}")
proposed = 'export PYTHONPATH="/opt/Gym:$(printf %s /opt/Gym/.venv/lib/python3.13/site-packages)${PYTHONPATH:+:$PYTHONPATH}"'
for inherited in ["/host/component", None]:
env = {} if inherited is None else {"PYTHONPATH": inherited}
result = subprocess.run(
["bash", "-c", proposed + '; printf "%s" "$PYTHONPATH"'],
env=env,
text=True,
capture_output=True,
check=True,
)
print(f"proposed inherited={inherited!r} -> assigned={result.stdout!r}")
PY
printf '%s\n' '--- YAML interpolation warning near the assignment ---'
sed -n '31,40p;68,71p' .agents/skills/evaluation/recipes/examples/gym_gdpval/_gym_prepare.yamlRepository: NVIDIA/Model-Optimizer
Length of output: 1583
Preserve inherited PYTHONPATH without ${...} syntax.
Line 70 discards any PYTHONPATH inherited from the base image or launcher. Use a shell conditional with $PYTHONPATH; ${...} conflicts with OmegaConf interpolation.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.agents/skills/evaluation/recipes/examples/gym_gdpval/_gym_prepare.yaml
around lines 27 - 70, Update the final PYTHONPATH export in gym_prepare to
preserve any inherited $PYTHONPATH while prepending /opt/Gym and the main venv
site-packages. Use a shell conditional or equivalent that references $PYTHONPATH
without ${...} syntax, and keep the existing path ordering for Gym and its
dependencies.
Corrections found while bringing GDPVal up on an aarch64 GB300 cluster, in
both rubric and comparison mode.
Correctness:
- _gym_prepare: run the rollout command from a script written via a QUOTED
heredoc instead of `bash -c '...'`. Gym params legitimately contain single
quotes (comparison mode's ++multistage.stages='[{num_tasks: 45}, ...]' and
++...judge_panel='[{...}]'), which closed the wrapper early so Hydra got the
value split on spaces and died with "no viable alternative at input
'[{num_tasks:'". The quoted delimiter keeps $$ and $*_API_KEY unexpanded
until run time.
- example: never put '#' comments inside a folded (>-) params scalar. YAML
keeps them as literal text; the block folds to one line and the first '#'
comments out every override after it in the shell command.
- example: arch-aware pre_cmd (Ubuntu PPA) — the hardcoded amd64 apptainer
.deb cannot install on aarch64, which is most Blackwell/Grace clusters.
- gdpval-sif.sh: align the default SIF name and gym commit with the template,
so build and run no longer disagree by default.
Scoping (rubric default; comparison is a conversion):
- The template is now rubric-only and self-consistent: no dangling
reference_elo / reference_deliverables_dir, which CONFLICT with comparison
mode's reference_models map. Comparison needs a reference set, a newer gym
image, and its own overrides -- documented, not half-wired here.
Documented traps:
- install_on_the_fly.commit is INERT on images that bake Gym as a non-git
directory (the public nemo-gym image): the prepare step logs "/opt/Gym is
not a git repo" and the pin is ignored. Do not attribute behaviour changes
to it without "=== NeMo Gym commit ===" + a SHA in the log; a port-11000
head-server hang is a transient collision, not a version symptom.
- What NEL validates and what it does not: mount dirs fail loudly before
sbatch, but `test -d` cannot see a missing/misnamed SIF *file* (silent
unsandboxed fallback) and the container is never checked; --dry-run skips
remote validation entirely. Adds `gdpval-sif.sh --check` as the preflight.
- num_repeats depends on the flow: multistage comparison uses 1 (top-level
++num_repeats works), pre-multistage rubric/single-ref used 2.
- Prefer a site-provided SIF over building, when one exists: a self-built SIF
resolves its pip stack at build time and can drift from the sandbox a
published reference set was generated in.
Validated on aws-cmh (GB300, aarch64): deploy -> SIF build+sandboxed exec ->
gym head server -> rollouts + deliverables -> judge scoring, in rubric mode
(219/220 tasks, 0 judge failures) and in comparison mode against a 9-reference
set (verify_mode=comparison, per-reference win/loss/tie).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Chenjie Luo <chenjiel@nvidia.com>
There was a problem hiding this comment.
Warning
CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.
Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
@.agents/skills/evaluation/recipes/examples/gym_gdpval/example_gym_gdpval.yaml:
- Around line 175-183: Update the pre_cmd dependency-installation flow to remove
“|| true” from the apt-get install command so missing squashfuse or fuse3 causes
the setup to fail. Keep apptainer installation validation and add an SIF
execution canary on the target cluster to verify mount dependencies are usable,
not merely that the binary exists.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 3adfdf25-b2a6-4f9a-bc34-7f661848dc9f
📒 Files selected for processing (5)
.agents/scripts/gdpval-sif.sh.agents/skills/evaluation/recipes/examples/gym_gdpval/_gym_prepare.yaml.agents/skills/evaluation/recipes/examples/gym_gdpval/example_gym_gdpval.yaml.agents/skills/evaluation/recipes/tasks/aa_gym/gdpval.md.agents/skills/evaluation/references/gym-gdpval.md
🚧 Files skipped from review as they are similar to previous changes (1)
- .agents/skills/evaluation/recipes/tasks/aa_gym/gdpval.md
| pre_cmd: | | ||
| set -ex | ||
| export DEBIAN_FRONTEND=noninteractive | ||
| apt-get update -qq | ||
| apt-get install -y -qq software-properties-common squashfuse fuse3 ca-certificates || true | ||
| add-apt-repository -y ppa:apptainer/ppa | ||
| apt-get update -qq | ||
| apt-get install -y -qq apptainer | ||
| apptainer --version |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
file=".agents/skills/evaluation/recipes/examples/gym_gdpval/example_gym_gdpval.yaml"
printf '%s\n' '--- target lines ---'
sed -n '165,190p' "$file"
printf '%s\n' '--- related Apptainer and squashfuse references ---'
rg -n -i --glob '!node_modules' 'squashfuse|squashfuse_ll|apptainer|pre_cmd' .agents .github README.md 2>/dev/null | head -200
printf '%s\n' '--- repository status ---'
git diff --stat -- "$file"Repository: NVIDIA/Model-Optimizer
Length of output: 9483
🏁 Script executed:
#!/bin/bash
set -e
ref=".agents/skills/evaluation/references/gym-gdpval.md"
script=".agents/scripts/gdpval-sif.sh"
printf '%s\n' '--- Apptainer runtime documentation ---'
sed -n '20,75p' "$ref"
printf '%s\n' '--- canary and mount guidance ---'
sed -n '235,260p' "$ref"
printf '%s\n' '--- SIF preparation script ---'
sed -n '35,60p' "$script"
sed -n '100,155p' "$script"
printf '%s\n' '--- all relevant GDPVal configuration ---'
sed -n '145,190p' ".agents/skills/evaluation/recipes/examples/gym_gdpval/example_gym_gdpval.yaml"Repository: NVIDIA/Model-Optimizer
Length of output: 10989
🌐 Web query:
Apptainer official documentation non-suid SIF squashfuse fallback extraction mount
💡 Result:
In Apptainer, the mechanism used to run SIF (Singularity Image Format) containers depends on the installation mode and available system capabilities. Non-Setuid/Unprivileged Mode When Apptainer is run without setuid privileges (the default in modern versions), it relies on user namespaces to perform container operations [1][2]. By default, Apptainer attempts to mount the SIF container's SquashFS partition using squashfuse [3][4][2]. Squashfuse enables unprivileged users to mount SquashFS archives in user space via FUSE (Filesystem in Userspace) without requiring kernel-level privileges or the setuid-root fusermount helper [2][5][6]. Fallback and Extraction If squashfuse is unavailable or unsupported on the system, Apptainer may fall back to extracting the SIF image [7]. 1. Automatic Extraction: If Apptainer determines that mounting via squashfuse is not possible (e.g., due to missing dependencies or kernel restrictions), it can fall back to extracting the SIF image into a temporary sandbox directory [1][2][7]. 2. Forced Extraction: Users can explicitly trigger this behavior by using the --unsquash flag with commands like apptainer run, exec, or shell [8]. This converts the SIF file into a temporary sandbox directory on the fly before execution [1][8]. Key Considerations - Performance: While squashfuse provides a direct mount that avoids the overhead of file extraction, it may be slower than kernel-level SquashFS mounts [9]. Performance can be improved by using the squashfuse_ll (low-level) implementation [9][5]. - Security/Persistence: Unlike setuid mode, where encrypted containers are mounted directly via the kernel, non-setuid mode uses FUSE-based programs (like gocryptfs) to handle encryption/decryption, avoiding the need to decrypt contents to disk [3][4][6]. - Sandbox Limitation: When running in non-setuid mode, all containers are essentially run from sandbox directories [1]. When Apptainer extracts a SIF image to a sandbox, it prevents runtime verification of the image, potentially allowing external modifications to the container files while it is running [1]. Top Results: [1], [3], [2], [9], [6]
Citations:
- 1: https://apptainer.org/docs/admin/1.0/user_namespace.html
- 2: https://indico.jlab.org/event/459/papers/11669/files/364-CHEP23_Paper_ApptainerWithoutSuid.pdf
- 3: https://apptainer.org/docs/user/latest/security.html
- 4: https://apptainer.org/docs/user/main/security.html
- 5: https://deepwiki.com/vasi/squashfuse
- 6: https://www.epj-conferences.org/articles/epjconf/pdf/2024/05/epjconf_chep2024_07005.pdf
- 7: Fails to run unsquashfs apptainer/apptainer#760
- 8: https://apptainer.org/docs/user/latest/cli/apptainer_exec.html
- 9: squashfuse Performance apptainer/apptainer#665
Fail when SIF mount dependencies are unavailable.
At line 179, || true can hide a failed squashfuse or fuse3 installation. apptainer --version only checks that the binary exists. Apptainer can fall back to extraction, but this causes a silent performance degradation. Remove || true, install the dependencies successfully, and run an SIF execution canary on the target cluster.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
@.agents/skills/evaluation/recipes/examples/gym_gdpval/example_gym_gdpval.yaml
around lines 175 - 183, Update the pre_cmd dependency-installation flow to
remove “|| true” from the apt-get install command so missing squashfuse or fuse3
causes the setup to fail. Keep apptainer installation validation and add an SIF
execution canary on the target cluster to verify mount dependencies are usable,
not merely that the binary exists.
1 participant
What does this PR do?
Type of change: new feature (agent skill)
Adds GDPVal support to the
evaluationagent skill. GDPVal is an agentic AAbenchmark: the NeMo Gym "Stirrup" agent produces office/PDF deliverables inside a
per-task Apptainer code-exec sandbox, and a judge panel scores them. It runs on the
0.2.6 launcher as a
nemo_gymtask, but it is standalone (one gym eval perconfig) and mechanically unlike the
aa/nemo-skills tasks, so it gets its ownbranch in the skill rather than being merged into the
aa/task list.recipes/tasks/aa_gym/gdpval.md— task recipe: standalone rule, rubric-vs-comparisonscoring, canary, score extraction.
references/gym-gdpval.md— the machinery: Apptainer SIF sandbox, the_gym_preparevenv-repair / process-group-reap workaround, deployment sizing,scoring modes, the MLflow deliverables trap, canary failure modes, and the
SIF ↔ Gym-version rebuild coupling.
recipes/examples/gym_gdpval/— self-contained SLURM + vLLM template plus theco-located
_gym_prepare.yamlHydra include (it must travel with the config).scripts/gdpval-sif.sh— build-if-absent / reuse-if-present Apptainer SIF helper.Builds on the target cluster only (never copies across clusters), flock-guarded and
atomic, driven by
$GDPVAL_SIF_DIR.SKILL.md/references/quantization-benchmarks.md— GDPVal is part of the AAsuite but a different harness, so it is generated as a companion standalone config.
recipes/env.example—TAVILY_API_KEY(agent web search) andGDPVAL_SIF_DIR.Usage
Testing
Validated end-to-end on an aarch64 GB300 SLURM cluster with an NVFP4 MoE checkpoint:
deploy → SIF build + sandboxed exec → gym head server → 220 rollouts + deliverables →
judge scoring, producing a real rubric score with zero judge failures. Several traps
found during that run are now documented in the reference (silent unsandboxed
fallback, gym-commit/head-server hang, judge api-key value-vs-name, SIF ↔ Gym version
coupling,
limit_samplesnot limiting gym rollouts).Before your PR is "Ready for review"
CONTRIBUTING.md: N/AAdditional Information
Docs/skill-only change under
.agents/; nomodelopt/source is touched.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation