[megatron][vlm] Allow sequence packing for VLMs incl. Qwen3.5-VL (CP=1) - #2357
Draft
xinyuangui2 wants to merge 5 commits into
Draft
xinyuangui2 wants to merge 5 commits into
xinyuangui2 wants to merge 5 commits into
Conversation
Megatron-Bridge's Qwen3-VL model rebuilds 3D mRoPE positions per packed sub-sequence when it receives a THD stream with position_ids=None, so the existing packed path works for VLMs. Context parallelism stays blocked. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KuNZVD7sZm2e3tySMqTsCG Signed-off-by: xgui <xgui@anyscale.com>
…dDeltaNet layers The double-packing guard from NovaSky-AI#1769 matched every Qwen3VLModel, so it also blocked dense Qwen3-VL, which takes SkyRL's [1, T] THD stream without re-packing. Found by the 4xH100 A/B run (worker init ValueError). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KuNZVD7sZm2e3tySMqTsCG Signed-off-by: xgui <xgui@anyscale.com>
Megatron-Bridge#4532 (in the pinned 8e7077c6) makes Qwen3VLModel treat a caller-packed [1, T] THD stream as authoritative: no second compaction, and the caller's packed_seq_params reach every layer unchanged. The guard from NovaSky-AI#1769 (model_packs_sequences_internally) guarded against the pre-#4532 behaviour, so drop it, its stale GPU-test case, and comments that said the VL path cannot pack. language_model_only stays: it skips the vision tower for text-only Qwen3.5 training. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KuNZVD7sZm2e3tySMqTsCG Signed-off-by: xgui <xgui@anyscale.com>
xinyuangui2
changed the title
…n3-VL and Qwen3.5-VL Variable-length image samples plus one text-only row, microbatches of 4 and a trailing single-sample microbatch, TP=1 and TP=2+SP. Compares every scored token and splits the stats by slot in the packed microbatch, so a sample-boundary leak (mRoPE restart, or GatedDeltaNet state/conv carried over) shows up as later slots being worse than slot 0. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MZKuFwA4VYK2tfFDVgsWt2 Signed-off-by: xgui <xgui@anyscale.com>
…okens only The test read logprobs through the forward(loss_fn=cross_entropy) path, which returns each sample's values compacted to its loss_mask and left-aligned, but indexed them as right-aligned, so it compared the wrong positions (and zeros). Use the inference forward path (the RL old/ref-logprob path, right-aligned [B, response_length]) and score only each row's assistant answer, as RL does: image-placeholder targets of random-noise images have huge, bf16-sensitive logprobs that are never trained on. Found by the 4xH100 run. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MZKuFwA4VYK2tfFDVgsWt2 Signed-off-by: xgui <xgui@anyscale.com>
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
1 participant
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.
Allow sequence packing (
trainer.remove_microbatch_padding=true) for VLMs on Megatron with CP=1._assert_vlm_supported. Megatron-Bridge'sQwen3VLModeltakes SkyRL's [1, T] THD stream withposition_ids=Noneand rebuilds 3D mRoPE per packed sub-sequence (rope.get_rope_index).model_packs_sequences_internallyand its error (from [megatron] Add seq packing support for qwen3.5 #1769). It guarded againstQwen3VLModelre-packing an already-packed stream, which [training] fix Qwen3-VL packed vlm_step MRoPE NVIDIA-NeMo/Megatron-Bridge#4532 fixed: a collate-packed [1, T] batch is now used as-is and the caller'spacked_seq_paramsreach every layer, including Qwen3.5's GatedDeltaNet. #4532 is in the pinned bridge (8e7077c6); [megatron] Add seq packing support for qwen3.5 #1769 was written against 91a15142, which predates it.language_model_onlystays: it skips the vision tower for text-only Qwen3.5.preprocess_packed_seqspre-shards per CP rank, and the bridge then needs rank-local 3D position ids.Qwen3-VL-8B geometry3k, 4xH100, TP=2, 10 steps, means over steps 3-10:
Pending: per-token packed-vs-unpacked GPU test (Qwen3-VL-2B, Qwen3.5-0.8B with images) and a Qwen3.5-VL RL run.
🤖 Generated with Claude Code
https://claude.ai/code/session_01KuNZVD7sZm2e3tySMqTsCG