Skip to content

fix: use no_mask in local ViT layer spec - #4395

Merged
Phlip79 merged 1 commit into
NVIDIA:mainfrom
Phlip79:philip/fix-vit
May 14, 2026
Merged

fix: use no_mask in local ViT layer spec#4395
Phlip79 merged 1 commit into
NVIDIA:mainfrom
Phlip79:philip/fix-vit

Conversation

@Phlip79

@Phlip79 Phlip79 commented Apr 20, 2026

Copy link
Copy Markdown
Member

Summary

  • Changes AttnMaskType.causalAttnMaskType.no_mask in get_vit_layer_with_local_spec() (megatron/core/models/vision/vit_layer_specs.py:72) to match the TE spec.
  • ViTs use bidirectional self-attention over image patches; causal masking is incorrect.
  • Commit 20abc85 ("fix vit mask") fixed this in the TE path but missed the local path.

Fixes #1791

Test plan

  • Confirm pretrain_vlm.py with --transformer-impl local produces correct ViT behavior
  • Existing vision unit tests (tests/unit_tests/transformer/test_vision_cuda_graphs.py) still pass
  • No regression in TE path (unchanged)

🤖 Generated with Claude Code

The local ViT layer spec incorrectly used AttnMaskType.causal while the
TE spec uses AttnMaskType.no_mask. ViTs use bidirectional self-attention
over image patches, so causal masking is incorrect. Commit 20abc85
("fix vit mask") fixed this in the TE path but missed the local path.

Fixes NVIDIA#1791

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@copy-pr-bot

copy-pr-bot Bot commented Apr 20, 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.

@Phlip79
Phlip79 marked this pull request as ready for review April 20, 2026 19:06
@Phlip79
Phlip79 requested review from a team as code owners April 20, 2026 19:06
@Phlip79

Phlip79 commented Apr 20, 2026

Copy link
Copy Markdown
Member Author

/ok to test 5ab31f4

@svcnvidia-nemo-ci
svcnvidia-nemo-ci requested a review from a team April 20, 2026 19:06
@svcnvidia-nemo-ci svcnvidia-nemo-ci added the Final Review PR is in the "final review" stage label Apr 20, 2026
@svcnvidia-nemo-ci svcnvidia-nemo-ci added this to the Core 0.16 milestone Apr 20, 2026
@Phlip79
Phlip79 enabled auto-merge April 21, 2026 20:02
@Phlip79

Phlip79 commented May 1, 2026

Copy link
Copy Markdown
Member Author

@ericharper this is a test

@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 May 14, 2026
@Phlip79
Phlip79 added this pull request to the merge queue May 14, 2026
@svcnvidia-nemo-ci

Copy link
Copy Markdown
Contributor

🔄 Merge queue validation started!

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

Merged via the queue into NVIDIA:main with commit 1ba0aa9 May 14, 2026
74 of 76 checks passed
@Phlip79
Phlip79 deleted the philip/fix-vit branch May 14, 2026 03:26
cspades pushed a commit to cspades/Megatron-LM that referenced this pull request May 14, 2026
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
janEbert pushed a commit to janEbert/Megatron-LM that referenced this pull request Jun 2, 2026
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
yhgalaxy pushed a commit to yhgalaxy/Megatron-LM that referenced this pull request Jun 17, 2026
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: yhgalaxy <yhgalaxy@outlook.com>
jon-barker pushed a commit to jon-barker/Megatron-LM that referenced this pull request Jul 10, 2026
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Jon Barker <jbarker@aws-cmh-slurm-1-vscode-02.cm.cluster>
terminator123 pushed a commit to 021ai/Megatron-LM that referenced this pull request Aug 3, 2026
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
svcnvidia-nemo-ci pushed a commit to dimapihtar/Megatron-LM that referenced this pull request Aug 4, 2026
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Dmytro Pykhtar <dpykhtar@nvidia.com>
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: low

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] why use causal in ViT layer spec with Mcore local layers?

4 participants