-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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 pipeline support to eval methods #3684
Conversation
Signed-off-by: ericharper <[email protected]>
Signed-off-by: ericharper <[email protected]>
Signed-off-by: ericharper <[email protected]>
This pull request introduces 3 alerts and fixes 2 when merging ad23ab8 into b466ebc - view on LGTM.com new alerts:
fixed alerts:
|
Signed-off-by: ericharper <[email protected]>
This pull request introduces 3 alerts and fixes 2 when merging ae1b9d4 into b5012d0 - view on LGTM.com new alerts:
fixed alerts:
|
Signed-off-by: ericharper <[email protected]>
Signed-off-by: ericharper <[email protected]>
This pull request introduces 4 alerts and fixes 2 when merging 1aee4e4 into 8ffc92e - view on LGTM.com new alerts:
fixed alerts:
|
This pull request introduces 4 alerts and fixes 2 when merging 0206a4a into 37fe5b4 - view on LGTM.com new alerts:
fixed alerts:
|
Signed-off-by: ericharper <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This pull request introduces 2 alerts and fixes 2 when merging ebed265 into a8f29af - view on LGTM.com new alerts:
fixed alerts:
|
This pull request introduces 2 alerts and fixes 2 when merging 44ec294 into 49183b7 - view on LGTM.com new alerts:
fixed alerts:
|
@@ -194,6 +194,8 @@ def pad_collate(batch): | |||
print("***************************") | |||
print(response) | |||
print("***************************") | |||
if args.prompt and not args.compute_logprobs: | |||
print(f'Prompt: {args.prompt}\n\nResponse: {response[0][0][0]}') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow thats a pretty nested list. Maybe we can make it more structured?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, but not in this PR
This pull request introduces 2 alerts and fixes 2 when merging d26665a into 6dd4263 - view on LGTM.com new alerts:
fixed alerts:
|
* update complete method for pipeline Signed-off-by: ericharper <[email protected]> * broadcast tokens Signed-off-by: ericharper <[email protected]> * print args.prompt and response Signed-off-by: ericharper <[email protected]> * print response Signed-off-by: ericharper <[email protected]> * update compute_logprobs Signed-off-by: ericharper <[email protected]> * style Signed-off-by: ericharper <[email protected]> * remove imports Signed-off-by: ericharper <[email protected]>
What does this PR do ?
Updates
.complete
and.compute_logprobs
to work with pipeline parallelism.Collection: [NLP]
Changelog
Usage
Before your PR is "Ready for review"
Pre checks:
PR Type:
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