Skip to content

refactor: extract CI regression-rerun into models.json + regression_rerun.py - #341

Merged
valarLip merged 7 commits into
mainfrom
refactor/ci-regression-rerun
Mar 16, 2026
Merged

valarLip merged 7 commits into
mainfrom
refactor/ci-regression-rerun

Conversation

@valarLip

Copy link
Copy Markdown
Collaborator

Summary

Extract regression re-run logic from inline shell into structured scripts.

Changes

  • models.json — single source of truth for model configs
  • regression_rerun.py — parse regression report + generate action file
  • atom_test.sh — add NUM_PROMPTS_OVERRIDE, compile cache clear
  • summarize.py — add model_id to regression report
  • atom-benchmark.yaml — split regression-analysis into rerun + profiler-analysis

5 commits, 5 files. No model code changes.

Split from #334 (CI part only).

Copilot AI review requested due to automatic review settings March 16, 2026 05:58
@valarLip
valarLip marked this pull request as draft March 16, 2026 05:59

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Refactors the benchmark workflow’s regression re-run path by moving model configuration and rerun-selection logic out of inline shell and into structured, reusable files/scripts.

Changes:

  • Centralizes benchmark model definitions in .github/benchmark/models.json and loads them into the workflow matrix dynamically.
  • Adds .github/scripts/regression_rerun.py to select top-N regressed configs from regression_report.json and emit a flat rerun list for profiler collection.
  • Updates CI scripts/workflow to support profiler reruns and trace analysis, including adding model_id into regression reports and extending atom_test.sh controls.

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 from models.json, adds GPU regression rerun job + separate profiler analysis job, and wires rerun config generation via regression_rerun.py.
.github/scripts/summarize.py Adds model_id to structured regression entries to enable reliable model matching downstream.
.github/scripts/regression_rerun.py New helper script to map regressions to models.json and generate rerun configs for the workflow loop.
.github/scripts/atom_test.sh Adds compile-cache clearing on launch, stop command, and NUM_PROMPTS_OVERRIDE/RESULT_FILENAME handling for reruns.
.github/benchmark/models.json New single source of truth for benchmark model matrix entries and rerun resolution.

💡 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 thread .github/scripts/atom_test.sh Resolved
Comment on lines 674 to 676
### Next Steps
1. Download \`regression-traces-${context.runId}\` artifact
1. Download \`profiler-analysis-${context.runId}\` artifact
2. Open trace files in Perfetto UI
…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.
- 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
…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.
@valarLip
valarLip force-pushed the refactor/ci-regression-rerun branch from 82973bf to 7a49c09 Compare March 16, 2026 12:52
- atom_test.sh stop: wait for trace .tmp completion before killing
  server, then wait for GPU memory release with force-kill fallback
- Per-config trace directories to prevent overwrites when multiple
  configs are profiled in sequence
- Per-config error tolerance (no -e in loop): failed configs are
  skipped with warnings instead of aborting the entire re-run
- Pass env_vars and prefix from models.json through regression_rerun.py
  so model-specific env vars are applied during re-run
- RESULT_FILENAME includes model prefix to avoid overwrites
- NUM_PROMPTS_OVERRIDE computed on host side (no mixed shell expansion)
- profiler-analysis condition relaxed to run on partial success
- profiler-analysis adapted for per-config trace subdirectories
@valarLip
valarLip marked this pull request as ready for review March 16, 2026 14:10
Copilot AI review requested due to automatic review settings March 16, 2026 14:10

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Refactors the benchmark/regression CI workflow to centralize model configs in .github/benchmark/models.json and move regression re-run selection into a dedicated script (regression_rerun.py), while updating the profiling/analysis pipeline and benchmark helper script (atom_test.sh).

Changes:

  • Add .github/benchmark/models.json as the single source of truth for benchmark model entries and load it into the workflow matrix.
  • Add .github/scripts/regression_rerun.py to select top regressed configs from regression_report.json and generate a flat rerun config list for CI.
  • Update CI workflow and helper scripts to support regression rerun profiling, trace handling, and new stop/NUM_PROMPTS_OVERRIDE behavior.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
.github/workflows/atom-benchmark.yaml Loads models from models.json, adds regression rerun + profiler analysis split, updates regression issue generation.
.github/scripts/summarize.py Adds model_id to regression report entries.
.github/scripts/regression_rerun.py New script to map regressions to models.json entries and output rerun configs.
.github/scripts/atom_test.sh Adds compile cache clearing, --mark-trace, stop command, and prompt count override for benchmarks.
.github/benchmark/models.json New centralized model registry used by workflow matrix and rerun script.

💡 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 thread .github/scripts/regression_rerun.py Resolved
Comment thread .github/scripts/atom_test.sh Resolved
Comment thread .github/scripts/atom_test.sh Resolved
Comment thread .github/workflows/atom-benchmark.yaml Resolved
Comment thread .github/workflows/atom-benchmark.yaml Resolved
Comment thread .github/workflows/atom-benchmark.yaml Resolved
Comment thread .github/workflows/atom-benchmark.yaml Resolved
Review fixes:
- Fix performance_summary.md path in GitHub Issue (read from per-config
  subdirectory instead of non-existent flat path)
- Decouple Issue creation from regression-rerun success (file issues
  even when GPU re-run job fails)
- Add defensive .get() for isl/osl/conc in regression_rerun.py to
  avoid KeyError on malformed regression entries

Cleanup:
- Remove ATOM_GPT_OSS_MODEL=1 from atom-test.yaml and ci-pr-guide.md
  (no code reads this env var; confirmed grep across ATOM, aiter, and
  all installed Python packages)
@valarLip
valarLip merged commit cbca4c1 into main Mar 16, 2026
24 of 25 checks passed
@valarLip
valarLip deleted the refactor/ci-regression-rerun branch March 16, 2026 15:50
Jasen2201 pushed a commit to Jasen2201/ATOM that referenced this pull request Apr 10, 2026
…erun.py (ROCm#341)

* refactor: extract regression re-run into models.json + regression_rerun.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

* fix: remove || 'true' that swallowed explicit false in model enable check

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.

* refactor: split regression re-run into prepare/download/run/wait steps

- 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

* refactor: split regression-analysis into regression-rerun + profiler-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.

* fix: clear compile cache before server launch in atom_test.sh

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.

* fix: improve regression re-run reliability and trace capture

- atom_test.sh stop: wait for trace .tmp completion before killing
  server, then wait for GPU memory release with force-kill fallback
- Per-config trace directories to prevent overwrites when multiple
  configs are profiled in sequence
- Per-config error tolerance (no -e in loop): failed configs are
  skipped with warnings instead of aborting the entire re-run
- Pass env_vars and prefix from models.json through regression_rerun.py
  so model-specific env vars are applied during re-run
- RESULT_FILENAME includes model prefix to avoid overwrites
- NUM_PROMPTS_OVERRIDE computed on host side (no mixed shell expansion)
- profiler-analysis condition relaxed to run on partial success
- profiler-analysis adapted for per-config trace subdirectories

* fix: address review findings and remove dead ATOM_GPT_OSS_MODEL env var

Review fixes:
- Fix performance_summary.md path in GitHub Issue (read from per-config
  subdirectory instead of non-existent flat path)
- Decouple Issue creation from regression-rerun success (file issues
  even when GPU re-run job fails)
- Add defensive .get() for isl/osl/conc in regression_rerun.py to
  avoid KeyError on malformed regression entries

Cleanup:
- Remove ATOM_GPT_OSS_MODEL=1 from atom-test.yaml and ci-pr-guide.md
  (no code reads this env var; confirmed grep across ATOM, aiter, and
  all installed Python packages)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants