diff --git a/.github/PULL_REQUEST_TEMPLATE/pull_request_template.md b/.github/PULL_REQUEST_TEMPLATE/pull_request_template.md index 383466360a..5636e00ddc 100644 --- a/.github/PULL_REQUEST_TEMPLATE/pull_request_template.md +++ b/.github/PULL_REQUEST_TEMPLATE/pull_request_template.md @@ -32,8 +32,8 @@ - [ ] I have completed the AI model disclosure and kept it current - [ ] I have tested my changes locally - [ ] I have updated documentation if necessary -- [ ] **For every change that can affect benchmark performance and every recipe addition or modification, I have appended a new entry to the physical end of `inferencex-e2e/perf-changelog.yaml` and have not edited historical entries** -- [ ] **If this PR can affect benchmark performance or adds or modifies a recipe, it carries exactly one primary sweep label** (a maintainer applies it on fork PRs): `full-sweep-fail-fast` (recommended), `full-sweep-enabled`, or `non-canary-full-sweep-enabled`. Optional modifiers `all-evals`, `evals-only`, and `agentx-fast` require a primary label; the last two block reuse while applied. +- [ ] **If this PR should produce new published results, I have appended one new entry to the physical end of `inferencex-e2e/perf-changelog.yaml`; otherwise I have added none. I have not edited historical entries** +- [ ] **If this PR appends an `inferencex-e2e/perf-changelog.yaml` entry, it carries exactly one primary sweep label** (a maintainer applies it on fork PRs): `full-sweep-fail-fast` (recommended), `full-sweep-enabled`, or `non-canary-full-sweep-enabled`. Optional modifiers `all-evals`, `evals-only`, and `agentx-fast` require a primary label; the last two block reuse while applied. - [ ] **Before merging via reuse, an authorized maintainer (`OWNER`/`MEMBER`/`COLLABORATOR`) has commented `/use ` (or the legacy `/reuse-sweep-run`) on this PR**. Do this **only once there is a final full sweep that is all green with evals passing**, since after this comment the primary sweep label will no longer automatically kick off new sweeps. Remove and re-add the primary sweep label to force a new sweep. diff --git a/.github/workflows/claude.yml b/.github/workflows/claude.yml index c36885c07f..7f34e4afe5 100644 --- a/.github/workflows/claude.yml +++ b/.github/workflows/claude.yml @@ -184,7 +184,7 @@ jobs: - Only wait for the final successful run to complete before analyzing benchmark results - Do NOT claim completion until the most recent job finishes and results are analyzed - If jobs cannot be run, say exactly what you could not run and why - - **Important** Modify inferencex-e2e/perf-changelog.yaml for any config changes affecting performance + - **Important** Append an inferencex-e2e/perf-changelog.yaml entry only when the change should produce new published results (see "Updating perf-changelog.yaml" below) ## Profiling (SGLang only) When asked to profile a config, dispatch the `profile.yml` workflow. **Only SGLang configs can be profiled** — the profiler uses SGLang's `/start_profile` and `/stop_profile` HTTP endpoints. Reject profiling requests for vLLM, TRT, or other frameworks. @@ -234,7 +234,7 @@ jobs: ## Updating perf-changelog.yaml - See `AGENTS.md` → "Non-negotiable benchmark invariants" for changelog rules and `inferencex-e2e/perf-changelog.yaml` for entry format. Required whenever you change image tags, env vars, or perf-affecting params in `inferencex-e2e/configs/*-master.yaml` or `inferencex-e2e/benchmarks/*.sh`. Use `XXX` as the PR-link placeholder until the PR exists. + See `AGENTS.md` → "Non-negotiable benchmark invariants" for changelog rules and `inferencex-e2e/perf-changelog.yaml` for entry format. Add an entry only when the change should produce new published results, not for every config, recipe, or script edit; the entry triggers result collection. If the request does not say whether to collect new results, ask before adding one. Use `XXX` as the PR-link placeholder until the PR exists. If an entry uses `append-only: true`, require the generated base matrix to remain an immutable subset of the generated head matrix. New concurrency values or new recipe variants may be added inside a selected existing config/scenario, but no existing generated point may be removed or modified, and every addition must retain the target visual curve's single non-null image. Do not enforce a file allowlist: supporting code, benchmark scripts, launchers, helpers, and other files may change when their benchmark effect is exclusive to the corresponding newly appended points. All added changelog entries must be append-only, and eval modifiers are forbidden. Trace behavior through the complete diff; unguarded changes or paths reachable by an existing point are blocking. @@ -397,15 +397,16 @@ jobs: - For re-reviews, prefix summary with "Re-review:" and note what changed ## Master Config and Perf Changelog Validation: + A PR adds an `inferencex-e2e/perf-changelog.yaml` entry only when it should produce new published results, not for every config edit; the entry triggers result collection. + When reviewing a PR, check if any of the following master config files were modified: - `inferencex-e2e/configs/amd-master.yaml` - `inferencex-e2e/configs/nvidia-master.yaml` If either master config file was edited AND `inferencex-e2e/perf-changelog.yaml` was NOT edited in the same PR: - - This is a 🔴 **BLOCKING** issue - - Comment that `inferencex-e2e/perf-changelog.yaml` must also be updated when master config files are changed - - The perf-changelog entry should document what changed in the config and include the PR link - - Format: "Master config files were modified but `inferencex-e2e/perf-changelog.yaml` was not updated. When changing `inferencex-e2e/configs/amd-master.yaml` or `inferencex-e2e/configs/nvidia-master.yaml`, you must add a corresponding entry to `inferencex-e2e/perf-changelog.yaml` documenting the changes." + - It is NOT 🔴 **BLOCKING** or 🟡 **WARNING**; do not post an inline comment for it + - Ask once in the summary comment whether the author intends to collect results; append the question even to "LGTM - no blocking issues found" + - Format: "Master config files changed without a new `inferencex-e2e/perf-changelog.yaml` entry. Do you intend to collect new results for these configs? If so, append an entry for the affected config keys to the end of `inferencex-e2e/perf-changelog.yaml`; if not, no entry is needed." ### Perf Changelog Entry Position: `inferencex-e2e/perf-changelog.yaml` is read in chronological order — oldest entries at the top, newest at the bottom. New entries MUST be appended to the END of the file. Never insert in the middle or prepend. diff --git a/AGENTS.md b/AGENTS.md index 5b8dac415e..2605383f2d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -107,8 +107,8 @@ Deleting a test that fails these questions needs no replacement. Do not preserve ## Non-negotiable benchmark invariants - Every priority-scheduled benchmark job on a self-hosted cluster must request exactly one `nodes:N` label, where `N` is the positive integer number of physical Slurm nodes required. Single-node jobs use `nodes:1`; generated multi-node jobs must forward their computed `node-count`. A queued job missing this label is ineligible for priority scheduling, and labels cannot be added retroactively, so fix the source branch and dispatch a new run. -- Every change that can affect benchmark performance and every recipe addition or modification requires a new `inferencex-e2e/perf-changelog.yaml` entry. The file is append-only and byte-sensitive. Preserve all existing bytes and separator whitespace, and append only at the tail. -- One PR maps to one `inferencex-e2e/perf-changelog.yaml` block. By convention, a PR with a perf changelog entry is a change that affects inference performance or otherwise requires a rerun to collect up-to-date results. That block may list one or more `config-keys` to run, but do not add a second block for the same PR, even when the change evolved across commits. Revise the PR's existing block so it describes the final state of the change. +- Add a new `inferencex-e2e/perf-changelog.yaml` entry only when a change should produce new published results, not for every recipe or config edit; the entry triggers result collection. The file is append-only and byte-sensitive. Preserve all existing bytes and separator whitespace, and append only at the tail. +- One PR maps to at most one `inferencex-e2e/perf-changelog.yaml` block. That block may list one or more `config-keys` to run, but do not add a second block for the same PR, even when the change evolved across commits. Revise the PR's existing block so it describes the final state of the change. - New `inferencex-e2e/perf-changelog.yaml` entries must be English-only. Do not add Chinese translations or bilingual descriptions; the bilingual README and GitHub-content rules do not apply to these entries. Leave historical entries unchanged. - Multi-node srt-slurm changes update the recipe YAML and matching master config together. For image bumps, `model.container` must equal `image`. - Every speculative fixed-sequence benchmark renders prompts with the chat template: single-node srt-slurm recipes that speculate set `benchmark.env.USE_CHAT_TEMPLATE: "true"` (enforced by `inferencex-e2e/infx/srt_slurm/single_node.py::validate_recipe`), which `python3 -m infx.bench fixed-seq` (run by `srt_fixed_sequence.sh`) turns into `--use-chat-template` for the benchmark client. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e0a5857ba5..027a18b845 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -27,7 +27,7 @@ Every PR description must include an **AI model disclosure** section inside a co 3. Ping a core maintainer on Slack for final approval, after obtaining the checklist sign-off when required. 4. An authorized maintainer posts `/use ` (see below) and the PR is merged via the reuse path. -**Performance changelog requirement:** Every change that can affect benchmark performance and every recipe addition or modification **MUST** append a new entry to the physical end of `inferencex-e2e/perf-changelog.yaml`. Historical entries **MUST NOT** be edited. +**Performance changelog:** Append a new entry to the physical end of `inferencex-e2e/perf-changelog.yaml` only when the PR should produce new published results, not for every recipe or config edit; the entry triggers result collection. Historical entries **MUST NOT** be edited. ## Draft-model precision diff --git a/inferencex-e2e/benchmarks/multi_node/srt-slurm-recipes/RECIPES.md b/inferencex-e2e/benchmarks/multi_node/srt-slurm-recipes/RECIPES.md index 22cedceac7..5b4126bc3b 100644 --- a/inferencex-e2e/benchmarks/multi_node/srt-slurm-recipes/RECIPES.md +++ b/inferencex-e2e/benchmarks/multi_node/srt-slurm-recipes/RECIPES.md @@ -71,4 +71,4 @@ The initial migration also resolves compatibility issues that `srtctl migrate` c - The power reader accepts both generations of samples CSV while validating utilization values and continuing to compute board energy from watts. - Post-eval selection uses native `post_eval.command` and `post_eval.passthrough_env` with [`srt_eval.sh`](../srt_eval.sh). TRT AgentX recipes declare their existing Dynamo fork with `dynamo.source.git`; launchers no longer rewrite the srt-slurm source. -Append a new entry to the physical end of `perf-changelog.yaml` for every recipe or runtime change. Preserve all historical bytes. Validate the PR with `full-sweep-fail-fast`, including evals, before following the repository's review and artifact-reuse merge process. +Append a new entry to the physical end of `perf-changelog.yaml` only when the change should produce new published results, not for every recipe or runtime edit; the entry triggers result collection. Preserve all historical bytes. When you append an entry, validate the PR with `full-sweep-fail-fast`, including evals, before following the repository's review and artifact-reuse merge process.