Skip to content

feat: post-train environment - #3494

Open
cmunley1 wants to merge 6 commits into
mainfrom
cmunley1/rlenv
Open

cmunley1 wants to merge 6 commits into
mainfrom
cmunley1/rlenv

Conversation

@cmunley1

Copy link
Copy Markdown
Contributor

What does this PR do?

Policy runs in claude code to edit NemoRL and Gym, then a 1hr GRPO training run is ran in a GPU sandbox on 1 H100. then AIME25 is evaluated as the reward

Validation

Rollouts

Compatibility and benchmark impact

Checklist

  • I have read the contributing guidelines.
  • The change is focused; unrelated "drive-by" edits are tracked as separate issues/PRs.
  • Tests added or updated and pass locally, or N/A for docs-only / non-code changes (so CI unit/server checks pass when applicable).
  • Documentation added or updated, or N/A with justification.
  • Pre-commit checks pass locally (pre-commit run --all-files) (so CI lint/format checks pass).
  • New source files include the required Apache-2.0 SPDX header.
  • All commits have DCO sign-off (git commit -s) (so the DCO check passes).

Signed-off-by: Christian Munley <cmunley@nvidia.com>
Signed-off-by: Christian Munley <cmunley@nvidia.com>
Signed-off-by: Christian Munley <cmunley@nvidia.com>
Signed-off-by: Christian Munley <cmunley@nvidia.com>
@copy-pr-bot

copy-pr-bot Bot commented Sep 17, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

Signed-off-by: Christian Munley <cmunley@nvidia.com>
@cmunley1
cmunley1 marked this pull request as ready for review September 17, 2026 16:55
Signed-off-by: Christian Munley <cmunley@nvidia.com>
@cmunley1

Copy link
Copy Markdown
Contributor Author

/ok to test a4c854d

@cmunley1
cmunley1 requested a review from a team as a code owner September 17, 2026 17:06
@cmunley1

Copy link
Copy Markdown
Contributor Author

/claude review

@claude

claude Bot commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

SHIP WITH CARE — new nemorl_env training environment + focused claude_code_agent enhancement (cwd, per-arch Node install, npm cache). No blocker found.

The reward-integrity surface is the right thing to scrutinize hardest here, and it's guarded well:

  • Held-out AIME25 evaluation runs in a fresh, unpatched GPU sandbox (evaluate.py is dropped from the training spec; launch_inner.py/recipe.yaml dropped from the eval spec) — test_source_training_is_separate_from_trusted_evaluation asserts the file split and that the author patch never reaches the evaluator.
  • Scoring fails loud rather than silently returning a bogus reward: evaluate.py rejects non-binary rewards, a wrong answer count (not exactly 8×30=240), duplicate/failed rollouts, and a non-finite checkpoint; _evaluate raises on any missing NEMORL_ENV_RESULT/NEMORL_ENV_TRAIN_RESULT record or non-zero exit. test_failed_run_returns_http_error_not_zero_reward confirms a verifier failure surfaces as HTTP 500, not reward=0.
  • Async/operability: subprocess output decoded with errors="replace"; concurrency bounded via Semaphore; checkpoint transfers serialized through _transfer_sem (test_checkpoint_transfers_are_serialized_and_release_after_errors); external asyncio.timeout tears the sandbox down even if the inner trainer ignores its deadline. No httpx in async paths, no ray.get(); the author worker serves the policy model through the global aiohttp client.
  • Training-token capture fails closed (model_post_init raises if enabled) — appropriate, since this env is eval-only.

NOTEs (author's call, non-blocking):

  • _author builds the job with {key: model[key] for key in ("base_url","api_key","model","uses_reasoning_parser")} (app.py:112). A policy_model config missing any of those keys raises a bare KeyError with no context. Minor: a clearer message would help operators who point this at a non-vllm_model server.
  • run.sh retries clone/submodule/uv sync up to 5× with fixed 5s sleeps; on a persistently unreachable mirror the setup step burns its full window before failing. Acceptable given the 7200s setup budget, but worth being aware of.

Test coverage is genuinely strong (AST-sliced execution of the inner runner/evaluator, real safetensors round-trip against a tiny Qwen, secret-leakage assertions on sandbox specs). Per CLAUDE.md, green tests aren't sufficient for a behavior-changing env — confirm the PR body records a real smoke rollout with a model before marking ready.

@yaoyu-33 yaoyu-33 added area:environment Individual environments, benchmarks, verifiers, and environment-specific resources servers complexity:high Cross-component or shared-contract change with a large review surface feature New capabilities, enhancements, or enablement work needs-review PR is ready for code review and waiting on a reviewer labels Sep 17, 2026
@github-actions github-actions Bot added the sla:triage-overdue Review assignment is over the one-business-day SLA label Sep 18, 2026

This branch was previously deployed

1 inactive deployment
public — a4c854de Deployed Sep 17, 2026 by copy-pr-bot[bot] via release / finalize / notify #3119
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:environment Individual environments, benchmarks, verifiers, and environment-specific resources servers complexity:high Cross-component or shared-contract change with a large review surface feature New capabilities, enhancements, or enablement work needs-review PR is ready for code review and waiting on a reviewer sla:triage-overdue Review assignment is over the one-business-day SLA

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants