Conversation
Signed-off-by: Linglin Jing <50938792+jinglinglingling@users.noreply.github.com>
Contributor
Author
|
/ok to test 2bb044e |
rohitrango
reviewed
Sep 14, 2026
Contributor
|
/ok to test 167ad7b |
Signed-off-by: rohitrango <rohit.rango@gmail.com>
Contributor
|
/ok to test 188c841 |
Contributor
Author
|
Addressed the remaining review comment with a two-turn |
Contributor
|
could you sign your latest commit? once it passes DCO I will re-run |
Signed-off-by: Linglin Jing <50938792+jinglinglingling@users.noreply.github.com>
jinglinglingling
force-pushed
the
linglinj/fix-async-vlm-image-payload
branch
from
September 15, 2026 02:51
ba19807 to
2037327
Compare
Contributor
|
/ok to test 2037327 |
Contributor
|
/ok to test 6d89cac |
terrykong
approved these changes
Sep 16, 2026
seonjinn
added a commit
to seonjinn/RL
that referenced
this pull request
Sep 16, 2026
NVIDIA-NeMo#3803 replaced NATIVE_MULTIMODAL_KEYS with VLLM_PROMPT_KEYS in nemo_rl/data/multimodal_utils.py; NVIDIA-NeMo#4113 landed 37 minutes later still importing the old name into nemo_rl/experience/rollout_manager.py. Both merged clean on their own, so main carries an unresolvable import that reds the pyrefly lint job on every open PR. rollouts.py:1320 already builds the same per-sample multimodal dict from VLLM_PROMPT_KEYS; this restores parity with it. Signed-off-by: seonjinn <sna@nvidia.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
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 ?
Preserves native multimodal side-channel fields when SingleController dispatches async VLM rollouts, so vLLM and the training forward receive the same image input.
The async per-sample path previously rebuilt generation input from tokenized message data but dropped fields such as
vllm_contentandvllm_images. This made vLLM generation image-blind while Megatron recomputed log probabilities with the image, producing largetoken_mult_prob_errorvalues.For multi-turn rollouts, later turns retain native media while clearing stale
vllm_content, allowing the updated tokenized prefix to be used.This PR does not modify recipes or generation lengths. The existing defaults remain Qwen
max_new_tokens=1024and Nemotronmax_new_tokens=4096.Validation results:
train/token_mult_prob_error = 2.776391.02max_new_tokens=1024, 8 prompts x 16 generations), 100/100 steps: all TMPE values below1.05; range1.02006–1.02614; truncation rate0max_new_tokens=4096, 8 prompts x 16 generations), 100/100 steps completed. Most aggregate TMPE values were approximately1.03; three outliers remain under investigation, including one immediately after checkpoint resume.Reproduction commands
Qwen native-length 100-step validation:
Nemotron native-length 100-step validation (3 nodes / 8 GPUs per node):