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

Add the strategy argument to MegatronGPTModel.generate() #7264

Merged
merged 1 commit into from
Sep 27, 2023

Conversation

odelalleau
Copy link
Collaborator

@odelalleau odelalleau commented Aug 18, 2023

What does this PR do ?

The motivation is to be able to override the text generation strategy when calling MegatronGPTModel.generate().

It is passed as an explicit argument rather than through **strategy_args so as to ensure someone cannot accidentally pass other arguments that would end up being ignored.

It is a keyword-only argument to ensure that if in the future we want to update the signature to **strategy_args, we can do it without breaking code.

Collection: nlp

Changelog

  • Add the strategy argument to MegatronGPTModel.generate()

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

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 Aug 18, 2023
@odelalleau odelalleau force-pushed the od/strategy-args branch 2 times, most recently from 43681ae to 87d2d9c Compare August 21, 2023 00:28
@odelalleau odelalleau changed the title Add the strategy_args argument to MegatronGPTModel.generate() Add the strategy argument to MegatronGPTModel.generate() Aug 21, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Sep 4, 2023

This PR is stale because it has been open for 14 days with no activity. Remove stale label or comment or update or this will be closed in 7 days.

It is passed as an explicit argument rather than through
`**strategy_args` so as to ensure someone cannot accidentally pass other
arguments that would end up being ignored.

It is a keyword-only argument to ensure that if in the future we want to
update the signature to `**strategy_args`, we can do it without breaking
code.

Signed-off-by: Olivier Delalleau <507137+odelalleau@users.noreply.github.com>
Copy link
Collaborator

@yidong72 yidong72 left a comment

Choose a reason for hiding this comment

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

LGTM

@odelalleau
Copy link
Collaborator Author

Regarding the CodeQL error: CodeQL is out of memory. Try running CodeQL on a larger runner (hosted or self-hosted). If you continue to encounter this issue, contact GitHub Support.

@MaximumEntropy MaximumEntropy merged commit a376063 into NVIDIA:main Sep 27, 2023
5 of 6 checks passed
@odelalleau odelalleau deleted the od/strategy-args branch September 27, 2023 20:36
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.

3 participants