-
Notifications
You must be signed in to change notification settings - Fork 54
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 t4 for llm perf leaderboard #238
Conversation
workflow_dispatch: # Manual trigger | ||
release: # Trigger on new release | ||
types: [published] |
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.
I don't think this needs commenting, and why on release ?
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.
Ok, I can remove the comments
Good question. I think it would be more efficient to run the full benchmark with each release of the pip package, rather than on a daily basis. Running it daily seems wasteful, as the hardware remains unchanged, and we’re simply repeating the benchmark for every code change. Since users are likely to benchmark using the PyPI package, it makes more sense to align this workflow with each release. We could also run them manually if we discover any issues with our benchmarks. However, if you prefer running the benchmark daily, I can revert to that schedule. Just let me know your preference
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.
i guess there's a misunderstanding. the daily trigger runs different benchmarks (different model+opt+quant) each time because it skips already benchmarked configurations. it is also a way to benchmark all configurations without being limited by the 6 hours time constraint of runners.
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.
Thanks for the explanation, it makes much more sense now. I removed the release schedule and left the original one
Summary
This PR adds Nvidia T4 to the LLM-Perf Leaderboard:
Fixes
trust_remote_code
issue that was broken for the CI/CD pipeline.Features
runs-on
definition (without tags).Related PRs and Discussions
Workflow Trigger Changes
One thing that I am unsure about is that I modified the trigger for the workflow. To reduce unnecessary compute, you can manually trigger the workflow, and it is triggered with each new release of the repo (releases).
I think this could be better. What do you think, @IlyasMoutawwakil?