Skip to content

add offsets calculation to MegatronGPTModel.complete method #3117

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

Merged
merged 11 commits into from
Nov 13, 2021
Merged

add offsets calculation to MegatronGPTModel.complete method #3117

merged 11 commits into from
Nov 13, 2021

Conversation

dimapihtar
Copy link
Collaborator

Signed-off-by: dimapihtar dpykhtar@nvidia.com

Signed-off-by: dimapihtar <dpykhtar@nvidia.com>
Signed-off-by: dimapihtar <dpykhtar@nvidia.com>
Signed-off-by: dimapihtar <dpykhtar@nvidia.com>
@lgtm-com
Copy link

lgtm-com bot commented Nov 2, 2021

This pull request introduces 1 alert when merging 863ab71 into 663c76a - view on LGTM.com

new alerts:

  • 1 for Unused import

okuchaiev
okuchaiev previously approved these changes Nov 2, 2021
Copy link
Member

@okuchaiev okuchaiev left a comment

Choose a reason for hiding this comment

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

lgtm

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@lgtm-com
Copy link

lgtm-com bot commented Nov 2, 2021

This pull request introduces 1 alert when merging 060dd9b into 663c76a - view on LGTM.com

new alerts:

  • 1 for Unused import

@@ -33,7 +33,7 @@ def main():
parser = ArgumentParser()
parser.add_argument("--model_file", type=str, default="", required=True, help="Pass path to model's .nemo file")
parser.add_argument(
"--prompt", type=str, default="", required=True, help="Prompt for the model (a text to complete)"
"--path_to_file", type=str, default="", required=True, help="Path to file with prompts (a text to complete)"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we have to remove --prompt ? It's nice to be able to feed in some text from the CLI.

Copy link
Collaborator Author

@dimapihtar dimapihtar Nov 3, 2021

Choose a reason for hiding this comment

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

@ericharper Should I create a separate .py example for evaluation prompts from file?

Copy link
Member

Choose a reason for hiding this comment

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

@dimapihtar let's just have both "--prompt" and "--path_to_file" options and require at least one to be specified. If both are specified, use "--prompt" and ignore "--path_to_file"

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.

There's a binary file that needs to be removed from the PR: examples/nlp/language_modeling/.megatron_gpt_eval.py.swp

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@lgtm-com
Copy link

lgtm-com bot commented Nov 5, 2021

This pull request introduces 1 alert when merging a81e484 into 875f544 - view on LGTM.com

new alerts:

  • 1 for Unused import

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@lgtm-com
Copy link

lgtm-com bot commented Nov 8, 2021

This pull request introduces 1 alert when merging c8d926a into dc9ed88 - view on LGTM.com

new alerts:

  • 1 for Unused import

Signed-off-by: dimapihtar <dpykhtar@nvidia.com>
Signed-off-by: dimapihtar <dpykhtar@nvidia.com>
@dimapihtar dimapihtar requested a review from ericharper November 8, 2021 14:26
@lgtm-com
Copy link

lgtm-com bot commented Nov 8, 2021

This pull request introduces 1 alert when merging 936e4f9 into dc9ed88 - view on LGTM.com

new alerts:

  • 1 for Unused import

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@lgtm-com
Copy link

lgtm-com bot commented Nov 10, 2021

This pull request introduces 1 alert when merging 3824177 into 1106ff9 - view on LGTM.com

new alerts:

  • 1 for Unused import

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@lgtm-com
Copy link

lgtm-com bot commented Nov 11, 2021

This pull request introduces 1 alert when merging aadbdd1 into b7a175b - view on LGTM.com

new alerts:

  • 1 for Unused import

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!

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@lgtm-com
Copy link

lgtm-com bot commented Nov 12, 2021

This pull request introduces 1 alert when merging 44f46bc into 583aa6a - view on LGTM.com

new alerts:

  • 1 for Unused import

@okuchaiev okuchaiev merged commit e987e17 into NVIDIA:main Nov 13, 2021
titu1994 pushed a commit to titu1994/NeMo that referenced this pull request Nov 18, 2021
)

