Fix a performance comparison issue in Benchmark Suite #23047
Conversation
|
👋 Hi! Thank you for contributing to the vLLM project. 💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels. Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging. To run CI, PR reviewers can either: Add 🚀 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request significantly improves the benchmark comparison script by aligning results based on key columns instead of relying on row order, which makes the comparison much more robust. The new logic is well-structured and handles various edge cases. I have a few suggestions to further refine the implementation, focusing on using up-to-date library APIs, removing redundant code, and clarifying the usage of the comparison script in the CI workflow. Overall, this is a great enhancement.
.buildkite/nightly-benchmarks/scripts/run-performance-benchmarks.sh
Outdated
Show resolved
Hide resolved
louie-tsai
changed the title
louie-tsai
force-pushed
the
benmark_suite_fixes
branch
from
d84e507 to
493b0d0
Compare
There was a problem hiding this comment.
Perhaps we can add an ENV for exporting comparison reports, likes EXPORT_COMPARISON. Looks like it is just needed when doing interactive benchmarking.
There was a problem hiding this comment.
removed it from run script, and leave it for interactive performance comparison only.
louie-tsai
force-pushed
the
benmark_suite_fixes
branch
3 times, most recently
from
32b3513 to
06360cf
Compare
Signed-off-by: Tsai, Louie <louie.tsai@intel.com>
Signed-off-by: Tsai, Louie <louie.tsai@intel.com>
Signed-off-by: Tsai, Louie <louie.tsai@intel.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Signed-off-by: Louie Tsai <louie.tsai@intel.com>
Co-authored-by: Li, Jiang <bigpyj64@gmail.com> Signed-off-by: Louie Tsai <louie.tsai@intel.com>
louie-tsai
force-pushed
the
benmark_suite_fixes
branch
from
06360cf to
04322bb
Compare
github-actions
bot
added
the
ready
…3047) Signed-off-by: Tsai, Louie <louie.tsai@intel.com> Signed-off-by: Louie Tsai <louie.tsai@intel.com> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Co-authored-by: Li, Jiang <bigpyj64@gmail.com>
…3047) Signed-off-by: Tsai, Louie <louie.tsai@intel.com> Signed-off-by: Louie Tsai <louie.tsai@intel.com> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Co-authored-by: Li, Jiang <bigpyj64@gmail.com>
…3047) Signed-off-by: Tsai, Louie <louie.tsai@intel.com> Signed-off-by: Louie Tsai <louie.tsai@intel.com> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Co-authored-by: Li, Jiang <bigpyj64@gmail.com> Signed-off-by: Duncan Moss <djm.moss@gmail.com>
…3047) Signed-off-by: Tsai, Louie <louie.tsai@intel.com> Signed-off-by: Louie Tsai <louie.tsai@intel.com> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Co-authored-by: Li, Jiang <bigpyj64@gmail.com>
…3047) Signed-off-by: Tsai, Louie <louie.tsai@intel.com> Signed-off-by: Louie Tsai <louie.tsai@intel.com> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Co-authored-by: Li, Jiang <bigpyj64@gmail.com> Signed-off-by: Xiao Yu <xiao.yu@amd.com>
…3047) Signed-off-by: Tsai, Louie <louie.tsai@intel.com> Signed-off-by: Louie Tsai <louie.tsai@intel.com> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Co-authored-by: Li, Jiang <bigpyj64@gmail.com>
…3047) Signed-off-by: Tsai, Louie <louie.tsai@intel.com> Signed-off-by: Louie Tsai <louie.tsai@intel.com> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Co-authored-by: Li, Jiang <bigpyj64@gmail.com>
…3047) Signed-off-by: Tsai, Louie <louie.tsai@intel.com> Signed-off-by: Louie Tsai <louie.tsai@intel.com> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Co-authored-by: Li, Jiang <bigpyj64@gmail.com>
…3047) Signed-off-by: Tsai, Louie <louie.tsai@intel.com> Signed-off-by: Louie Tsai <louie.tsai@intel.com> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Co-authored-by: Li, Jiang <bigpyj64@gmail.com>
2 participants
Purpose
For different benchmark_results.json comparison. there is model mismatch issue in the comparison table.
this PR fixes the issue to have correct comparison among different benchmark_results.json
Without the fix, it might compare performance among different models
After the fix, it will only compare performance under the same model
Test Plan
Manually Test it.
python3 .buildkite/nightly-benchmarks/scripts/compare-json-results.py -f A/benchmark_results.json B/benchmark_results.jsonTest Result
(Optional) Documentation Update
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.