Repository navigation
refactor(ci): split nightly benchmark into per-model jobs - #327
Conversation
What changed: - .github/workflows/nightly-benchmark.yml: Replace monolithic single-sglang and single-vllm jobs with a matrix-based single-worker job (9 models × 2 runtimes). Each model now runs as a separate job with fail-fast: false, allowing failures to be isolated without blocking other models. - e2e_test/benchmarks/test_nightly_perf.py: Enable _TEST_MODE for faster benchmark runs during workflow testing (1 concurrency, 10 requests, 5min timeout instead of full benchmark settings). Why: - Previous workflow ran all models in a single pytest session, making it impossible to debug which model failed and causing the entire workflow to fail on any single model failure. - With per-model jobs, failures are isolated, easier to identify, and don't block other models from running. How: - Unified single-worker and multi-worker jobs to use the same matrix pattern - Added `if: !cancelled()` and `fail-fast: false` to ensure all jobs run - Updated job dependencies: multi-worker depends on single-worker - Temporarily enabled _TEST_MODE and added PR trigger for testing Refs: workflow testing
Summary of ChangesHello @slin1237, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly improves the robustness and maintainability of the nightly benchmark CI pipeline. By isolating each model's benchmark run into a separate job, it becomes much easier to identify and debug failures without impacting the overall benchmark suite. This change streamlines the process of evaluating model performance and ensures that issues are pinpointed efficiently. Highlights
Changelog
Ignored Files
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughThe GitHub Actions nightly benchmark workflow is restructured from separate single-sglang and single-vllm jobs to a unified single-worker job using matrices for models and variants, with conditional filtering and dynamic installation paths based on runtime selection. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Code Review
This pull request refactors the nightly benchmark workflow to run each model in a separate job, which is a great improvement for debuggability and fault isolation. The code change enables a test mode to allow for faster validation of the new workflow. My review includes a suggestion to make this test mode activation more robust by using an environment variable instead of a hardcoded boolean. This will prevent accidentally merging the code with test settings enabled, removing the need for a manual revert step.
| # TODO: Set to False after testing workflow changes | ||
| _TEST_MODE = True |
There was a problem hiding this comment.
While the TODO and checklist item in the PR description are good reminders, relying on a manual step to revert _TEST_MODE to False is error-prone. If this change is accidentally merged, the nightly benchmarks would run with incorrect test settings.
A more robust approach is to control this via an environment variable. This way, the production code remains unchanged, and you can enable test mode specifically in your CI workflow for validation runs by setting the environment variable.
| # TODO: Set to False after testing workflow changes | |
| _TEST_MODE = True | |
| _TEST_MODE = os.environ.get("E2E_BENCHMARK_TEST_MODE", "False").lower() in ("true", "1") |
What changed: - .github/workflows/nightly-benchmark.yml: Restore schedule trigger, remove PR trigger that was added for testing - e2e_test/benchmarks/test_nightly_perf.py: Set _TEST_MODE back to False to run full benchmarks Why: - Workflow testing is complete, reverting to production configuration
Signed-off-by: ppraneth <pranethparuchuri@gmail.com>
Summary
Refactors the nightly benchmark workflow to run each model as a separate job instead of running all models in a single pytest session. This improves debuggability and fault isolation.
Closes: N/A (workflow improvement)
What changed
.github/workflows/nightly-benchmark.yml:single-sglangandsingle-vllmjobs with a unifiedsingle-workermatrix jobfail-fast: falseso one model failure doesn't stop other modelsif: !cancelled()to ensure jobs run even if dependencies failmulti-workernow depends onsingle-workerllama-4-maverick-17bto single-worker matrix (was missing)e2e_test/benchmarks/test_nightly_perf.py:_TEST_MODE = Truefor fast benchmark runs during testingWhy
How
single-workerandmulti-workerjobs-kfilter → upload artifacts → cleanupmodelsandruntimeworkflow_dispatch inputsTest plan
pull_requesttrigger on workflow file changes)_TEST_MODE = TrueBefore merging
_TEST_MODE = Trueback toFalsescheduletriggerpull_requesttriggerSummary by CodeRabbit