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 goodput CLI doc which was missing from the main Readme for Genai-Perf. #273

Merged
merged 2 commits into from
Feb 5, 2025
Merged

Add goodput CLI doc which was missing from the main Readme for Genai-Perf. #273

merged 2 commits into from
Feb 5, 2025

Conversation

ganeshku1
Copy link
Collaborator

Add goodput CLI doc which was missing from the main Readme for Genai-Perf.

dyastremsky
dyastremsky reviewed Feb 5, 2025
View reviewed changes
Copy link
Contributor

@dyastremsky dyastremsky left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for catching this missing arg and making this change!

These help messages should match the parser.py messages exactly. Also, please specify the default value of None. And the value for this flag should be list, not options. You can look to other --model as a reference, since that flag functions similarly.

Also, any changes made here need to also be made to README_template.md. Finally, please break this into multiple lines so that no single line is greater than 80 characters.

@ganeshku1
Copy link
Collaborator Author

Thanks for catching this missing arg and making this change!

These help messages should match the parser.py messages exactly. Also, please specify the default value of None. And the value for this flag should be list, not options. You can look to other --model as a reference, since that flag functions similarly.

Also, any changes made here need to also be made to README_template.md. Finally, please break this into multiple lines so that no single line is greater than 80 characters.

@dyastremsky updated the the PR with this feedback included.
When you get a chance please feel free to review.

dyastremsky
dyastremsky reviewed Feb 5, 2025
View reviewed changes
Copy link
Contributor

@dyastremsky dyastremsky left a comment

Choose a reason for hiding this comment

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

Great updates! A couple of last comments.

genai-perf/README.md Outdated Show resolved Hide resolved
genai-perf/README.md Show resolved Hide resolved
dyastremsky
dyastremsky reviewed Feb 5, 2025
View reviewed changes
genai-perf/README.md Outdated Show resolved Hide resolved
@dyastremsky
Copy link
Contributor

Excellent work, Ganesh!

@ganeshku1 ganeshku1 merged commit 1496cdb into main Feb 5, 2025
6 checks passed
@ganeshku1 ganeshku1 deleted the addGoodPutDoc branch February 5, 2025 21:01
@ganeshku1
Copy link
Collaborator Author

Excellent work, Ganesh!

Thank you @dyastremsky

lkomali pushed a commit that referenced this pull request Feb 5, 2025
@ganeshku1 @lkomali
Add goodput CLI doc which was missing from the main Readme for Genai-…
b0e2232 
…Perf. (#273)

* Add gooput CLI doc which was missing from the main
Readme for Genai-Perf.

* Updated README_template file.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dyastremsky dyastremsky dyastremsky 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.
2 participants
@ganeshku1 @dyastremsky