* add offsets calculation to MegatronGPTModel.complete method

Signed-off-by: dimapihtar <dpykhtar@nvidia.com>

* decrease MegatronGPT model evaluation time

Signed-off-by: dimapihtar <dpykhtar@nvidia.com>

* decrease MegatronGPT model evaluation time

Signed-off-by: dimapihtar <dpykhtar@nvidia.com>

* decrease MegatronGPT model evaluation time

Signed-off-by: dimapihtar <dpykhtar@nvidia.com>

* decrease MegatronGPT model evaluation time

Signed-off-by: dimapihtar <dpykhtar@nvidia.com>

Co-authored-by: dimapihtar <dpykhtar@nvidia.com>
Co-authored-by: Oleksii Kuchaiev <okuchaiev@users.noreply.github.com>
Co-authored-by: Eric Harper <complex451@gmail.com>
jfsantos pushed a commit to jfsantos/NeMo that referenced this pull request Nov 19, 2021
)

* add offsets calculation to MegatronGPTModel.complete method

Signed-off-by: dimapihtar <dpykhtar@nvidia.com>

* decrease MegatronGPT model evaluation time

Signed-off-by: dimapihtar <dpykhtar@nvidia.com>

* decrease MegatronGPT model evaluation time

Signed-off-by: dimapihtar <dpykhtar@nvidia.com>

* decrease MegatronGPT model evaluation time

Signed-off-by: dimapihtar <dpykhtar@nvidia.com>

* decrease MegatronGPT model evaluation time

Signed-off-by: dimapihtar <dpykhtar@nvidia.com>

Co-authored-by: dimapihtar <dpykhtar@nvidia.com>
Co-authored-by: Oleksii Kuchaiev <okuchaiev@users.noreply.github.com>
Co-authored-by: Eric Harper <complex451@gmail.com>
ekmb pushed a commit that referenced this pull request Nov 22, 2021
* add offsets calculation to MegatronGPTModel.complete method

Signed-off-by: dimapihtar <dpykhtar@nvidia.com>

* decrease MegatronGPT model evaluation time

Signed-off-by: dimapihtar <dpykhtar@nvidia.com>

* decrease MegatronGPT model evaluation time

Signed-off-by: dimapihtar <dpykhtar@nvidia.com>

* decrease MegatronGPT model evaluation time

Signed-off-by: dimapihtar <dpykhtar@nvidia.com>

* decrease MegatronGPT model evaluation time

Signed-off-by: dimapihtar <dpykhtar@nvidia.com>

Co-authored-by: dimapihtar <dpykhtar@nvidia.com>
Co-authored-by: Oleksii Kuchaiev <okuchaiev@users.noreply.github.com>
Co-authored-by: Eric Harper <complex451@gmail.com>
jasonjjl1999 pushed a commit to jasonjjl1999/NeMo that referenced this pull request Nov 24, 2021
)

* add offsets calculation to MegatronGPTModel.complete method

Signed-off-by: dimapihtar <dpykhtar@nvidia.com>

* decrease MegatronGPT model evaluation time

Signed-off-by: dimapihtar <dpykhtar@nvidia.com>

* decrease MegatronGPT model evaluation time

Signed-off-by: dimapihtar <dpykhtar@nvidia.com>

* decrease MegatronGPT model evaluation time

Signed-off-by: dimapihtar <dpykhtar@nvidia.com>

* decrease MegatronGPT model evaluation time

Signed-off-by: dimapihtar <dpykhtar@nvidia.com>

Co-authored-by: dimapihtar <dpykhtar@nvidia.com>
Co-authored-by: Oleksii Kuchaiev <okuchaiev@users.noreply.github.com>
Co-authored-by: Eric Harper <complex451@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants