Skip to content

fix: build refit metadata before communicator initialization - #4111

Draft
seonjinn wants to merge 3 commits into
NVIDIA-NeMo:mainfrom
seonjinn:sna/refit-pp-init-20260911
Draft

seonjinn wants to merge 3 commits into
NVIDIA-NeMo:mainfrom
seonjinn:sna/refit-pp-init-20260911

Conversation

@seonjinn

Copy link
Copy Markdown
Contributor

Problem

Async NCCL reshard setup builds source metadata before initializing the refit communicator. The metadata builder reads self.my_pp_stage, which is assigned only during communicator initialization, and fails with AttributeError.

Change

Read the pipeline rank from the already-initialized Megatron model parallel state. This preserves rank ownership without relying on transport initialization or assuming rank zero. No weight conversion or transfer logic changes.

Validation

Added regression cases for pipeline ranks 0, 1, and 3 without a communicator-owned rank attribute. GPU regression execution is pending; this is a draft, not a validated 20-step result.

Observed in a GB200 Qwen3-30B-A3B Async run before the first refit. No documentation or configuration changes are required.

Signed-off-by: seonjinn <sna@nvidia.com>
@seonjinn seonjinn added the CI:L0 Run doctests and unit tests label Sep 12, 2026
@copy-pr-bot

copy-pr-bot Bot commented Sep 12, 2026

Copy link
Copy Markdown

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@seonjinn

Copy link
Copy Markdown
Contributor Author

GPU regression update: on GB200 with vLLM 0.25.1, the integration reproducer failed at pipeline ranks 0, 1, and 3 before the fix with the same missing my_pp_stage AttributeError. After the fix, all 18 selected source-metadata/native-export tests passed.

The run used integration commit a51843ec0f3ebefc627dfe35c36d95d90a22f7b6, not this exact PR head. The subsequent fixture change updates an existing grouped-expert test to mock the model pipeline rank; that follow-up has not yet been rerun on GPU.

Read-only lifecycle review confirms model parallel state is initialized before metadata preparation, while the communicator-owned rank is initialized later. This removes that initialization-order dependency without changing weight conversion. It does not establish arbitrary distributed rank-order correctness or full 20-step numerical parity; those remain validation work.

@seonjinn

Copy link
Copy Markdown
Contributor Author

/ok to test 4f53b46

Signed-off-by: seonjinn <sna@nvidia.com>
@seonjinn seonjinn added CI:Lfast Runs a fast test suite and re-use nightly `main` container (but sync dependencies to PRs version) and removed CI:L0 Run doctests and unit tests labels Sep 17, 2026

This branch has not been deployed

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

Labels

CI:Lfast Runs a fast test suite and re-use nightly `main` container (but sync dependencies to PRs version)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant