Skip to content

[recipe] feat: enable THD packing by default for Qwen3.5-VL finetune - #3481

Merged
yaoyu-33 merged 2 commits into
mainfrom
chcui/qwen35-vl-thd-default
May 18, 2026
Merged

[recipe] feat: enable THD packing by default for Qwen3.5-VL finetune#3481
yaoyu-33 merged 2 commits into
mainfrom
chcui/qwen35-vl-thd-default

Conversation

@cuichenx

@cuichenx cuichenx commented Apr 22, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Enable THD sequence packing by default for all Qwen3.5-VL SFT and PEFT recipes so attention skips padding FLOPs on short samples.

Changelog

  • Flip cfg.dataset.pack_sequences_in_batch from False to True in _qwen35_vl_apply_common (shared by every Qwen3.5-VL SFT and PEFT recipe)
  • Pretrain recipes are unaffected — they route through _qwen3_vl_common and do not set this flag

Motivation

When PP>1 or EP>1, qwen3_vl_step.pack_or_pad_batch_sequences already sets force_to_pad_to_seq_len=True, so every micro-batch is padded to the full seq_length (4096). Without THD, attention computes the full seq_length^2 tile for every sample — documents of ~1000 tokens waste roughly 15x attention FLOPs. Flipping pack_sequences_in_batch=True builds cu_seqlens_q/kv(_padded) so flash-attn treats the pad tail as an independent empty segment and skips it. Non-attention layers (MLP, norms, router) still run on the padded positions, but that's a cheaper cost than letting attention eat the pad.

Dependency

This change requires Megatron-LM NVIDIA/Megatron-LM#2645feat(moe): Support packed sequence for gated delta net (GDN). Qwen3.5-VL uses a hybrid attention/GDN architecture (experimental_attention_variant='gated_delta_net'), and prior to that PR the GDN block raised NotImplementedError("GDN does not support packed sequence for now.") at megatron/core/ssm/gated_delta_net.py:303. This PR should land after the next mcore-main bump that pulls in #2645.

Empirical verification

Compared pack_sequences_in_batch=False (baseline) vs True (this PR) on Qwen3.5-VL 2B SFT, cord_v2, 1 node × 8 GPUs, MBS=2, GBS=16, 100 iters, seed=1234, mcore @ NVIDIA/Megatron-LM main (includes #2645).

pack=False pack=True
iter 1 lm_loss 0.74019 0.74025
iter 100 lm_loss 0.03465 0.03222
iter 100 mtp_1 loss 0.05543 0.05354
100-iter wall time 315 s 158 s (~2× speedup)
max delta on any iter < 1e-3

GitHub Actions CI

See the CI section in the Contributing doc for how to trigger the CI.
A Nvidia developer will need to approve and trigger the CI for external contributors.

Before your PR is "Ready for review"

Pre checks:

  • Make sure you read and followed Contributor guidelines
  • Did you write any new necessary tests?
  • Did you add or update any necessary documentation?
  • Does the PR affect components that are optional to install? (Ex: Numba, Pynini, Apex etc)
    • Reviewer: Does the PR have correct import guards for all optional libraries?

If you haven't finished some of the above items you can still open "Draft" PR.

Additional Information

Related to # (issue)

Flip pack_sequences_in_batch from False to True in the shared
_qwen35_vl_apply_common helper used by all Qwen3.5-VL SFT and PEFT
recipes. When PP>1 or EP>1 the step function already forces each
micro-batch to pad to seq_length; building THD cu_seqlens lets the
attention kernel skip padding FLOPs on short samples. Pretrain recipes
are unaffected (they route through _qwen3_vl_common).

Signed-off-by: Chen Cui <chcui@nvidia.com>
@copy-pr-bot

copy-pr-bot Bot commented Apr 22, 2026

Copy link
Copy Markdown

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@yaoyu-33 yaoyu-33 added feature New capabilities, enhancements, or enablement work area:recipe Training recipes and launch configs labels Apr 23, 2026
@cuichenx
cuichenx marked this pull request as ready for review May 14, 2026 22:07
@cuichenx

Copy link
Copy Markdown
Contributor Author

/ok to test a83db63

@claude

claude Bot commented May 14, 2026

Copy link
Copy Markdown
Contributor

Light Review

Clean, minimal change — flips pack_sequences_in_batch from False to True in _qwen35_vl_apply_common, aligning Qwen3.5-VL SFT/PEFT recipes with the VLM defaults already set by _sft_common_vlm() and _peft_common_vlm() in common.py (both set pack_sequences_in_batch=True).

Potential issue

MTP + in-batch packing interaction: All Qwen3.5-VL SFT/PEFT recipes enable MTP (mtp_num_layers=1, line 77) and now also enable in-batch packing. The sequence-packing skill doc notes "MTP finetuning is documented as incompatible with packed sequences" (pitfall #6). This likely refers to offline packed LLM sequences (PackedSequenceSpecs), not VLM in-batch packing, and I see no config validation preventing the combination — but please confirm this has been tested end-to-end with MTP + THD packing before merging.

Missing test coverage

No existing unit test asserts the value of cfg.dataset.pack_sequences_in_batch for any Qwen3.5-VL recipe (tests/unit_tests/recipes/qwen_vl/test_qwen35_vl_recipes.py). Consider adding a simple assertion in one of the default-checking tests (e.g., test_qwen35_vl_800m_sft_defaults) to pin the new default:

assert cfg.dataset.pack_sequences_in_batch is True

This prevents accidental regression if someone re-adds the False override.

Suggested test cases

No perf tests impacted. The perf configs under scripts/performance/configs/qwen_vl/ are all pretrain workloads, which route through _qwen3_vl_common and are unaffected by this change.

@yaoyu-33 yaoyu-33 added the needs-review PR is ready for code review and waiting on a reviewer label May 14, 2026
@yaoyu-33
yaoyu-33 merged commit ae0ae7e into main May 18, 2026
100 checks passed
@yaoyu-33
yaoyu-33 deleted the chcui/qwen35-vl-thd-default branch May 18, 2026 21:49
vasunvidia pushed a commit to vasunvidia/Megatron-Bridge that referenced this pull request Jun 10, 2026
…VIDIA-NeMo#3481)

Signed-off-by: Chen Cui <chcui@nvidia.com>
Signed-off-by: Vasudevan Rengasamy <vrengasamy@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:recipe Training recipes and launch configs feature New capabilities, enhancements, or enablement work needs-review PR is ready for code review and waiting on a reviewer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants