Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move Parallelism usage from Apex -> Megatron Core #6393

Merged
merged 48 commits into from
Apr 13, 2023
Merged

Conversation

aklife97
Copy link
Collaborator

@aklife97 aklife97 commented Apr 7, 2023

What does this PR do ?

This PR moves model parallelism in NeMo to use Megatron-core instead of Apex.

  • All models use dataloader_iter in train and eval step (except eval for T5 p-tuning)
  • All pre-training models that use a constant sequence length sample based on microbatches
  • All downstream tasks - p-tuning, adapter etc - that depend on dynamic seq len use global batch based sampling

We still use Apex for microbatch calculator, some enums/utils [both to be soon shifted to core], LayerNorm/other things with kernel

Collection: NLP

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 

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)

ericharper and others added 14 commits October 5, 2022 10:00
Signed-off-by: ericharper <[email protected]>
Signed-off-by: Abhinav Khattar <[email protected]>
Signed-off-by: Abhinav Khattar <[email protected]>
Signed-off-by: ericharper <[email protected]>
Signed-off-by: SeanNaren <[email protected]>
* fix tests

Signed-off-by: Yi Dong <[email protected]>

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

---------

Signed-off-by: Yi Dong <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Signed-off-by: Abhinav Khattar <[email protected]>
* Support T5 with Megatron Core

Signed-off-by: SeanNaren <[email protected]>

* Remove comment

Signed-off-by: SeanNaren <[email protected]>

* Update prediction step

Signed-off-by: SeanNaren <[email protected]>

* Further changes to fix fine-tuning

Signed-off-by: SeanNaren <[email protected]>

* Bug fixes from runs

Signed-off-by: SeanNaren <[email protected]>

* Revert changes to batch sampler, swap to pretrained sampler

Signed-off-by: SeanNaren <[email protected]>

* Address feedback

Signed-off-by: SeanNaren <[email protected]>

---------

Signed-off-by: SeanNaren <[email protected]>
Signed-off-by: Abhinav Khattar <[email protected]>
@github-actions github-actions bot added core Changes to NeMo Core NLP labels Apr 7, 2023
@aklife97 aklife97 requested a review from ericharper April 7, 2023 15:52
@github-actions github-actions bot added the CI label Apr 7, 2023
@aklife97 aklife97 marked this pull request as ready for review April 7, 2023 21:28
@aklife97 aklife97 requested a review from okuchaiev April 7, 2023 22:35
Signed-off-by: Abhinav Khattar <[email protected]>
Signed-off-by: Abhinav Khattar <[email protected]>
Signed-off-by: Abhinav Khattar <[email protected]>
Signed-off-by: Abhinav Khattar <[email protected]>
Signed-off-by: Abhinav Khattar <[email protected]>
Signed-off-by: Abhinav Khattar <[email protected]>
Signed-off-by: Abhinav Khattar <[email protected]>
Signed-off-by: Abhinav Khattar <[email protected]>
Signed-off-by: Abhinav Khattar <[email protected]>
Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

CodeQL found more than 10 potential problems in the proposed changes. Check the Files changed tab for more details.

ericharper
ericharper previously approved these changes Apr 11, 2023
Copy link
Collaborator

@ericharper ericharper left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you!

This PR was a massive effort. Thanks to all for their contributions and especially @aklife97 for putting it all together here.

Signed-off-by: Abhinav Khattar <[email protected]>
Signed-off-by: Abhinav Khattar <[email protected]>
Copy link
Collaborator

@ericharper ericharper left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@ericharper ericharper merged commit 7854bd4 into main Apr 13, 2023
@ericharper ericharper deleted the GPT_integrate_core branch April 13, 2023 16:42
hsiehjackson pushed a commit to hsiehjackson/NeMo that referenced this pull request Jun 2, 2023
* import parallel_state and tensor_parallel from megatron.core

Signed-off-by: ericharper <[email protected]>

* update column parallel async allreduce arg

Signed-off-by: ericharper <[email protected]>

* typos

Signed-off-by: ericharper <[email protected]>

* play stash + some changes

Signed-off-by: Abhinav Khattar <[email protected]>

* make grad scaler callable

Signed-off-by: ericharper <[email protected]>

* Fixed formatting

Signed-off-by: SeanNaren <[email protected]>

* Make sure RETRO integrates well with the core (NVIDIA#6207)

* fix tests

Signed-off-by: Yi Dong <[email protected]>

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

---------

Signed-off-by: Yi Dong <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* [NLP] Support T5 with Megatron Core (NVIDIA#6222)

* Support T5 with Megatron Core

Signed-off-by: SeanNaren <[email protected]>

* Remove comment

Signed-off-by: SeanNaren <[email protected]>

* Update prediction step

Signed-off-by: SeanNaren <[email protected]>

* Further changes to fix fine-tuning

Signed-off-by: SeanNaren <[email protected]>

* Bug fixes from runs

Signed-off-by: SeanNaren <[email protected]>

* Revert changes to batch sampler, swap to pretrained sampler

Signed-off-by: SeanNaren <[email protected]>

* Address feedback

Signed-off-by: SeanNaren <[email protected]>

---------

Signed-off-by: SeanNaren <[email protected]>

* GPT P-tuning core (max_len pad -> slow)

Signed-off-by: Abhinav Khattar <[email protected]>

* add GPT p-tuning w/ global batch based passing

Signed-off-by: Abhinav Khattar <[email protected]>

* add T5 p-tuning support

Signed-off-by: Abhinav Khattar <[email protected]>

* add megatron core install to Jenkinsfile

Signed-off-by: ericharper <[email protected]>

* fix command

Signed-off-by: ericharper <[email protected]>

* add guard efault for arg

Signed-off-by: ericharper <[email protected]>

* shift bert, retro, adapter + other namespace changes

Signed-off-by: Abhinav Khattar <[email protected]>

* build_model merge into one

Signed-off-by: Abhinav Khattar <[email protected]>

* Ensure fine-tuning/prompt learning work for T5 (NVIDIA#6385)

Signed-off-by: SeanNaren <[email protected]>

* rm extra split impl

Signed-off-by: Abhinav Khattar <[email protected]>

* fix for CI

Signed-off-by: Abhinav Khattar <[email protected]>

* temp change for tests

Signed-off-by: Abhinav Khattar <[email protected]>

* add bs=1 for log

Signed-off-by: Abhinav Khattar <[email protected]>

* fix

Signed-off-by: Abhinav Khattar <[email protected]>

* iter changes NMT

Signed-off-by: Abhinav Khattar <[email protected]>

* NMT partial fix

Signed-off-by: Abhinav Khattar <[email protected]>

* move on_train_batch_end to base_model

Signed-off-by: Abhinav Khattar <[email protected]>

* rm on_train_batch_end

Signed-off-by: Abhinav Khattar <[email protected]>

* temp remove NMT test

Signed-off-by: Abhinav Khattar <[email protected]>

* add training_step logic for T5 derived dynamic len models

Signed-off-by: Abhinav Khattar <[email protected]>

* add NMT test back

Signed-off-by: Abhinav Khattar <[email protected]>

* style fix

Signed-off-by: Abhinav Khattar <[email protected]>

* change no_async_tensor_model_parallel_allreduce

Signed-off-by: Abhinav Khattar <[email protected]>

* sequence_parallel_enabled -> sequence_parallel

Signed-off-by: Abhinav Khattar <[email protected]>

* fix T5 FT batch size

Signed-off-by: Abhinav Khattar <[email protected]>

* seq enabled

Signed-off-by: Abhinav Khattar <[email protected]>

* T5 sequence length fix

Signed-off-by: Abhinav Khattar <[email protected]>

* NMT mp fork to spawn

Signed-off-by: Abhinav Khattar <[email protected]>

* make function signatures consistent across models

Signed-off-by: Abhinav Khattar <[email protected]>

* make print log

Signed-off-by: Abhinav Khattar <[email protected]>

* rm unused import

Signed-off-by: Abhinav Khattar <[email protected]>

* update Dockerfile to install core

Signed-off-by: Abhinav Khattar <[email protected]>

* keep core path in workspace

Signed-off-by: Abhinav Khattar <[email protected]>

---------

Signed-off-by: ericharper <[email protected]>
Signed-off-by: Abhinav Khattar <[email protected]>
Signed-off-by: SeanNaren <[email protected]>
Signed-off-by: Yi Dong <[email protected]>
Co-authored-by: ericharper <[email protected]>
Co-authored-by: SeanNaren <[email protected]>
Co-authored-by: Yi Dong <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Signed-off-by: hsiehjackson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI core Changes to NeMo Core NLP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants