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

Token count and sequence length logging for MegatronGPTSFTModel #8136

Merged
merged 1 commit into from
Jan 14, 2024

Conversation

vysarge
Copy link
Contributor

@vysarge vysarge commented Jan 8, 2024

What does this PR do ?

Adds an option to enable progress bar logging for sequence length (including padding tokens) and real token count for each train step with MegatronGPTSFTModel to enable accurate calculation of tokens/sec and related metrics.

Collection: nlp/language_modeling

Changelog

  • Alters GPTSFTDataset and GPTSFTPackedDataset to pass out a list of token counts as part of the batch
  • Alters MegatronGPTSFTModel to expect, average, and log that list of token counts at the beginning of the step if enabled via config

Usage

python megatron_gpt_peft_tuning.py \
    ... \
    ++model.log_token_counts=true

Jenkins CI

To run Jenkins, a NeMo User with write access must comment jenkins on the PR.

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.

@github-actions github-actions bot added the NLP label Jan 8, 2024
@vysarge vysarge marked this pull request as ready for review January 9, 2024 00:05
@vysarge vysarge changed the title [Draft] Token count and sequence length logging for MegatronGPTSFTModel Token count and sequence length logging for MegatronGPTSFTModel Jan 9, 2024
@vysarge
Copy link
Contributor Author

vysarge commented Jan 9, 2024

@cuichenx / @ericharper can you review?

Copy link
Collaborator

@erhoo82 erhoo82 left a comment

Choose a reason for hiding this comment

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

LGTM

@cuichenx
Copy link
Collaborator

jenkins

Copy link
Collaborator

@cuichenx cuichenx left a comment

Choose a reason for hiding this comment

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

+1 LGTM Thanks

@erhoo82
Copy link
Collaborator

erhoo82 commented Jan 11, 2024

jenkins

@ericharper
Copy link
Collaborator

jenkins

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 1fede57 into NVIDIA:main Jan 14, 2024
11 checks passed
ppisljar pushed a commit to ppisljar/NeMo that referenced this pull request Jan 16, 2024
…s a config option (NVIDIA#8136)

Signed-off-by: Valerie Sarge <[email protected]>
Signed-off-by: ppisljar <[email protected]>
minitu pushed a commit to minitu/NeMo that referenced this pull request Jan 19, 2024
ssh-meister pushed a commit to ssh-meister/NeMo that referenced this pull request Feb 15, 2024
…s a config option (NVIDIA#8136)

Signed-off-by: Valerie Sarge <[email protected]>
Signed-off-by: Sasha Meister <[email protected]>
@vysarge vysarge deleted the vsarge/seqlen_logging branch March 12, 2024 22:36
pablo-garay pushed a commit that referenced this pull request Mar 19, 2024
…s a config option (#8136)

Signed-off-by: Valerie Sarge <[email protected]>
Signed-off-by: Pablo Garay <[email protected]>
rohitrango pushed a commit to rohitrango/NeMo that referenced this pull request Jun 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants