Skip to content

feat(model): accept pre-sharded packed Qwen3-VL inputs - #4917

Merged
yaoyu-33 merged 1 commit into
mainfrom
yuya/feat-qwen-vl-pre-sharded-cp-input
Jul 24, 2026
Merged

feat(model): accept pre-sharded packed Qwen3-VL inputs#4917
yaoyu-33 merged 1 commit into
mainfrom
yuya/feat-qwen-vl-pre-sharded-cp-input

Conversation

@yaoyu-33

@yaoyu-33 yaoyu-33 commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

What does this PR do ?

Accept Qwen3-VL packed THD inputs that a caller has already partitioned into Megatron's load-balanced zigzag context-parallel layout.

The current packed-CP path assumes input_ids still contains the full packed stream and always computes a local partition index. Some training frameworks pass rank-local tokens together with global packed-sequence metadata. Partitioning those tokens a second time either selects the wrong rows or fails before vision embeddings can be scattered.

This PR recognizes that contract when the physical global token count equals cp_size * local_token_count, validates that each packed segment can be divided into 2 * cp_size chunks, and skips the second partition. Because local token chunks no longer contain enough information to reconstruct multimodal positions, callers on this path must provide explicit rank-local 3D MRoPE position_ids.

Vision embeddings are still produced in full-sequence order. A count-only CP all-gather reconstructs the vision-token offset of each zigzag chunk and selects the rows owned by the local rank. The same indices are applied to deepstack features; ranks with no local visual tokens receive an empty selection.

Relationship to #4784

#4784 has landed, and this branch is now cleanly rebased onto current main.

This PR deliberately does not duplicate #4784's positional autograd.Function.apply fix, reduce-scatter backward correction, placeholder dtype handling, or zero-image-rank hang fix. The combined CP path still needs to be revalidated on two GPUs before final approval.

Attribution

The algorithm and current-main adaptation come from the radixark fork. This PR preserves those contributions directly:

The original downstream validation reported Qwen3-VL-2B CP2 with TP2/TP4, THD packing, healthy logprob differences of 0.0127-0.0131, and no NCCL timeout. Those GPU results belong to the original contributors and were not re-run in this adaptation.

Changelog

  • Detect and validate pre-sharded single-row THD packed inputs.
  • Skip native packed-CP indexing when the input is already rank-local.
  • Require explicit local MRoPE positions for pre-sharded inputs.
  • Select local vision and deepstack rows using per-chunk visual-token counts.
  • Add focused tests for full-input detection, CP2 multi-segment zigzag ordering, a zero-visual-token rank, deepstack selection, and preservation of the existing full-input partition path.

GitHub Actions CI

Local checks:

  • uv run --no-project --with pre-commit pre-commit run --all-files — passed
  • Focused CPU tests — 6 passed, 12 deselected
  • git diff --check and Python compilation — passed

The focused tests used a temporary local import stub for CUDA/Transformer Engine modules unavailable on macOS; no stub is present in this branch. A normal full environment cannot be resolved locally because nvidia-resiliency-ext is Linux-only.

Before marking ready, run in the standard Linux/GPU environment:

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? (The model docstring documents the new input contract; no user guide currently exposes this internal forward path.)
  • Does the PR affect components that are optional to install? (No new optional component or dependency.)

Additional Information

@yaoyu-33 yaoyu-33 added community-request feature New capabilities, enhancements, or enablement work blocked Work cannot move forward until an external dependency is cleared high-complexity Harder to merge: prone to conflicts and needs additional test coverage area:model Model implementations and HF bridge logic needs-more-tests Requires additional L0 and L1 test coverage before merge full-test-suite labels Jul 17, 2026
@copy-pr-bot

copy-pr-bot Bot commented Jul 17, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@yaoyu-33

Copy link
Copy Markdown
Contributor Author

/ok to test 449ced7

@yaoyu-33

Copy link
Copy Markdown
Contributor Author

/ok to test edbd870

@yaoyu-33

Copy link
Copy Markdown
Contributor Author

/ok to test edbd870

@yaoyu-33

Copy link
Copy Markdown
Contributor Author

/ok to test e7cdad0

Detect THD inputs that already use MCore zigzag context-parallel layout, skip a second partition, and select matching vision and deepstack rows with a count-only CP collective.

Adapted from the original algorithm by Zhichen Zeng (@Zhichenzzz): radixark@eeb84d7

Current-main integration based on Yusheng Su (@yushengsu-thu): radixark@390f983

Signed-off-by: Yu Yao <yaoyu.094@gmail.com>
@yaoyu-33
yaoyu-33 force-pushed the yuya/feat-qwen-vl-pre-sharded-cp-input branch from e7cdad0 to 49a3fe5 Compare July 23, 2026 23:23
@yaoyu-33 yaoyu-33 removed the blocked Work cannot move forward until an external dependency is cleared label Jul 23, 2026
@yaoyu-33

Copy link
Copy Markdown
Contributor Author

/ok to test 49a3fe5

@yaoyu-33 yaoyu-33 added the waiting-on-customer Waiting on the original author to respond label Jul 24, 2026
@yaoyu-33
yaoyu-33 merged commit a0ecffe into main Jul 24, 2026
156 checks passed
@yaoyu-33
yaoyu-33 deleted the yuya/feat-qwen-vl-pre-sharded-cp-input branch July 24, 2026 20:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:model Model implementations and HF bridge logic feature New capabilities, enhancements, or enablement work full-test-suite high-complexity Harder to merge: prone to conflicts and needs additional test coverage needs-more-tests Requires additional L0 and L1 test coverage before merge waiting-on-customer Waiting on the original author to respond

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant