Repository navigation
feat(token-capture): carry media through Megatron capture rollouts - #4332
Merged
terrykong merged 24 commits intoOct 14, 2026
Merged
Conversation
lauradang
force-pushed
the
laurad/pr4129-multimodal-capture
branch
from
September 29, 2026 23:25
7bf8f11 to
f5d3810
Compare
Contributor
Author
|
/ok to test a9332d4 |
Token capture on the Megatron Inference backend was text-only: receipt rows carried no pixel_values, so a VLM capture run trained image-blind and Gym refused Megatron capture requests with image or video parts. This adds media capture on the Megatron backend on top of the shared vLLM media contract (TQ media columns, validate_media_tensors, batched fetch_media, and the finalizer). - Expanded-prefix splicing: TQMegatronPromptPreparer splices the exact expanded staged chain into the compact render, checks the endpoint's media count against what the chain staged, and answers with the expanded prefix length so the engine expands only the media placeholders after it. - EOS sets: splice_prefix_tokens accepts a set of EOS ids, counts any member as a turn boundary, and keeps the EOS the model emitted. - Media as per-call deltas: the preparer records how many media items the parent chain already staged; the stager slices the engine's packed tensors at that boundary and stages the rest on the staging key. - Finalizer: RolloutReassembler validates each call's media columns against its staged geometry, concatenates deltas in chain order, and publishes packed patches as pixel_values with imgs_sizes/num_frames. - Setup: registers media columns for multimodal runs on either backend, requires megatron-core media payload fields for multimodal Megatron capture, and rejects grpo.deduplicate_multimodal_data. - Sampling defaults: the Megatron worker passes the policy's temperature/top_p/top_k as HTTP server defaults so omitted request fields are not filled from the model's generation_config.json. Documents multimodal rollouts on Megatron Inference in the token-capture ledger design doc. Signed-off-by: Laura Dang <laurad@nvidia.com> Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
lauradang
force-pushed
the
laurad/pr4129-multimodal-capture
branch
from
September 30, 2026 07:04
62f30b1 to
2ed7f1b
Compare
Remove the multi-line explanatory comment blocks the media-capture change added outside docstrings, keep single-line comments and the parametrize legend, and shrink deferred-import justifications to one line. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Laura Dang <laurad@nvidia.com>
Moving it above the new PrefixChains helpers made the PR diff show the whole function as rewritten. Put it back where it was so the diff shows only the additions. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Laura Dang <laurad@nvidia.com>
terrykong
reviewed
Oct 12, 2026
terrykong
left a comment
Collaborator
There was a problem hiding this comment.
Follow-up to my earlier review. All 10 resolved threads were checked against the code at 27d5432: 9 are fixed. The empty-media-rows change behind the last one works, but it turned the Lint job red, so there is one inline comment with a one-click fix.
The PR also has merge conflicts with main. Please rebase so the full CI can run.
Generated by Claude Code
terrykong
enabled auto-merge (squash)
October 12, 2026 05:36
terrykong
previously approved these changes
Oct 12, 2026
terrykong
left a comment
Collaborator
There was a problem hiding this comment.
lgtm @lauradang . all that's left is to resolve the linting issues and rebase to resolve the conflicts
Resolve conflicts with the zero-segment capture client change: Megatron capture builds its client with segment_size=0 and still pins the vision pixel dtype; setup starts storage units before token-capture setup. Also type the empty media segment lists for pyrefly. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Laura Dang <laurad@nvidia.com>
Upstream now passes the training sampling profile to the Megatron HTTP server through _http_sampling_defaults, which this branch also did inline; keep upstream's helper and drop the inline copy so start_text_gen_server no longer receives the default_* keywords twice. The parse tests keep the shared stub-server helper and upstream's sampling-defaults cases. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Laura Dang <laurad@nvidia.com>
Contributor
Author
|
/ok to test 2526ab4 |
…r pyrefly pyrefly types `[t] * n` as list[Tensor] even under a list[Tensor | None] annotation, so PackedTensor rejected it; a comprehension passed straight to PackedTensor is checked against the parameter type. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Laura Dang <laurad@nvidia.com>
Contributor
Author
|
/ok to test de162d9 |
terrykong
previously approved these changes
Oct 12, 2026
3 of 7 tasks
…agent The pinned Gym rejects agent instances that no environment server references, so the sanity fixture failed at spinup. Front the agent with the legacy_agent environment server, matching Gym's own example_multi_step config. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Laura Dang <laurad@nvidia.com>
Contributor
Author
|
/ok to test 621842a |
Contributor
Author
|
/ok to test 502d2e5 |
Contributor
Author
|
/ok to test 8f7e92a |
terrykong
approved these changes
Oct 14, 2026
4 tasks done
3 of 4 tasks
4 tasks done
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 does this PR do ?
Builds on #4191 (vLLM VLM media capture) and #4129 (MInf ledger capture), both now on
main, which is this PR's base. This supersedes lauradang#4, which targeted a fork copy of #4191's pre-rebase branch.This PR adds media capture on the Megatron Inference generation backend, on top of #4191's shared media contract: the same TQ media columns,
validate_media_tensors, batchedfetch_media, and finalizer.Token capture on the Megatron backend was text-only. Receipt rows carried no
pixel_values, so a VLM capture run trained image-blind, and Gym refused Megatron capture requests with image or video parts outright._prefix_media_countintooffload_params.TQMegatronPromptPreparersplices the exact expanded staged chain into the compact render, checks the endpoint's media count against what the chain staged, and answers with_prefix_expanded_token_count(the chain's length). The engine splits that prefix back off and expands only the media placeholders after it. There is one token space end to end: the stagedtoken_ids_deltais both what the trainer reads and what the next turn splices.splice_prefix_tokensaccepts a set, counts any member as a turn boundary, and keeps the EOS the model actually emitted, so the spliced prompt still starts with the model's exact tokens (Gym verifies that prefix). Callers that pass one id (vLLM, TRT-LLM, Dynamo) behave exactly as before.media_tensorsfor every image in the prompt. The preparer records how many media items the parent chain already staged (offload_params["ng_capture_minf"]["media_prev_count"], counted from the parents' small media columns, never pixels). The preparer also forwards the(height, width)of each of those items (media_prev_sizes); the stager compares the leading items of the engine'smedia_tensorsagainst them and fails the call ascapture_failedon any difference, so a later turn that resends an earlier image at a different size cannot leave the trainer with the earlier turn's pixels. Placeholder-token checks stay vLLM-only, since Megatron Inference reports no spans. The stager then cuts the engine's packed tensors at that boundary (slice_media_tensors, now next to the stager since the vLLM path no longer uses it) and stages the rest on the same staging key as extra columns (MEDIA_STAGING_FIELDS), the way routed experts ride the row.RolloutReassemblerwalks the terminal chain, checks each call's media columns against that call's staged geometry (rejecting the group withmedia_chain_identity,media_chain_incompatible, orinvalid_media_columns), concatenates the deltas in chain order as it does for tokens, and publishes the engine's packed patches aspixel_valueswithimgs_sizesandnum_frames. The Megatron-Bridge Omni model accepts the[total_patches, C*P*P]layout directly, so training projects exactly the pixels the policy generated against.pixel_valuesused to strip media from the whole batch and the policy trained image-blind with no error. The TQ adapter'sget_samplesnow raisesKeyErrorwhen a requested column is missing (matchingLocalDataPlaneClient.get_samples), and undercapture_mediathe finalizer drops a group in which no valid rollout carried media. This applies on both backends: a vLLM capture run on a dataset that mixes text-only and media prompts now loses its all-text groups too, where before this PR they were published without apixel_valuescolumn. The drop shrinks that step (counted indropped_prompt_groups); it is not replaced, even underon_dropped_prompt="replace". The adapter check is shared, so it also changes behaviour outside Megatron capture: non-capture single-controller VLM runs on a mixed text/media dataset (or mixed image/video keys) now fail at the train fetch instead of silently training without media, and vLLM capture gets the same check.grpo.deduplicate_multimodal_datais rejected under capture.top_k, and newer Megatron-LM fills omitted sampling fields from the model'sgeneration_config.json(e.g. Qwen3'stop_k=20), which would sample off-policy. The Megatron worker now passes the policy'stemperature/top_p/top_kas the HTTP server defaults. This fix is independent of media and also applies to text capture.docs/design-docs/token-capture-ledger.mdgains a "Multimodal rollouts on Megatron Inference" section.Worked example
A two-turn VLM rollout under capture, with toy values: the media placeholder is token
99, EOS is2, one 4x4 image expands to three media tokens and to four packed patches of 12 features. Turn 1 sends text plus one image. Turn 2 sends the whole history plus a second image; the chat template re-tokenizes the turn-1 answer as13although the model generated12, which is what the splice corrects. Each box is a process; the label says whose code runs in it.sequenceDiagram box Gym server (Gym code) participant G as Agent + capture middleware<br/>+ MegatronWorkerCaptureHandler end box Megatron HTTP endpoint, in RL's Megatron worker (Megatron code) participant EP as chat_completions.py end box Engine TP0/PP0 (Megatron loop, RL+Gym hooks) participant P as TQMegatronPromptPreparer participant S as TQMegatronTokenStager<br/>+ Gym adapter end box Engine, all ranks (Megatron) participant E as add_request / _build_vlm_request end participant TQ as TransferQueue participant L as Gym ledger box RL actors (RL code) participant F as RolloutReassembler participant T as Trainer end Note over G,L: Turn 1 - text + image1 G->>EP: chat req, offload_params.ng_capture = {c1, text, prev_len 0} EP->>P: compact [80,99,81] + image bytes (via DP coordinator) P->>E: text mode, pass through, broadcast E->>E: decode img1 to imgs[1,4,12], expand to [80,99,99,99,81], gen [12,2] E->>S: payload {prompt/gen ids, media_tensors (image1)} S->>TQ: r0/c1 - token delta [80,99,99,99,81,12,2], media cols (image1) S->>EP: ng_commit_coords {r0/c1, cum_len 7} via engine reply EP->>G: "a cat" + coords G->>L: CallRecord c1 Note over G,L: Turn 2 - history + image2 G->>L: resolve_parent finds c1 G->>EP: ng_capture = {c2, token_in, prev_len 7, staging_chain [r0/c1]} EP->>P: compact [80,99,81,13,2,20,99,21], template_prefix [80,99,81,13,2], eos [2], _prefix_media_count 1 P->>TQ: fetch_prefix_chains([r0/c1]) TQ->>P: expanded [80,99,99,99,81,12,2], media_count 1 P->>E: splice to [80,99,99,99,81,12,2,20,99,21], _prefix_expanded_token_count 7, required_prefix = expanded, media_prev_count 1 E->>E: keep prefix[:7], expand suffix [20,99,21] with image2 only, prompt [80,99,99,99,81,12,2,20,99,99,99,21], gen [30,2] E->>S: payload {prompt/gen ids, media_tensors (image1 + image2)} S->>S: Gym checks prompt[:7] == expanded chain, delta [20,99,99,99,21,30,2], slice media at 1 (image2 only) S->>TQ: r0/c2 token delta + media cols (image2) S->>EP: coords {r0/c2, prev_len 7, cum_len 14} EP->>G: answer + coords G->>L: CallRecord c2 (parent c1) Note over F,T: Rollout end F->>L: manifest to receipt {terminal c2, [c1, c2]} F->>TQ: fetch rows for 14 expanded tokens, fetch_media for r0/c1 and r0/c2, concat to imgs[1,8,12] F->>F: imgs_sizes cols == extras OK, pixel_values[8,12] F->>TQ: publish canonical row, clear r0/c1 and r0/c2 T->>TQ: read input_ids + pixel_values, six 99s match six embeddingsWhy the prefix length matters: if the engine were handed
[80,99,99,99,81,12,2,20,99,21]without_prefix_expanded_token_count, it would treat all four99s as placeholders for the request's two images and reject the request on the placeholder count. With the count, it expands only the99after position 7, and image 1's embeddings fill the three already-expanded slots.Dependencies
OffloadedRequestPayload.compact_prompt_token_idsand sends EOS ids as a list). It reaches RL through the Megatron-Bridge bump toec835530e(ci: Bump MBridge to ec835530efeff71b55ec015f36bcc0c33b8b52b7. #4363, merged into this branch), which nests Megatron-LMb4d72b79c.test_offloaded_payload_exposes_multimodal_capture_fieldspins the payload field and stitching keys on that pin, and the preparer tests import the real keys instead of patching them in.main2c10237cc, which includes feat(token-capture): admit multimodal Megatron capture requests Gym#3787 (admit multimodal Megatron capture requests) and requiresray[default]>=2.58.0; RL's lock is already on ray 2.58.0 from upstream main, anduv.lockis regenerated for the new Gym metadata (no resolved versions change).test_megatron_capture_handler_admits_media_requestssends image and video parts through the Megatron capture handler, so a re-pin to a Gym without fix(sc): apply reward scaling before advantage estimation #3787 fails RL CI.Testing
Local (macOS, in-process TransferQueue simple backend, Gym @
a2ef85b0e), after merging #4191 at691b9359b:test_openai_server_utils.py,test_setup.py,test_captured_media.py,test_multimodal_dict.py: 221 passedtest_tq_token_sink.py+test_rollout_reassembler.py(-m nemo_gym, megatron-coreinference_requeststubbed because megatron-core cannot install on macOS): 113 passed, including the vLLM/Megatron worked-example parity test and the expanded-prefix preparer casestest_megatron_token_capture_hosting.pyand themcoretests intest_megatron_generation_parse.py(including the new sampling-defaults test) need real megatron-core; CI onlyEnd-to-end Nemotron Omni video run (earlier compact-chain revision)
This run used the earlier revision of this PR, which spliced a compact chain against a Megatron-LM fork. The expanded-prefix revision needs a fresh run once RL's Megatron-Bridge pin carries NVIDIA/Megatron-LM#7598. The same bump also changes Nemotron Omni prompt rendering, so compare against a new baseline.
Completed a 42-step, 8-node × 4-GPU VSTAT GRPO run in 19m14s with this PR stack:
79fa6856b5e40a456c8d932f434a490c9682e296fafecf7c71ef63d027bb2197895e93c9effced4a0aa6cce27f75c2a4cf733b8eeb0e8180dc4517078e4ccc34b67a79bca73cb76242479f9090856ff4Run: PR stack (
jtg9a7rp) · cye baseline (o4ed2p8n) · W&B overlayThe reward curves track closely: Pearson correlation
0.9169, mean absolute error0.0486; mean reward over all 42 steps was0.1692for this PR stack vs.0.1592for the baseline. Steps 23–42 averaged0.2229vs.0.2281.The chart uses
nemo_rl/stepon the x-axis (not W&B's syntheticSTEP). Duplicate history entries from continuation were reduced to the final value for eachnemo_rl/step.Important effective configuration:
The dataset rows contain real local
videopaths. The step-40 native TQ checkpoint containspixel_values,imgs_sizes, andnum_frames(plus their row-shape metadata) inreplay_buffer_metadata.pt, and the same media columns in the TQ storage-unit pickle. A continuation read those columns from TQ and reached Nemotron Omni's_encode_images(images, ..., num_frames)path during policy logprob computation.🤖 Generated with Claude Code