Skip to content

Support multimodal metadata and vision context parallelism - #7315

Merged
tylerpoon merged 22 commits into
NVIDIA:mainfrom
tylerpoon:tpoon/github-vlm-sft-03-vision-context-parallel
Sep 19, 2026
Merged

tylerpoon merged 22 commits into
NVIDIA:mainfrom
tylerpoon:tpoon/github-vlm-sft-03-vision-context-parallel

Conversation

@tylerpoon

@tylerpoon tylerpoon commented Sep 14, 2026 •

Copy link
Copy Markdown
Contributor
  • I, the PR author, have personally reviewed every line of this PR.

What does this PR do?

Carry packed multimodal metadata through LLaVA, split/gather vision tokens for context parallelism, and handle padding and loss weighting consistently. Include model tests and single-writer dataloader checkpoint-state handling.

Replaces #7250, which was automatically closed when its bot-managed base branch was deleted after #7249 merged. The original discussion remains in #7250. This replacement reuses the original source branch and commits without code changes.

Production run scripts, data YAMLs, checkpoint conversion tooling, and separate CI compatibility fixes are outside this series.

Dependencies and review scope

#7249 has merged. This PR targets main and will remain based on main, not a bot mirror. Merge this PR before #7316 (the language-only checkpoint-initialization replacement for #7251). That dependent PR also targets main; the author will manage merge order.

The retained branch predates the squash merge of #7249, so its commit history/review diff may still include inherited vision-utility changes until refreshed against current main. Do not interpret those inherited changes as a new feature request.

Validation

  • Reuses the existing signed and signed-off head c4ca837e4ffaa0705e95778024ef4564d25f11c1.
  • Prior local Black, isort, pylint, Ruff, and Python syntax checks passed for the original branch; mypy was unavailable locally.
  • Combined-stack Super SFT validation is not an isolated-PR test result. Creating this replacement does not add a new test result or establish that current-main integration is green.
  • Kept as a draft pending review and CI. No functional-test label or manual full-CI trigger is requested, to limit CI load. Reviewers are selected by the author.

Signed-off-by: Tyler Poon <tpoon@nvidia.com>
Signed-off-by: Tyler Poon <tpoon@nvidia.com>
Signed-off-by: Tyler Poon <tpoon@nvidia.com>
Signed-off-by: Tyler Poon <tpoon@nvidia.com>
Signed-off-by: Tyler Poon <tpoon@nvidia.com>
@copy-pr-bot

copy-pr-bot Bot commented Sep 14, 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.

@tylerpoon
tylerpoon marked this pull request as ready for review September 16, 2026 17:04
@tylerpoon
tylerpoon requested review from a team as code owners September 16, 2026 17:04
Comment thread megatron/training/checkpointing.py Resolved
Signed-off-by: Tyler Poon <tpoon@nvidia.com>
Signed-off-by: Tyler Poon <tpoon@nvidia.com>
Keep per-replica CP partition diagnostics while removing the global TP rank lookup from the splitter. Simplify the unreachable video frame-count fallback.

Signed-off-by: Tyler Poon <tpoon@nvidia.com>
Signed-off-by: Tyler Poon <tpoon@nvidia.com>
@svcnvidia-nemo-ci svcnvidia-nemo-ci added the Final Review PR is in the "final review" stage label Sep 18, 2026
@tylerpoon
tylerpoon enabled auto-merge September 18, 2026 20:08
@Phlip79

Phlip79 commented Sep 18, 2026

Copy link
Copy Markdown
Member

/ok to test 76dfaae

@svcnvidia-nemo-ci svcnvidia-nemo-ci added Approved All necessary approvals have been made and removed Final Review PR is in the "final review" stage labels Sep 18, 2026

Copy link
Copy Markdown
Collaborator

Formal /review skipped: the reviewed head ccce1ea is no longer current; re-run the command

@tylerpoon
tylerpoon added this pull request to the merge queue Sep 19, 2026
@nemo-automation-bot

Copy link
Copy Markdown

🔄 Merge queue validation started!

You can track the progress here: https://github.com/NVIDIA/Megatron-LM/actions/runs/35415930608

@nemo-automation-bot

Copy link
Copy Markdown

🔄 Merge queue validation started!

You can track the progress here: https://github.com/NVIDIA/Megatron-LM/actions/runs/35416653178

Merged via the queue into NVIDIA:main with commit d7e6824 Sep 19, 2026
111 checks passed
@tylerpoon
tylerpoon deleted the tpoon/github-vlm-sft-03-vision-context-parallel branch September 19, 2026 04:54
Connor-XY added a commit to Connor-XY/Megatron-LM that referenced this pull request Oct 7, 2026
test_forward_video_accounts_for_native_merging (added upstream in NVIDIA#7315)
builds a bare LLaVAModel and patched the global context-parallel size and
rank. LLaVAModel.forward now reads self.pg_collection.cp and passes it to the
dynamic-resolution CP helpers, which size the split from that group, so the
test failed with "'LLaVAModel' object has no attribute 'pg_collection'".

Give the model a collection with a sentinel CP group, patch the group-based
size and rank lookups in context_parallel instead, and check that the
gather receives the same group.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
Connor-XY added a commit to Connor-XY/Megatron-LM that referenced this pull request Oct 8, 2026
test_forward_video_accounts_for_native_merging (added upstream in NVIDIA#7315)
builds a bare LLaVAModel and patched the global context-parallel size and
rank. LLaVAModel.forward now reads self.pg_collection.cp and passes it to the
dynamic-resolution CP helpers, which size the split from that group, so the
test failed with "'LLaVAModel' object has no attribute 'pg_collection'".

Give the model a collection with a sentinel CP group, patch the group-based
size and rank lookups in context_parallel instead, and check that the
gather receives the same group.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>

This branch was previously deployed

2 inactive deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Approved All necessary approvals have been made complexity: high

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants