Repository navigation
Conversation
…un.py - Create .github/benchmark/models.json as single source of truth for model configs, shared between benchmark matrix and regression re-run - Create .github/scripts/regression_rerun.py to parse regression report and generate action file (DOWNLOAD/LAUNCH/BENCHMARK/STOP), matching by model_id + variant suffix instead of fragile display name / org guessing - Add model_id field to regression report entries in summarize.py - Add NUM_PROMPTS_OVERRIDE and RESULT_FILENAME fallback to atom_test.sh - Add load-models job to read models.json for benchmark matrix via fromJson - Replace ~130 lines of inline shell+python in regression-analysis with ~40 lines that delegate to regression_rerun.py + atom_test.sh
…heck Boolean inputs.xxx is falsy when false, so `false || 'true'` always evaluates to 'true', making it impossible to disable a model via workflow_dispatch. Input defaults already provide the true fallback.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the benchmark and regression-analysis GitHub Actions workflow to centralize model configuration in a single JSON registry and to replace inline YAML scripting with a dedicated regression re-run planner script.
Changes:
- Add
.github/benchmark/models.jsonas the single source of truth for benchmark model configs, and load it into the benchmark job matrix viafromJson. - Add
.github/scripts/regression_rerun.pyto convertregression_report.json+models.jsoninto a pipe-delimited action plan (DOWNLOAD/LAUNCH/BENCHMARK/STOP) consumed by the workflow. - Extend regression report entries in
summarize.pywithmodel_id, and add benchmark script fallbacks (NUM_PROMPTS_OVERRIDE,RESULT_FILENAME) inatom_test.sh.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| .github/workflows/atom-benchmark.yaml | Loads model configs from models.json for the benchmark matrix and delegates regression re-runs to regression_rerun.py + atom_test.sh. |
| .github/scripts/summarize.py | Adds model_id into regression report entries for robust model matching. |
| .github/scripts/regression_rerun.py | New script that generates regression re-run actions by matching report entries to models.json. |
| .github/scripts/atom_test.sh | Adds RESULT_FILENAME default and NUM_PROMPTS_OVERRIDE support for benchmark runs. |
| .github/benchmark/models.json | New centralized model registry used by both the benchmark matrix and regression re-run tooling. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Comment on lines
+485
to
+492
| if [ -d "/models" ]; then MODEL_DIR="/models/$ARG1"; else MODEL_DIR="$ARG1"; fi | ||
| docker exec atom-regression bash -lc "set -euo pipefail | ||
| pkill -f 'atom.entrypoints' || true; sleep 3 | ||
| .github/scripts/atom_test.sh launch $MODEL_DIR $ARG2" | ||
| ;; | ||
| BENCHMARK) | ||
| echo "=== Profiling: $ARG1 ISL=$ARG2 OSL=$ARG3 CONC=$ARG4 ===" | ||
| if [ -d "/models" ]; then MODEL_DIR="/models/$ARG1"; else MODEL_DIR="$ARG1"; fi |
Comment on lines
+493
to
+498
| docker exec atom-regression bash -lc " | ||
| ENABLE_TORCH_PROFILER=1 \ | ||
| ISL=$ARG2 OSL=$ARG3 CONC=$ARG4 \ | ||
| RANDOM_RANGE_RATIO=0.8 \ | ||
| NUM_PROMPTS_OVERRIDE=\$((${ARG4}*2)) \ | ||
| RESULT_FILENAME=regression-${ARG2}-${ARG3}-${ARG4} \ |
- atom_test.sh: add `stop` subcommand, add `--mark-trace` to launch when ENABLE_TORCH_PROFILER=1 (profiler logic fully in atom_test.sh) - regression_rerun.py: output flat config list (model|args|bench_args| isl|osl|conc) instead of DOWNLOAD/LAUNCH/STOP lifecycle actions; sorted by model+args for server grouping - YAML: split monolithic step into 4 independent steps: Prepare configs → Download models → Run with profiler → Wait traces "Run with profiler" uses same atom_test.sh launch/benchmark/stop pattern as the benchmark job
Contributor
There was a problem hiding this comment.
Pull request overview
This PR centralizes benchmark model configuration in a single JSON registry and refactors the regression re-run flow to consume that registry via a dedicated script, simplifying the GitHub Actions workflow and making regression matching more robust.
Changes:
- Introduce
.github/benchmark/models.jsonand load it inatom-benchmark.yamlto drive the benchmark matrix viafromJson. - Add
.github/scripts/regression_rerun.pyand refactor regression-analysis to generate and execute re-run configs from the regression report + models registry. - Extend
summarize.pyregression entries withmodel_id, and enhanceatom_test.shto support configurable prompt counts and result filenames.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| .github/workflows/atom-benchmark.yaml | Loads models.json for the matrix and delegates regression re-run config generation/execution to regression_rerun.py + atom_test.sh. |
| .github/scripts/summarize.py | Adds model_id into regression report entries to enable stable matching. |
| .github/scripts/regression_rerun.py | New script to select top regressions and output pipe-delimited rerun configs resolved from models.json. |
| .github/scripts/atom_test.sh | Adds stop command, NUM_PROMPTS_OVERRIDE, and RESULT_FILENAME support; updates profiler launch args. |
| .github/benchmark/models.json | New centralized model registry used by both benchmark matrix and regression re-run logic. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| CURRENT_SERVER="" | ||
|
|
||
| while IFS='|' read -r MODEL_PATH SERVER_ARGS BENCH_ARGS ISL OSL CONC; do | ||
| if [ -d "/models" ]; then MODEL_DIR="/models/$MODEL_PATH"; else MODEL_DIR="$MODEL_PATH"; fi |
Comment on lines
+507
to
+511
| docker exec atom-regression bash -lc " | ||
| ENABLE_TORCH_PROFILER=1 \ | ||
| ISL=$ISL OSL=$OSL CONC=$CONC \ | ||
| RANDOM_RANGE_RATIO=0.8 \ | ||
| NUM_PROMPTS_OVERRIDE=\$((${CONC}*2)) \ |
| - name: Download models | ||
| if: steps.prepare.outputs.has_configs == 'true' | ||
| run: | | ||
| cut -d'|' -f1 /tmp/regression_configs.txt | sort -u | while read MODEL_PATH; do |
Comment on lines
+41
to
+50
| for m in models: | ||
| if m["path"] == model_id and m.get("suffix", "") == variant_suffix: | ||
| return m | ||
|
|
||
| # Fallback: match by path basename (for old reports without model_id) | ||
| display_base = re.sub(r"-(mtp\d*)$", "", display, flags=re.IGNORECASE) | ||
| for m in models: | ||
| path_basename = m["path"].split("/")[-1] | ||
| if path_basename == display_base and m.get("suffix", "") == variant_suffix: | ||
| return m |
…analysis - regression-rerun (GPU): only on regression, re-runs configs with profiler - profiler-analysis (ubuntu-latest): runs on regression OR manual enable_profiler - Regression mode: downloads traces from regression-rerun - Manual mode: downloads traces from benchmark jobs - Issue creation only for regression mode This allows manual profiler data collection via workflow_dispatch with enable_profiler=true, without requiring an actual regression.
Stale generated code in ~/.cache/atom/ may reference record_function without importing it, causing NameError when --mark-trace is enabled. Clear cache on every launch to ensure fresh compilation with correct instrumentation.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the benchmark workflow to centralize model configuration in a single registry and simplifies the regression re-run pipeline by moving logic out of the workflow YAML into a dedicated script.
Changes:
- Add
.github/benchmark/models.jsonas the shared model registry and load it into the benchmark job matrix via a newload-modelsjob. - Add
.github/scripts/regression_rerun.pyto select top regressed configs fromregression_report.jsonand emit a flat rerun plan consumed by the workflow. - Extend
.github/scripts/atom_test.shand.github/scripts/summarize.pyto support regression rerun needs (model_idin report entries, prompt/result overrides, and stop support).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| .github/workflows/atom-benchmark.yaml | Loads models.json for the matrix; replaces inline regression-analysis logic with a regression re-run job + profiler analysis job updates. |
| .github/scripts/summarize.py | Adds model_id into regression report entries to enable stable matching in reruns. |
| .github/scripts/regression_rerun.py | New script to map regression report entries to models.json and generate rerun configs. |
| .github/scripts/atom_test.sh | Adds benchmark result filename fallback, prompt override, profiler mark-trace, cache clearing, and a stop action. |
| .github/benchmark/models.json | New centralized model config registry consumed by both the matrix and rerun logic. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| uses: actions/upload-artifact@v7 | ||
| with: | ||
| name: regression-traces-${{ github.run_id }} | ||
| path: regression-traces/ |
Comment on lines
+87
to
+95
| configs.append( | ||
| { | ||
| "model_path": model_cfg["path"], | ||
| "server_args": model_cfg.get("args", ""), | ||
| "bench_args": model_cfg.get("bench_args", ""), | ||
| "isl": r["isl"], | ||
| "osl": r["osl"], | ||
| "conc": r["conc"], | ||
| } |
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
.github/benchmark/models.jsonas single source of truth for model configs, shared between benchmark matrix and regression re-run.github/scripts/regression_rerun.pyto parse regression report and generate action file (DOWNLOAD/LAUNCH/BENCHMARK/STOP), matching bymodel_id+ variant suffix instead of fragile display name / org guessingmodel_idfield to regression report entries insummarize.pyNUM_PROMPTS_OVERRIDEandRESULT_FILENAMEfallback toatom_test.shload-modelsjob to readmodels.jsonfor benchmark matrix viafromJsonregression_rerun.py+atom_test.shKey improvements
Test plan