Temporarily Remove Encoder PP Support by yaoyu-33 · Pull Request #14167 · NVIDIA-NeMo/NeMo

yaoyu-33 · 2025-07-09T03:24:00Z

Important

The Update branch button must only be pressed in very rare occassions.
An outdated branch is never blocking the merge of a PR.
Please reach out to the automation team before pressing that button.

As a part of effort to refactor parallel management in Mcore, we need to temporarily remove the encoder pp support, the api is also changed in mcore side.

STOP USING encoder_pipeline_model_parallel_size , encoder_tensor_model_parallel_size , pipeline_model_parallel_split_rank

Collection: [nlp, llm, vlm]

Changelog

Add specific line by line info of high level changes in this PR.

Usage

You can potentially add a usage example below

# Add a code snippet demonstrating how to use this

GitHub Actions CI

The Jenkins CI system has been replaced by GitHub Actions self-hosted runners.

The GitHub Actions CI will run automatically when the "Run CICD" label is added to the PR.
To re-run CI remove and add the label again.
To run CI on an untrusted fork, a NeMo user with write access must first click "Approve and run".

Before your PR is "Ready for review"

Pre checks:

Make sure you read and followed Contributor guidelines
Did you write any new necessary tests?
Did you add or update any necessary documentation?
Does the PR affect components that are optional to install? (Ex: Numba, Pynini, Apex etc)
- Reviewer: Does the PR have correct import guards for all optional libraries?

PR Type:

New Feature
Bugfix
Documentation

If you haven't finished some of the above items you can still open "Draft" PR.

Who can review?

Anyone in the community is free to review the PR once the checks have passed.
Contributor guidelines contains specific people who can review PRs to various areas.

Additional Information

Related to # (issue)

Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>

Signed-off-by: yaoyu-33 <yaoyu-33@users.noreply.github.com>

nemo/collections/nlp/modules/common/megatron/megatron_init.py

nemo/lightning/megatron_init.py

Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>

Signed-off-by: yaoyu-33 <yaoyu-33@users.noreply.github.com>

Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>

…a/remove_encoder_pp

ZhiyuLi-Nvidia · 2025-07-09T17:50:36Z

nemo/collections/nlp/modules/common/megatron/transformer.py

        if parallel_state.get_pipeline_model_parallel_world_size() > 1:
-            if self.model_type == ModelType.encoder_and_decoder:
+            if self.model_type == ModelType.encoder_or_decoder:
                assert parallel_state.get_pipeline_model_parallel_split_rank() is not None


would this assertion still be valid after we remove pipeline_model_parallel_split_rank in setting?

ZhiyuLi-Nvidia · 2025-07-09T17:56:51Z

Thank you @yaoyu-33.
Is there a future plan to fully remove the encoder_pipeline_model_parallel_size , encoder_tensor_model_parallel_size , pipeline_model_parallel_split_rank. That would fully eliminate the associated attributes, state fields, and functions rather than simply adding assertions to prevent their use.
I think we will do that once after refactoring mcore. Am I right?

ZhiyuLi-Nvidia

Thank you @yaoyu-33.
Just few nits.

Other than that, LGTM.

Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>

Signed-off-by: yaoyu-33 <yaoyu-33@users.noreply.github.com>

ko3n1g

Transient CI failure, will merge manually

* remove encoder pp api Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com> * Apply isort and black reformatting Signed-off-by: yaoyu-33 <yaoyu-33@users.noreply.github.com> * update Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com> * Apply isort and black reformatting Signed-off-by: yaoyu-33 <yaoyu-33@users.noreply.github.com> * lint Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com> * fix test Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com> * some addtional removing in strategy lib Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com> * update assert Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com> * Apply isort and black reformatting Signed-off-by: yaoyu-33 <yaoyu-33@users.noreply.github.com> --------- Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com> Signed-off-by: yaoyu-33 <yaoyu-33@users.noreply.github.com> Co-authored-by: yaoyu-33 <yaoyu-33@users.noreply.github.com> Signed-off-by: Amir Hussein <amhussein@nvidia.com>

* remove encoder pp api Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com> * Apply isort and black reformatting Signed-off-by: yaoyu-33 <yaoyu-33@users.noreply.github.com> * update Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com> * Apply isort and black reformatting Signed-off-by: yaoyu-33 <yaoyu-33@users.noreply.github.com> * lint Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com> * fix test Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com> * some addtional removing in strategy lib Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com> * update assert Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com> * Apply isort and black reformatting Signed-off-by: yaoyu-33 <yaoyu-33@users.noreply.github.com> --------- Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com> Signed-off-by: yaoyu-33 <yaoyu-33@users.noreply.github.com> Co-authored-by: yaoyu-33 <yaoyu-33@users.noreply.github.com>

* remove encoder pp api Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com> * Apply isort and black reformatting Signed-off-by: yaoyu-33 <yaoyu-33@users.noreply.github.com> * update Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com> * Apply isort and black reformatting Signed-off-by: yaoyu-33 <yaoyu-33@users.noreply.github.com> * lint Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com> * fix test Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com> * some addtional removing in strategy lib Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com> * update assert Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com> * Apply isort and black reformatting Signed-off-by: yaoyu-33 <yaoyu-33@users.noreply.github.com> --------- Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com> Signed-off-by: yaoyu-33 <yaoyu-33@users.noreply.github.com> Co-authored-by: yaoyu-33 <yaoyu-33@users.noreply.github.com> Signed-off-by: Guyue Huang <guyueh@nvidia.com>

remove encoder pp api

0bae372

Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>

github-actions bot added TTS NLP Multi Modal labels Jul 9, 2025

Apply isort and black reformatting

0e8a353

Signed-off-by: yaoyu-33 <yaoyu-33@users.noreply.github.com>

github-advanced-security bot found potential problems Jul 9, 2025

View reviewed changes

yaoyu-33 and others added 4 commits July 9, 2025 09:43

update

487dcfb

Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>

Apply isort and black reformatting

8b87bea

Signed-off-by: yaoyu-33 <yaoyu-33@users.noreply.github.com>

lint

89fec88

Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>

Merge remote-tracking branch 'origin/yuya/remove_encoder_pp' into yuy…

9635c2e

…a/remove_encoder_pp

yaoyu-33 requested a review from ZhiyuLi-Nvidia July 9, 2025 17:18

yaoyu-33 added the Run CICD label Jul 9, 2025

yaoyu-33 temporarily deployed to test July 9, 2025 17:19 — with GitHub Actions Inactive

ZhiyuLi-Nvidia reviewed Jul 9, 2025

View reviewed changes

ZhiyuLi-Nvidia previously approved these changes Jul 9, 2025

View reviewed changes

github-actions bot removed the Run CICD label Jul 10, 2025

fix test

35e924b

Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>

yaoyu-33 dismissed ZhiyuLi-Nvidia’s stale review via 35e924b July 11, 2025 20:42

ko3n1g added the Run CICD label Jul 14, 2025

ko3n1g temporarily deployed to test July 14, 2025 06:55 — with GitHub Actions Inactive

github-actions bot removed the Run CICD label Jul 14, 2025

some addtional removing in strategy lib

661d6b8

Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>

yaoyu-33 added the Run CICD label Jul 14, 2025

yaoyu-33 temporarily deployed to test July 14, 2025 17:02 — with GitHub Actions Inactive

github-actions bot removed the Run CICD label Jul 15, 2025

yaoyu-33 and others added 3 commits July 15, 2025 08:08

Merge branch 'main' into yuya/remove_encoder_pp

24649f6

update assert

39c0752

Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>

Apply isort and black reformatting

6b15aa3

Signed-off-by: yaoyu-33 <yaoyu-33@users.noreply.github.com>

ko3n1g added the Run CICD label Jul 16, 2025

ko3n1g temporarily deployed to test July 16, 2025 05:45 — with GitHub Actions Inactive

ZhiyuLi-Nvidia approved these changes Jul 17, 2025

View reviewed changes

ko3n1g approved these changes Jul 17, 2025

View reviewed changes

ko3n1g merged commit 54a2602 into main Jul 17, 2025
594 of 748 checks passed

ko3n1g deleted the yuya/remove_encoder_pp branch July 17, 2025 07:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Temporarily Remove Encoder PP Support#14167

Temporarily Remove Encoder PP Support#14167
ko3n1g merged 11 commits intomainfrom
yuya/remove_encoder_pp

yaoyu-33 commented Jul 9, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ZhiyuLi-Nvidia Jul 9, 2025

Uh oh!

ZhiyuLi-Nvidia commented Jul 9, 2025

Uh oh!

ZhiyuLi-Nvidia left a comment

Uh oh!

ko3n1g left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

yaoyu-33 commented Jul 9, 2025

Changelog

Usage

GitHub Actions CI

Before your PR is "Ready for review"

Who can review?

Additional Information

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ZhiyuLi-Nvidia Jul 9, 2025

Choose a reason for hiding this comment

Uh oh!

ZhiyuLi-Nvidia commented Jul 9, 2025

Uh oh!

ZhiyuLi-Nvidia left a comment

Choose a reason for hiding this comment

Uh oh!

ko3n1g left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants