Skip to content

Megatron-FSDP: log mcore detection only after imports succeed#4400

Merged
wujingyue merged 1 commit intoNVIDIA:mainfrom
wujingyue:fix/fsdp-mcore-detect-log-order
Apr 21, 2026
Merged

Megatron-FSDP: log mcore detection only after imports succeed#4400
wujingyue merged 1 commit intoNVIDIA:mainfrom
wujingyue:fix/fsdp-mcore-detect-log-order

Conversation

@wujingyue
Copy link
Copy Markdown
Contributor

@wujingyue wujingyue commented Apr 20, 2026

Summary

  • In megatron_fsdp.py, the "Detected Megatron Core…" log was emitted before the megatron.core imports inside the try block. When mcore isn't installed, the user saw "Detected…" immediately followed by the fallback "not installed" log — confusing and misleading.

Test plan

  • isort + python -m py_compile on the edited file
  • CI (Run tests label — log-ordering only, no behavior change)

🤖 Generated with Claude Code

Previously the "Detected Megatron Core" log was emitted before the
megatron.core imports inside the try block, so when mcore isn't
installed the user saw "Detected…" immediately followed by the
"not installed" fallback log. Move the success-case log to after
the imports, matching the pattern already in param_and_grad_buffer.py.
The except-branch log stays at the top of its block since we already
know mcore isn't available there.

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

copy-pr-bot Bot commented Apr 20, 2026

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.

@wujingyue wujingyue marked this pull request as ready for review April 20, 2026 22:53
@wujingyue wujingyue requested review from a team as code owners April 20, 2026 22:53
@svcnvidia-nemo-ci svcnvidia-nemo-ci requested a review from a team April 20, 2026 22:53
Copy link
Copy Markdown
Member

@cspades cspades left a comment

Choose a reason for hiding this comment

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

THX, is this the only offender?

@svcnvidia-nemo-ci svcnvidia-nemo-ci added the Final Review PR is in the "final review" stage label Apr 20, 2026
@wujingyue
Copy link
Copy Markdown
Contributor Author

/ok to test a5c574a

@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 Apr 21, 2026
@wujingyue
Copy link
Copy Markdown
Contributor Author

/ok to test a5c574a

@wujingyue wujingyue added this pull request to the merge queue Apr 21, 2026
@svcnvidia-nemo-ci
Copy link
Copy Markdown

🔄 Merge queue validation started!

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

@svcnvidia-nemo-ci
Copy link
Copy Markdown

🔄 Merge queue validation started!

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

@svcnvidia-nemo-ci
Copy link
Copy Markdown

🔄 Merge queue validation started!

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

Merged via the queue into NVIDIA:main with commit e5ec9ab Apr 21, 2026
76 of 78 checks passed
@wujingyue wujingyue deleted the fix/fsdp-mcore-detect-log-order branch April 21, 2026 22:32
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.

6 participants