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

add offsets calculation to MegatronGPTModel.complete method #3117

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 [email protected]

@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
View reviewed changes
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

@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

ericharper
ericharper reviewed Nov 3, 2021
View reviewed changes
examples/nlp/language_modeling/megatron_gpt_eval.py Outdated
@@ -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 •
edited
Loading

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"

ericharper
ericharper requested changes Nov 3, 2021
View reviewed changes
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

@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

@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

@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

@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

@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

ericharper
ericharper approved these changes Nov 12, 2021
View reviewed changes
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!

@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
@dimapihtar @okuchaiev
@ericharper @titu1994
add offsets calculation to MegatronGPTModel.complete method (NVIDIA#3117
b63126b 
)

* add offsets calculation to MegatronGPTModel.complete method

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

* decrease MegatronGPT model evaluation time

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

* decrease MegatronGPT model evaluation time

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

* decrease MegatronGPT model evaluation time

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

* decrease MegatronGPT model evaluation time

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

Co-authored-by: dimapihtar <[email protected]>
Co-authored-by: Oleksii Kuchaiev <[email protected]>
Co-authored-by: Eric Harper <[email protected]>
jfsantos pushed a commit to jfsantos/NeMo that referenced this pull request Nov 19, 2021
@dimapihtar @okuchaiev
@ericharper @jfsantos
add offsets calculation to MegatronGPTModel.complete method (NVIDIA#3117
135ab71 
)

* add offsets calculation to MegatronGPTModel.complete method

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

* decrease MegatronGPT model evaluation time

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

* decrease MegatronGPT model evaluation time

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

* decrease MegatronGPT model evaluation time

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

* decrease MegatronGPT model evaluation time

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

Co-authored-by: dimapihtar <[email protected]>
Co-authored-by: Oleksii Kuchaiev <[email protected]>
Co-authored-by: Eric Harper <[email protected]>
ekmb pushed a commit that referenced this pull request Nov 22, 2021
@dimapihtar @okuchaiev
@ericharper @ekmb
add offsets calculation to MegatronGPTModel.complete method (#3117)
a5b177e 
* add offsets calculation to MegatronGPTModel.complete method

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

* decrease MegatronGPT model evaluation time

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

* decrease MegatronGPT model evaluation time

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

* decrease MegatronGPT model evaluation time

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

* decrease MegatronGPT model evaluation time

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

Co-authored-by: dimapihtar <[email protected]>
Co-authored-by: Oleksii Kuchaiev <[email protected]>
Co-authored-by: Eric Harper <[email protected]>
jasonjjl1999 pushed a commit to jasonjjl1999/NeMo that referenced this pull request Nov 24, 2021
@dimapihtar @okuchaiev
@ericharper @jasonjjl1999
add offsets calculation to MegatronGPTModel.complete method (NVIDIA#3117
e3c01d7 
)

* add offsets calculation to MegatronGPTModel.complete method

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

* decrease MegatronGPT model evaluation time

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

* decrease MegatronGPT model evaluation time

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

* decrease MegatronGPT model evaluation time

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

* decrease MegatronGPT model evaluation time

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

Co-authored-by: dimapihtar <[email protected]>
Co-authored-by: Oleksii Kuchaiev <[email protected]>
Co-authored-by: Eric Harper <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@okuchaiev okuchaiev okuchaiev left review comments

@ericharper ericharper ericharper approved these changes

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
3 participants
@dimapihtar @ericharper @okuchaiev