Skip to content

chore(beep boop 🤖): Bump uv.lock (main, mcore-dev) (2026-04-18) - #3404

Closed
svcnvidia-nemo-ci wants to merge 3 commits into
mainfrom
bump-ci-container-2026-04-18-main-dev
Closed

chore(beep boop 🤖): Bump uv.lock (main, mcore-dev) (2026-04-18)#3404
svcnvidia-nemo-ci wants to merge 3 commits into
mainfrom
bump-ci-container-2026-04-18-main-dev

Conversation

@svcnvidia-nemo-ci

@svcnvidia-nemo-ci svcnvidia-nemo-ci commented Apr 18, 2026

Copy link
Copy Markdown
Contributor

🚀 PR to bump uv.lock in main.

🤖 This PR will be merged automatically once CI passes.

Summary by CodeRabbit

  • Chores
    • Updated internal build references and external dependency versions to align with current development state.

Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@svcnvidia-nemo-ci

Copy link
Copy Markdown
Contributor Author

/ok to test 8e1cb75

@copy-pr-bot

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

@coderabbitai

coderabbitai Bot commented Apr 18, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 2fb5ce70-cbc5-40e3-8b45-677eb18fcff6

📥 Commits

Reviewing files that changed from the base of the PR and between 5acef93 and 8e1cb75.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (2)
  • .dev.commit
  • 3rdparty/Megatron-LM

📝 Walkthrough

Walkthrough

This PR updates the commit hash in .dev.commit and advances the 3rdparty/Megatron-LM submodule pointer to a new commit revision, synchronizing both references to track the same external dependency version.

Changes

Cohort / File(s) Summary
Commit Reference Updates
.dev.commit, 3rdparty/Megatron-LM
Updated commit hashes to track new revision of Megatron-LM submodule.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Possibly related PRs

Suggested reviewers

  • yaoyu-33
  • ko3n1g
🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Title check ⚠️ Warning The title mentions bumping 'uv.lock' but the actual changes are updates to '.dev.commit' and the 'Megatron-LM' submodule pointer, not the dependency lock file. Update the title to accurately reflect the actual changes, such as 'chore: Update dev commit hash and Megatron-LM submodule pointer' or clarify what 'Bump uv.lock' entails.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Test Results For Major Changes ✅ Passed Pull request contains only minor metadata updates to commit hash references and submodule pointers with no functional code modifications.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch bump-ci-container-2026-04-18-main-dev

Comment @coderabbitai help to get the list of available commands and usage tips.

TECudaGraphHelper and VisionTECudaGraphHelper on mcore dev branch
do not accept pg_collection. Use inspect.signature to conditionally
pass the kwarg only when the mcore version supports it.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
@yaoyu-33

Copy link
Copy Markdown
Contributor

/ok to test c5428ba

…reprocessing

MCore's unwrap_model now strips the MegatronFSDP layer (added in
NVIDIA/Megatron-LM#4273), so preprocess_fsdp_dtensor_state_dict receives
a fully unwrapped GPTModel. The downstream MCore functions
(handle_swiglu_in_state_dict, handle_gdn_in_state_dict) call
model.get_parameter("module.{key}") which requires a .module wrapper.
Re-wrap the model when it arrives without one.

Fixes: AttributeError: GPTModel has no attribute `module`

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
@yaoyu-33

Copy link
Copy Markdown
Contributor

/ok to test b55dd73

@cspades cspades left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a quick WAR to the Megatron-FSDP error, it's fine. But the DEV PR causing this error is here: NVIDIA/Megatron-LM#4273

Comment on lines +1547 to +1555
# MCore FSDP functions (handle_swiglu_in_state_dict, handle_gdn_in_state_dict)
# expect model.get_parameter("module.{key}") to resolve through a wrapper's
# .module attribute. Since MCore's unwrap_model now also strips the
# MegatronFSDP layer, the model arriving here may be fully unwrapped.
# Re-wrap it so the "module." parameter lookups succeed.
if not hasattr(model, "module"):
_wrapper = torch.nn.Module()
_wrapper.module = model
model = _wrapper

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. So we yanked out Megatron-FSDP.module with unwrap_model (MCore dev) and now we're generating a state dictionary with a dummy Module wrapping.

I think this hack is a WAR for the direct error, and even if we put Megatron-FSDP back, not hasattr(model, "module") will be False, so then this code will not be run.

@cspades

cspades commented Apr 20, 2026

Copy link
Copy Markdown
Contributor

If @wplf reverts NVIDIA/Megatron-LM#4273 -> NVIDIA/Megatron-LM#4393 we don't need the Megatron-FSDP fix anymore.

No promises whether the current DEV branch would work, if you spot any issue with checkpointing please ping me. Can do another Megatron-Core DEV bump after the reversion PR is merged into DEV.

@yaoyu-33 yaoyu-33 added ci CI, automation, test queue, or workflow infrastructure work area:build Dependencies, packaging, images, and environment setup labels Apr 20, 2026
@yaoyu-33

Copy link
Copy Markdown
Contributor

Superseded by newer 2026-04-27 bump PR.

@yaoyu-33 yaoyu-33 closed this Apr 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:build Dependencies, packaging, images, and environment setup ci CI, automation, test queue, or workflow infrastructure work full-test-suite

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants