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.

Sign up for GitHub

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

Jump to bottom

Fixes bugs with loss averaging with validation_drop_last=False for Megatron GPT #5329

Merged
merged 17 commits into from
Nov 14, 2022
Merged

Fixes bugs with loss averaging with validation_drop_last=False for Megatron GPT #5329

merged 17 commits into from
Nov 14, 2022

Conversation

shanmugamr1992
Copy link
Collaborator

What does this PR do ?

Add a one line overview of what this PR aims to accomplish.

Collection: [Note which collection this PR will affect]

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)

@lgtm-com
Copy link

lgtm-com bot commented Nov 5, 2022

This pull request introduces 1 alert when merging 08e67cf into b47a467 - view on LGTM.com

new alerts:

  • 1 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented Nov 5, 2022

This pull request introduces 1 alert when merging 103a3a3 into b47a467 - view on LGTM.com

new alerts:

  • 1 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented Nov 7, 2022

This pull request introduces 1 alert when merging 60a28c0 into 1826b73 - view on LGTM.com

new alerts:

  • 1 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented Nov 8, 2022

This pull request introduces 1 alert when merging 7a50d5e into d37db88 - view on LGTM.com

new alerts:

  • 1 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented Nov 8, 2022

This pull request introduces 1 alert when merging 15900cd into 83cb869 - view on LGTM.com

new alerts:

  • 1 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented Nov 8, 2022

This pull request introduces 1 alert when merging 27c4227 into 31e0440 - view on LGTM.com

new alerts:

  • 1 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented Nov 8, 2022

This pull request introduces 1 alert when merging 7a1b78b into 8df11ae - view on LGTM.com

new alerts:

  • 1 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented Nov 9, 2022

This pull request introduces 1 alert when merging 1910d93 into 373cbd6 - view on LGTM.com

new alerts:

  • 1 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented Nov 9, 2022

This pull request introduces 2 alerts when merging aafb93b into 373cbd6 - view on LGTM.com

new alerts:

  • 1 for Unused import
  • 1 for Wrong name for an argument in a class instantiation

@lgtm-com
Copy link

lgtm-com bot commented Nov 9, 2022

This pull request introduces 2 alerts when merging 66d4e35 into 265056e - view on LGTM.com

new alerts:

  • 1 for Unused import
  • 1 for Wrong name for an argument in a class instantiation

@lgtm-com
Copy link

lgtm-com bot commented Nov 9, 2022

This pull request introduces 2 alerts when merging 429ab5f into c29d779 - view on LGTM.com

new alerts:

  • 1 for Unused import
  • 1 for Wrong name for an argument in a class instantiation

@lgtm-com
Copy link

lgtm-com bot commented Nov 9, 2022

This pull request introduces 2 alerts when merging 4172a59 into c5c46ba - view on LGTM.com

new alerts:

  • 1 for Unused import
  • 1 for Wrong name for an argument in a class instantiation

@lgtm-com
Copy link

lgtm-com bot commented Nov 9, 2022

This pull request introduces 2 alerts when merging 9a8dd9f into f771f92 - view on LGTM.com

new alerts:

  • 1 for Unused import
  • 1 for Wrong name for an argument in a class instantiation

@MaximumEntropy MaximumEntropy changed the title fixing bugs Fixes bugs with loss averaging with validation_drop_last=False for Megatron GPT Nov 9, 2022
@lgtm-com
Copy link

lgtm-com bot commented Nov 14, 2022

This pull request introduces 2 alerts when merging e85456e into 785057e - view on LGTM.com

new alerts:

  • 1 for Unused import
  • 1 for Wrong name for an argument in a class instantiation

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine ⚙️ that powers LGTM.com. For more information, please check out our post on the GitHub blog.

@lgtm-com
Copy link

lgtm-com bot commented Nov 14, 2022

This pull request introduces 2 alerts when merging 62093ce into 3f2c1ae - view on LGTM.com

new alerts:

  • 1 for Unused import
  • 1 for Wrong name for an argument in a class instantiation

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine ⚙️ that powers LGTM.com. For more information, please check out our post on the GitHub blog.

@shanmugamr1992 shanmugamr1992 merged commit 3499e3f into main Nov 14, 2022
@shanmugamr1992 shanmugamr1992 deleted the bugFixValidation branch November 14, 2022 23:03
hainan-xv pushed a commit to hainan-xv/NeMo that referenced this pull request Nov 29, 2022
@shanmugamr1992 @pre-commit-ci @okuchaiev
Fixes bugs with loss averaging with validation_drop_last=False for …
39541bf 
…Megatron GPT (NVIDIA#5329)

* fixing bugs

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

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

* bug fix

* bug fix

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

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

Co-authored-by: Shanmugam Ramasamy <shanmugamr@shanmugamr-mlt.client.nvidia.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Oleksii Kuchaiev <okuchaiev@users.noreply.github.com>
Signed-off-by: Hainan Xu <hainanx@nvidia.com>
hainan-xv pushed a commit to hainan-xv/NeMo that referenced this pull request Nov 29, 2022
@shanmugamr1992 @pre-commit-ci @okuchaiev
Fixes bugs with loss averaging with validation_drop_last=False for …
8ed33a6 
…Megatron GPT (NVIDIA#5329)

* fixing bugs

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

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

* bug fix

* bug fix

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

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

Co-authored-by: Shanmugam Ramasamy <shanmugamr@shanmugamr-mlt.client.nvidia.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Oleksii Kuchaiev <okuchaiev@users.noreply.github.com>
Signed-off-by: Hainan Xu <hainanx@nvidia.com>
JimmyZhang12 pushed a commit to JimmyZhang12/NeMo that referenced this pull request Dec 14, 2022
@shanmugamr1992 @pre-commit-ci @okuchaiev
Fixes bugs with loss averaging with validation_drop_last=False for …
fa947b4 
…Megatron GPT (NVIDIA#5329)

* fixing bugs

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

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

* bug fix

* bug fix

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

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

Co-authored-by: Shanmugam Ramasamy <shanmugamr@shanmugamr-mlt.client.nvidia.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Oleksii Kuchaiev <okuchaiev@users.noreply.github.com>
andrusenkoau pushed a commit to andrusenkoau/NeMo that referenced this pull request Jan 5, 2023
@shanmugamr1992 @pre-commit-ci
@okuchaiev @andrusenkoau
Fixes bugs with loss averaging with validation_drop_last=False for …
37c9311 
…Megatron GPT (NVIDIA#5329)

* fixing bugs

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

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

* bug fix

* bug fix

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

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

Co-authored-by: Shanmugam Ramasamy <shanmugamr@shanmugamr-mlt.client.nvidia.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Oleksii Kuchaiev <okuchaiev@users.noreply.github.com>
Signed-off-by: andrusenkoau <andrusenkoau@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@anmolgupt anmolgupt anmolgupt left review comments

@yidong72 yidong72 yidong72 approved these changes

@ericharper ericharper Awaiting requested review from ericharper

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@shanmugamr1992 @anmolgupt @yidong72 @okuchaiev