cp: feat(pinchbench): add the benchmark config for the full 147-task suite (2259) into r0.5.0 - #2261
Open
laszkiewiczp wants to merge 4 commits into
Open
cp: feat(pinchbench): add the benchmark config for the full 147-task suite (2259) into r0.5.0#2261laszkiewiczp wants to merge 4 commits into
feat(pinchbench): add the benchmark config for the full 147-task suite (2259) into r0.5.0#2261laszkiewiczp wants to merge 4 commits into
Conversation
PinchBench shipped as an agent server (responses_api_agents/pinchbench) but had no entry under benchmarks/, so it was neither discoverable via `gym list benchmarks` nor preparable via `gym eval prepare`. Add the Layer-3 benchmark definition: config.yaml chains to the agent config and narrows it to the benchmark dataset via `_inherit_from` (the tau2 shape, the existing precedent for a benchmark chaining to a responses_api_agents config), and prepare.py clones the skill at v2.0.0 -- the ref Dockerfile.benchmark bakes into the per-task image -- to write all 147 manifest tasks. The eight run-time knobs the agent config interpolates are declared with `oc.env:...,null` defaults so `gym list` and `gym eval prepare` resolve without them; a null still reaching the agent fails fast, since the fields are required `str` in PinchBenchAgentConfig. CLI overrides keep working because Hydra parses them against an empty base before config_paths loads and merges them last. Preparation fails loudly if the manifest is not 147 tasks, or if the regenerated rows do not byte-match the committed example.jsonl, so upstream drift and divergence from dataset_preprocess.py cannot pass silently. Also declare domain/description/value on the agent config: listing metadata has to live on the base node, since adding it to the inheriting benchmark node raises `Key 'domain' is not in struct`. The README table row is regenerated by the update-readme-table hook. Signed-off-by: plaszkiewicz <plaszkiewicz@nvidia.com> (cherry picked from commit 8efb861)
Contributor
Author
|
/ok to test 6272ec8 |
kajalj22
previously approved these changes
Jul 31, 2026
Signed-off-by: Adam Rajfer <arajfer@nvidia.com>
AdamRajfer
force-pushed
the
cherry-pick-2259-r0.5.0
branch
from
July 31, 2026 17:38
9184ce7 to
032ae51
Compare
Signed-off-by: Adam Rajfer <arajfer@nvidia.com>
… (#2275) The direct apptainer sandbox was not isolated from the host process tree in two independent ways. Both let a sandbox terminate the infrastructure running the evaluation, and both produce rollouts that are silently recorded as zero-score successes rather than retried. The sandbox ran with `["--cleanenv", "--no-home"]` — no `--pid`. Tasks execute shell commands, so a pattern-matching kill inside the sandbox matches processes on the host: ``` 18:58:03.133Z sandbox: pkill -f tar 18:58:03 vLLM: RayWorkerProc rank=[1] died unexpectedly, shutting down executor EngineCore encountered a fatal error ``` Not a one-off pattern. Observed in sandboxes: `pkill -f pip`, `pkill -f wget`, `pkill -f tar`, `pkill -f "venv/bin/pip"`, `killall find`. An earlier incident had `pkill -f pip` matching `--pipeline-parallel-size` on a server command line. Substring matching means patching individual flags is whack-a-mole; the namespace is the fix. `asyncio.create_subprocess_exec` was called without `start_new_session`, so the sandbox — and everything inside it, including the in-sandbox teardown — inherited the server's process group. The teardown signals a process group (`kill -TERM -- "-$PID"`), which terminated the server and all of its workers simultaneously, followed by a restart and a burst of connection failures while in-flight requests retried against a server still coming up. The sandbox providers under `nemo_gym/sandbox/providers/` already pass `start_new_session=True`; only this direct path did not. Affected rollouts carry `output_tokens: 0` with `status=success` and no failure class, so they are cached as legitimate zeros. In one full run this silently removed 18% of rollouts and depressed the reported score by roughly 0.11 versus the surviving-rollout mean. Re-ran the exact tasks that had killed a worker, with both fixes: zero vLLM errors, zero zero-token rollouts, sandboxes started normally. Nested unprivileged PID namespaces are permitted in the environments tested. Five tests in a new `tests/test_sandbox_isolation.py`, split across the two commits. Two are true regression guards — reverting the fixes fails them: - `test_direct_exec_isolates_the_pid_namespace_by_default` — drives `_run_in_apptainer_direct` with a mocked spawn, asserts `--pid` in argv - `test_direct_exec_launches_in_a_new_session` — same path, asserts `start_new_session=True` - `test_explicit_direct_exec_args_are_honoured` — an explicit `direct_exec_args` still wins, so existing configs are unaffected - `test_new_session_puts_the_child_in_its_own_process_group` — spawns a child, asserts it leads its own group - `test_without_new_session_the_child_shares_our_process_group` — demonstrates the pre-fix condition directly Kept in a separate module so `test_app.py` is untouched, which avoids churning the recorded line numbers in the detect-secrets baseline. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- (cherry picked from commit ec5a813) Signed-off-by: plaszkiewicz <plaszkiewicz@nvidia.com> Signed-off-by: Adam Rajfer <arajfer@nvidia.com> Co-authored-by: laszkiewiczp <85294967+laszkiewiczp@users.noreply.github.com>
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
3 participants
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.
Cherry pick of #2259 into
r0.5.0, opened in parallel with it rather than after merge — please review the two together.This PR contains the definition of Pinchbench needed for the release, otherwise Pinchbench does not launch from the new containers