Repository navigation
Conversation
Signed-off-by: Anish Mahishi <amahishi@nvidia.com>
Signed-off-by: Anish Mahishi <amahishi@nvidia.com>
…fast on stranded ones A legacy_agent environment server only relays /run, so its environment participant never holds the episode; the Simple Agent's legacy session does. The commit summary counted only environment exports as owned, so every in-flight Workplace episode looked unexported and the drain waited for replies that cannot arrive while Gym is prepared. Every periodic checkpoint timed out. When no environment participant exported an episode, a legacy agent session (a record with an episode step boundary) now owns it. The adapter reads the agent's committed records through Gym's digest-checked store. A parked agent session that nobody owns fails the checkpoint immediately instead of waiting out the drain deadline. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Anish Mahishi <amahishi@nvidia.com>
…oint refusals An attempt was marked DISPATCHED before it waited at the dispatch gate, so a checkpoint could count an attempt that cannot reach Gym until the checkpoint releases the gate; its drain could only time out, fatally at a trainer checkpoint. A /run that reached Gym after prepare closed admission was refused with 409 checkpoint_parked, charged as a data failure, and stayed DISPATCHED while it retried behind the closed gate. The ledger now marks an attempt DISPATCHED inside the gate admission, in the same mutation as the Ray submission. NemoGym takes /run failures as Gym sidecar rows so a checkpoint_parked refusal names its row without ending the stream. The refused attempt returns to RESERVED with its identity and is re-sent once admission reopens, spending no row attempt and no data budget. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Anish Mahishi <amahishi@nvidia.com>
Two-process crash/restore test for a Workplace Assistant turn. Phase one publishes a bootstrap rollout snapshot that holds one turn after its calendar mutation, then is killed; phase two restores it and must dispatch that episode once, as Gym attempt + 1 on its original replica, and train every step exactly once with no invalid or poisoned rows. The turn is captured deterministically: the Gym checkpoint test agent holds the first post-mutation boundary until a real checkpoint parks it, and the rows force a single calendar_create_event whose sentinel the selector requires exactly once in the saved Workplace state. The selector copies the chosen bootstrap snapshot before step 1's trainer checkpoint prunes it, and phase two drops every trainer checkpoint so the restore starts from that anchor. The hook records dispatches once the ledger marks them, plus any checkpoint refusals, and patches run_rollout so RolloutManager's NeMo-Gym type checks still hold. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Anish Mahishi <amahishi@nvidia.com>
…kpoint Abandoning or discarding a group drops its attempts from the recovery ledger, but an episode Gym already admitted keeps running. Gym exports it at the next commit, outside the candidate set, and every checkpoint fails until the episode finishes on its own. The ledger now records each dropped attempt Gym may still hold (dispatched, or reserved with restored Gym state), in memory only. With dispatch closed and before preparing Gym, while Gym is still idle (it refuses a retire once a checkpoint is open), the controller retires them and forgets only what it retired, so a drop recorded meanwhile waits for the next checkpoint. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Anish Mahishi <amahishi@nvidia.com>
…lacing them Gym runs every row of a /run as its own task, and a stream that ends early cancels none of them. The sibling retry re-sent the missing rows under the same rollout ID while the old copies were still running in Gym, and a prompt-group retry left the old episodes for the next checkpoint to export. When a stream ends without some rows, the rows that never came back (and that Gym did not refuse at admission) are abandoned in the recovery ledger and queued for retirement, then retired immediately under dispatch admission, so no checkpoint is active and Gym accepts an idle retire. A failed retire is not fatal: the episodes stay queued and the next checkpoint retires them before it commits. A sibling re-send now runs as a fresh attempt under a new rollout ID; attempts that never reached Gym keep their identity. Prompt-group recovery retires and re-raises so the existing cohort retry takes over. The streamed-completion identity check reads the current gate ID from the ledger, and the functional-test hook resolves its group by ID and forwards the new recorder hooks. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Anish Mahishi <amahishi@nvidia.com>
…ing the run Three ways a Gym checkpoint could take the run down with it: - Control calls had no RL-side timeout. Gym enforces deadline_ts only once it runs a call, so an actor with a stuck event loop, or a call Ray never scheduled, left the coordinator waiting forever with dispatch admission closed. Each call is now bounded by its own deadline plus a short reply grace; an unanswered instance is reported as a TimeoutError and its Ray call is cancelled. Giving up is safe: Gym's lease reopens a parked deployment if the resume is lost too. - The periodic rollout checkpoint treated every Gym failure as a broken invariant and ended training. A checkpoint that could not park, went unanswered, hit a dead actor, or failed in Gym's control plane changed nothing RL holds, so it is now retried under a gym_unavailable reason, still bounded by max_consecutive_failures. Ownership, topology and reply mismatches still abort. Gym's CoordinationError cannot be pickled and reached RL as an untyped UnserializableException, so the adapter raises a picklable GymCheckpointUnavailable in its place for live checkpoint calls. - Rollout deadlines kept ticking while Gym held rollouts parked, so a long checkpoint could time out healthy rollouts as soon as it resumed. The request-deadline registry now tracks named holders (ported from #4266), and a Gym checkpoint holds its own pause from closing admission until Gym resumes, without releasing a colocated engine's pause. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Anish Mahishi <amahishi@nvidia.com>
…the checkpoint gym_checkpoint.json recorded actor instances, episodes and staging keys, but not the Gym participants whose state each instance committed. Gym restores each live participant from its own state, so a participant missing from the restored deployment had its state silently dropped, and one the checkpoint never saw started empty, while the restore succeeded. The manifest now records each instance's participants: server name, kind, state directory and records digest, taken from the commit replies the adapter already validates. Before Gym installs anything, the adapter requires the live deployment to match exactly and each participant's state on disk to carry the committed digest. A mismatch leaves Gym untouched. A manifest without participant topology is rejected on load. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Anish Mahishi <amahishi@nvidia.com>
…y rollout After a Gym commit, the checkpoint waits for the few episodes Gym did not export because their reply is already on the wire. It did so by rebuilding the full Gym inventory from the ledger every 50 ms. That scan is synchronous and scales with every live rollout -- about 0.2 s per pass at 200k rollouts -- so it blocked the event loop that delivers those very replies, while every rollout stayed parked behind the checkpoint. The drain now checks only the pending episodes, through a ledger lookup that applies the same ownership rule as the inventory, and sleeps until the next data-plane mutation finishes instead of polling. Every ledger change runs inside a barrier mutation, so that wakeup cannot miss one. The barrier wakes these waiters on their own condition, leaving mutations queued behind a checkpoint undisturbed. The drain's timeout and stranded-session errors now report a count and a short sample rather than every episode identity. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Anish Mahishi <amahishi@nvidia.com>
|
Auto-sync is disabled for ready for review pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
github-actions
Bot
added
the
Documentation
❌ Submodule Fast-Forward Check FailedCheck based on commit: 91b87e6 (PR #4544 from ❌ Submodules that need attention:Gym: ❌ Commits have DIVERGED from a common ancestor Please ensure all submodule commits are fast-forwards of the amahishi/ckpt-v2-gym-integration branch before merging. |
Contributor
|
LGTM! can we point this to #4500 instead, regenerate the uv lock and there's also some ruff cache committed in gym commit? |
NeMo Gym standardized the names of its partial-rollout checkpoint contract. This pins the Gym checkpoint stack that uses them, and renames NeMo RL's own names for the same concepts so both sides read alike. The Gym pin moves to ananthsub/partial-ckpt-nemo-rl-pin: the Gym stack on current main, through the Workplace Assistant port, plus the checkpoint test agent this stack's recovery tests use. The pin is a fast-forward of the Gym commit pinned before. uv.lock is regenerated with uv 0.11.28 (docker/Dockerfile's UV_VERSION) for the new Gym dependencies: mlflow and wandb moved behind nemo-gym extras, and jsonschema is new. It also carries the two Gym fixes this stack depended on: restored calls are verified against the capture key they were staged under, and the model commit reply lists staged keys per episode. Renamed to match Gym: - Gym refuses work while admission is closed with the code admission_closed (it was checkpoint_parked). GymCheckpointParked is now GymAdmissionClosed, and the rollout metric rollout/gym_checkpoint_parked_rows_total is now rollout/gym_admission_closed_rows_total, with the stats field and the dispatch locals renamed to match. - Retire stops an attempt's work, then frees its state; Gym keeps no lasting fence. GymCheckpointCoordinator.discard_restored is now retire_restored, and the restore cleanup operation reports itself as restore_retire. - The commit summary's episode_keys hold capture keys, so the field is now capture_keys. - The single-controller guide calls a restored attempt no replacement has claimed yet "unclaimed", and names RL's own /run dispatch admission gate. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>
ananthsub
force-pushed
the
ananthsub/ckpt-v2-gym-integration-names
branch
from
October 6, 2026 22:56
91b87e6 to
2f34523
Compare
Contributor
Author
|
Thanks, all three are addressed.
The submodule check failed because the Gym stack was rebased and rewritten, so the new pin was not a descendant of |
macandro96
force-pushed
the
amahishi/ckpt-v2-gym-integration
branch
2 times, most recently
from
October 11, 2026 22:01
160e8ec to
cf124ff
Compare
macandro96
force-pushed
the
amahishi/ckpt-v2-gym-integration
branch
4 times, most recently
from
October 19, 2026 20:42
29dad27 to
e36be15
Compare
4 tasks done
macandro96
force-pushed
the
amahishi/ckpt-v2-gym-integration
branch
from
October 22, 2026 15:26
e36be15 to
1a4d0d3
Compare
macandro96
force-pushed
the
amahishi/ckpt-v2-gym-integration
branch
5 times, most recently
from
October 28, 2026 14:58
0714200 to
3f24e0a
Compare
This branch has not been deployed
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
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.
What changed and why
Stacked on #4549 (the same renames for #4500's branch,
amahishi/gym-turn-recovery-pr1); its commit is applied here first. Review asked to point this change at #4500, so that PR now carries it, and this one carries what #4501 adds on top.#4501's own commits use the same names in two more places:
rollout_manager.py, where the cherry-pick conflicted and was resolved by keeping this branch's version of the file with the same renames applied, and one coordinator test that builds the commit summary withcapture_keys=().The Gym pin,
uv.lock, and every rename are as described in #4549.Compatibility
As in #4549.
Validation
Validation, in a NeMo RL environment with the pinned Gym checked out in
3rdparty/Gym-workspace/Gym, and Ray started fresh (RAY_ADDRESS=local):uvx --from uv==0.11.28 uv lock --check, and the same with--directory docker/dynamo: passed.pytest -m "nemo_gym or not nemo_gym": passed. Tests that also fail on the base branch in this environment fail the same way before and after this PR: they need vLLM modules ormegatron, which the environment lacks.pyrefly check: no new errors. Thepyrefly-typecheckpre-commit hook was skipped at commit time, because it builds a full environment that needs every submodule; the same check ran directly instead.ruffandruff formatthrough the repository's pre-commit hooks: passed.Signed-off-byline.