diff --git a/.github/codeowner-signoff-verify-prompt.md b/.github/codeowner-signoff-verify-prompt.md index f3a43222a1..d9f910f3ef 100644 --- a/.github/codeowner-signoff-verify-prompt.md +++ b/.github/codeowner-signoff-verify-prompt.md @@ -19,7 +19,7 @@ You are an automated merge-gate auditor for InferenceX. A CODEOWNER (`${SIGNOFF_AUTHOR}`) just posted the reviewer sign-off checklist (as a ${SIGNOFF_KIND}) that marks PR #${PR_NUMBER} as ready to merge. Your job is to -INDEPENDENTLY verify the checks below (0-10). Do not trust the reviewer's checkmarks. +INDEPENDENTLY verify the checks below (0-12). Do not trust the reviewer's checkmarks. Re-derive every conclusion from CODEOWNERS, CI runs, the PR diff, the master configs, and the linked recipe yourself. Be rigorous and specific. The checks encode the merge standard in `docs/PR_REVIEW_CHECKLIST.md`. Read it in the checked-out @@ -359,8 +359,38 @@ Verify BOTH: unless the sign-off documents a sanctioned exception. - N/A if the PR has no agentic speculative-decoding changes (state that in one line). +## Check 12 — Append-only changes only add new points to an unchanged curve +APPLICABILITY: this check applies when any new `perf-changelog.yaml` entry contains +`append-only: true`. If none does, report N/A. +- Confirm every new changelog entry in the sweep is append-only; mixed regular and + append-only entries are not allowed. +- Inspect the complete PR diff without using a file allowlist. Supporting code, + benchmark scripts, launchers, helpers, and other files may change. Their path alone + is never a reason to fail; determine whether each benchmark-affecting change is + behaviorally isolated to the appended points. +- For every selected config, compare the generated matrix at the PR base and head. + Treat the complete base matrix as an immutable subset of the head matrix: every + existing point must remain present with the same image and complete recipe. The + head may add concurrency points or entirely new recipe variants, such as a new + tensor-parallelism value, inside the selected existing config/scenario. Every + addition must retain the target visual curve's one non-null image. +- Trace the selected config and generated runtime values through every affected file + into the changed behavior. The behavior must be reachable only for the corresponding + newly appended points. PASS when the controlling condition is uniquely satisfied by + those points. FAIL an unguarded/shared setup change, a condition also satisfied by an + existing point, or any case where exclusivity cannot be proven from the diff. +- FAIL if any existing point or recipe is rerun, removed, or modified. New configs and + scenarios are out of scope, but new generated recipe variants inside the selected + existing config/scenario are allowed. Other benchmark-affecting changes are permitted + only under the behavioral-isolation rule above. +- Treat the repository's append-only matrix validation as supporting evidence, but + verify the diff independently and name the offending field/path when failing. Each + config revision is rendered with its own generator, validation code, and runner + metadata, but this does not mechanically prove that launcher or benchmark-script + changes are isolated at runtime. + ## Verdict and output -Decide PASS only if Checks 0-11 ALL pass. A check reported as `N/A` counts as a pass. +Decide PASS only if Checks 0-12 ALL pass. A check reported as `N/A` counts as a pass. Keep the `N/A — ` row so the reviewer sees it was considered. Post EXACTLY ONE summary comment on PR #${PR_NUMBER} using `gh pr comment`. Start the comment with the hidden marker so reruns are identifiable: @@ -386,7 +416,7 @@ single terse line. Rules: restating the checklist, no hedging ("if X then maybe Y"). Make the call. Link the run/recipe instead of describing it. -- If everything is to standard: post the verdict header + the twelve one-line rows +- If everything is to standard: post the verdict header + the thirteen one-line rows - If anything is NOT to standard: the verdict header must be immediately followed by a line that @-mentions the sign-off author as `@${SIGNOFF_AUTHOR}` with the blocking summary. Then the per-check lines, each failing one led by its root diff --git a/.github/workflows/benchmark-multinode-tmpl.yml b/.github/workflows/benchmark-multinode-tmpl.yml index 0a666a6e90..05532b1dd0 100644 --- a/.github/workflows/benchmark-multinode-tmpl.yml +++ b/.github/workflows/benchmark-multinode-tmpl.yml @@ -37,6 +37,11 @@ on: exp-name: required: true type: string + recipe-fingerprint: + description: "Deterministic generated-recipe identity" + required: false + type: string + default: '' isl: required: true type: string @@ -217,6 +222,7 @@ env: # once; sbatch/srun inherit this env so the token reaches the workers. HF_TOKEN: ${{ secrets.INFERENCEX_OFFICIAL_RO_HF_TOKEN }} EXP_NAME: ${{ inputs.exp-name }} + RECIPE_FINGERPRINT: ${{ inputs.recipe-fingerprint }} IMAGE: ${{ inputs.image }} MODEL_PREFIX: ${{ inputs.model-prefix }} MODEL: ${{ inputs.model }} @@ -336,10 +342,13 @@ jobs: env: RUNNER_NAME: ${{ runner.name }} RUNNER_TYPE: ${{ inputs.runner }} - # Hash uniquely on all prefill/decode parallelism fields, worker counts, serving mode, concurrency, and runner. - RESULT_FILENAME: ${{ env.EXP_NAME }}_${{ env.PRECISION }}_${{ env.FRAMEWORK }}_prefill-tp${{ env.PREFILL_TP }}-pp${{ env.PREFILL_PP_SIZE }}-dcp${{ env.PREFILL_DCP_SIZE }}-pcp${{ env.PREFILL_PCP_SIZE }}-ep${{ env.PREFILL_EP }}-dp${{ env.PREFILL_DP_ATTN }}-nw${{ env.PREFILL_NUM_WORKERS }}_decode-tp${{ env.DECODE_TP }}-pp${{ env.DECODE_PP_SIZE }}-dcp${{ env.DECODE_DCP_SIZE }}-pcp${{ env.DECODE_PCP_SIZE }}-ep${{ env.DECODE_EP }}-dp${{ env.DECODE_DP_ATTN }}-nw${{ env.DECODE_NUM_WORKERS }}_disagg-${{ env.DISAGG }}_spec-${{ env.SPEC_DECODING }}_conc${{ join(fromJson(inputs.conc-list), 'x') }}_${{ runner.name }} + RESULT_FILENAME_BASE: ${{ env.EXP_NAME }}_${{ env.PRECISION }}_${{ env.FRAMEWORK }}_prefill-tp${{ env.PREFILL_TP }}-pp${{ env.PREFILL_PP_SIZE }}-dcp${{ env.PREFILL_DCP_SIZE }}-pcp${{ env.PREFILL_PCP_SIZE }}-ep${{ env.PREFILL_EP }}-dp${{ env.PREFILL_DP_ATTN }}-nw${{ env.PREFILL_NUM_WORKERS }}_decode-tp${{ env.DECODE_TP }}-pp${{ env.DECODE_PP_SIZE }}-dcp${{ env.DECODE_DCP_SIZE }}-pcp${{ env.DECODE_PCP_SIZE }}-ep${{ env.DECODE_EP }}-dp${{ env.DECODE_DP_ATTN }}-nw${{ env.DECODE_NUM_WORKERS }}_disagg-${{ env.DISAGG }}_spec-${{ env.SPEC_DECODING }}_conc${{ join(fromJson(inputs.conc-list), 'x') }}_${{ runner.name }} run: | set -x + export RESULT_FILENAME="$RESULT_FILENAME_BASE" + if [ -n "$RECIPE_FINGERPRINT" ]; then + export RESULT_FILENAME="${RESULT_FILENAME}_recipe-${RECIPE_FINGERPRINT:0:16}" + fi # Export RESULT_FILENAME early so it's available for artifact uploads even if cancelled echo "RESULT_FILENAME=${RESULT_FILENAME}" >> "$GITHUB_ENV" diff --git a/.github/workflows/benchmark-tmpl.yml b/.github/workflows/benchmark-tmpl.yml index 6c4fe50fe5..bb3fcc259b 100644 --- a/.github/workflows/benchmark-tmpl.yml +++ b/.github/workflows/benchmark-tmpl.yml @@ -36,6 +36,11 @@ on: exp-name: required: true type: string + recipe-fingerprint: + description: "Deterministic generated-recipe identity" + required: false + type: string + default: '' isl: required: true type: string @@ -154,6 +159,7 @@ env: HF_TOKEN: ${{ secrets.INFERENCEX_OFFICIAL_RO_HF_TOKEN }} HF_HUB_CACHE: '/mnt/hf_hub_cache/' EXP_NAME: ${{ inputs.exp-name }} + RECIPE_FINGERPRINT: ${{ inputs.recipe-fingerprint }} MODEL: ${{ inputs.model }} MODEL_PREFIX: ${{ inputs.model-prefix }} ISL: ${{ inputs.isl }} @@ -269,17 +275,20 @@ jobs: env: RUNNER_NAME: ${{ runner.name }} RUNNER_TYPE: ${{ inputs.runner }} - # Hash uniquely on {EXP_NAME}_{PRECISION}_{FRAMEWORK}_tp{}-pp{}-dcp{}-pcp{}-ep{}-dpa{}_disagg-{}_spec-{}_conc{}_{runner} - RESULT_FILENAME: ${{ env.EXP_NAME }}_${{ env.PRECISION }}_${{ env.FRAMEWORK }}_tp${{ env.TP }}-pp${{ env.PP_SIZE }}-dcp${{ env.DCP_SIZE }}-pcp${{ env.PCP_SIZE }}-ep${{ env.EP_SIZE }}-dpa${{ env.DP_ATTENTION }}_disagg-${{ env.DISAGG }}_spec-${{ env.SPEC_DECODING }}_conc${{ env.CONC }}_${{ runner.name }} + RESULT_FILENAME_BASE: ${{ env.EXP_NAME }}_${{ env.PRECISION }}_${{ env.FRAMEWORK }}_tp${{ env.TP }}-pp${{ env.PP_SIZE }}-dcp${{ env.DCP_SIZE }}-pcp${{ env.PCP_SIZE }}-ep${{ env.EP_SIZE }}-dpa${{ env.DP_ATTENTION }}_disagg-${{ env.DISAGG }}_spec-${{ env.SPEC_DECODING }}_conc${{ env.CONC }}_${{ runner.name }} # Suppress per-job eval markdown from being appended to the step summary. # We'll publish a single combined eval table in the collection job instead. GITHUB_STEP_SUMMARY: '' run: | + export RESULT_FILENAME="$RESULT_FILENAME_BASE" + if [ -n "$RECIPE_FINGERPRINT" ]; then + export RESULT_FILENAME="${RESULT_FILENAME}_recipe-${RECIPE_FINGERPRINT:0:16}" + fi export GPU_COUNT=$((TP * PP_SIZE * PCP_SIZE)) echo "GPU_COUNT=${GPU_COUNT}" >> "$GITHUB_ENV" # Export RESULT_FILENAME early so it's available for artifact uploads even if cancelled - echo "RESULT_FILENAME=${RESULT_FILENAME}" >> $GITHUB_ENV + echo "RESULT_FILENAME=${RESULT_FILENAME}" >> "$GITHUB_ENV" bash ./runners/launch_${RUNNER_NAME%%_*}.sh diff --git a/.github/workflows/claude-pr-review.yml b/.github/workflows/claude-pr-review.yml index 09443b5f87..f74a2d9f3b 100644 --- a/.github/workflows/claude-pr-review.yml +++ b/.github/workflows/claude-pr-review.yml @@ -134,6 +134,16 @@ jobs: - This is a 🔴 **BLOCKING** issue - Comment: "New `perf-changelog.yaml` entries must be appended to the END of the file. The file is read chronologically (oldest at top, newest at bottom), so inserting in the middle or prepending breaks the ordering. Please move the new entry(ies) to the bottom of the file." + ### Append-only Perf Changelog Safety: + When a new `perf-changelog.yaml` entry contains `append-only: true`, verify the complete PR diff before approving it: + - Do not use a file allowlist. Supporting code, benchmark scripts, launchers, helpers, and other files may change. Inspect the complete diff and judge whether each benchmark-affecting change is behaviorally isolated to the appended points. + - Every newly added changelog entry must contain `append-only: true`; append-only and regular entries may not be mixed. + - Treat the generated base matrix as an immutable subset of the generated head matrix. Every existing point must remain present with the same image and complete recipe. Additions may include new concurrency values or entirely new recipe variants (for example, a new tensor-parallelism value) inside the selected existing config/scenario, but they must retain the existing visual curve's single non-null image. + - Benchmark or launch logic may change only when every changed behavior is on a control-flow path uniquely gated to the corresponding newly appended points. Trace the selected config and generated runtime values through every affected file into the condition. Confirm the path cannot be reached by any existing point. Unguarded/shared setup changes, or a branch also used by an existing concurrency/config/scenario, are blocking. + - No existing point, recipe variant, config, or scenario may be removed or replaced. New configs and scenarios are out of scope for append-only mode; new generated variants inside the selected existing config/scenario are allowed. + - Eval modifiers (`evals-only`, `all-evals`, `eval-min-prefill-ep`) are not allowed. + If any condition fails, report a 🔴 **BLOCKING** issue. Never reject a change merely because of its file path; reject it when its benchmark effect is not exclusive to the appended points or the exclusivity cannot be proven from the diff. + ## Terminology: - **STP (Single Token Prediction)**: Standard autoregressive decoding — one token per forward pass. No speculative decoding or MTP. Benchmarks labeled "STP only" use vanilla decoding. - **MTP (Multi-Token Prediction)**: Predicts multiple tokens per forward pass using speculative decoding (e.g., EAGLE, NEXTN). diff --git a/.github/workflows/claude.yml b/.github/workflows/claude.yml index e3c525d5e2..9f3798aaa9 100644 --- a/.github/workflows/claude.yml +++ b/.github/workflows/claude.yml @@ -223,6 +223,8 @@ jobs: See `docs/configuration-procedures.md` → "Update an image" and "Append the changelog safely" for entry format and rules. Required whenever you change image tags, env vars, or perf-affecting params in `configs/*-master.yaml` or `benchmarks/*.sh`. 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. + ## Spawning Additional Workers: You CAN spawn additional Claude workers by commenting "@claude" with a specific task. diff --git a/.github/workflows/e2e-tests.yml b/.github/workflows/e2e-tests.yml index c973df94e6..d4318a39fa 100644 --- a/.github/workflows/e2e-tests.yml +++ b/.github/workflows/e2e-tests.yml @@ -298,6 +298,7 @@ jobs: router: ${{ matrix.config.router && toJson(matrix.config.router) || '' }} kv-p2p-transfer: ${{ matrix.config['kv-p2p-transfer'] || '' }} exp-name: ${{ matrix.config.exp-name }} + recipe-fingerprint: ${{ matrix.config['recipe-fingerprint'] || '' }} conc-list: ${{ toJson(matrix.config.conc) }} spec-decoding: ${{ matrix.config.spec-decoding }} disagg: ${{ matrix.config.disagg }} @@ -351,6 +352,7 @@ jobs: router: ${{ matrix.config.router && toJson(matrix.config.router) || '' }} kv-p2p-transfer: ${{ matrix.config['kv-p2p-transfer'] || '' }} exp-name: ${{ matrix.config.exp-name }} + recipe-fingerprint: ${{ matrix.config['recipe-fingerprint'] || '' }} conc-list: ${{ toJson(matrix.config.conc) }} spec-decoding: ${{ matrix.config.spec-decoding }} disagg: ${{ matrix.config.disagg }} @@ -391,6 +393,7 @@ jobs: secrets: inherit with: exp-name: ${{ matrix.config.exp-name }} + recipe-fingerprint: ${{ matrix.config['recipe-fingerprint'] || '' }} runner: ${{ matrix.config.runner }} priority: ${{ matrix.config.priority }} queue-token: ${{ matrix.config['queue-token'] }} @@ -435,6 +438,7 @@ jobs: secrets: inherit with: exp-name: ${{ matrix.config.exp-name }} + recipe-fingerprint: ${{ matrix.config['recipe-fingerprint'] || '' }} runner: ${{ matrix.config.runner }} priority: ${{ matrix.config.priority }} queue-token: ${{ matrix.config['queue-token'] }} @@ -476,6 +480,7 @@ jobs: secrets: inherit with: exp-name: ${{ matrix.config.exp-name }} + recipe-fingerprint: ${{ matrix.config['recipe-fingerprint'] || '' }} isl: '0' osl: '0' max-model-len: '0' @@ -532,6 +537,7 @@ jobs: secrets: inherit with: exp-name: ${{ matrix.config.exp-name }} + recipe-fingerprint: ${{ matrix.config['recipe-fingerprint'] || '' }} isl: '0' osl: '0' max-model-len: '0' @@ -591,6 +597,7 @@ jobs: secrets: inherit with: exp-name: ${{ matrix.config.exp-name }} + recipe-fingerprint: ${{ matrix.config['recipe-fingerprint'] || '' }} isl: ${{ matrix.config.isl }} osl: ${{ matrix.config.osl }} max-model-len: ${{ matrix.config.max-model-len }} @@ -629,6 +636,7 @@ jobs: secrets: inherit with: exp-name: ${{ matrix.config.exp-name }} + recipe-fingerprint: ${{ matrix.config['recipe-fingerprint'] || '' }} isl: ${{ matrix.config.isl }} osl: ${{ matrix.config.osl }} max-model-len: ${{ matrix.config.max-model-len }} diff --git a/.github/workflows/run-sweep.yml b/.github/workflows/run-sweep.yml index b8e95f0806..c01093cb8e 100644 --- a/.github/workflows/run-sweep.yml +++ b/.github/workflows/run-sweep.yml @@ -411,6 +411,7 @@ jobs: secrets: inherit with: exp-name: ${{ matrix.config.exp-name }} + recipe-fingerprint: ${{ matrix.config['recipe-fingerprint'] || '' }} isl: ${{ matrix.config.isl }} osl: ${{ matrix.config.osl }} max-model-len: ${{ matrix.config.max-model-len }} @@ -469,6 +470,7 @@ jobs: router: ${{ matrix.config.router && toJson(matrix.config.router) || '' }} kv-p2p-transfer: ${{ matrix.config['kv-p2p-transfer'] || '' }} exp-name: ${{ matrix.config.exp-name }} + recipe-fingerprint: ${{ matrix.config['recipe-fingerprint'] || '' }} conc-list: ${{ toJson(matrix.config.conc) }} spec-decoding: ${{ matrix.config.spec-decoding }} disagg: ${{ matrix.config.disagg }} @@ -533,6 +535,7 @@ jobs: secrets: inherit with: &single-node-inputs exp-name: ${{ matrix.config.exp-name }} + recipe-fingerprint: ${{ matrix.config['recipe-fingerprint'] || '' }} isl: ${{ matrix.config.isl }} osl: ${{ matrix.config.osl }} max-model-len: ${{ matrix.config.max-model-len }} @@ -597,6 +600,7 @@ jobs: secrets: inherit with: exp-name: ${{ matrix.config.exp-name }} + recipe-fingerprint: ${{ matrix.config['recipe-fingerprint'] || '' }} runner: ${{ matrix.config.runner }} priority: ${{ matrix.config.priority }} queue-token: ${{ matrix.config['queue-token'] }} @@ -648,6 +652,7 @@ jobs: secrets: inherit with: exp-name: ${{ matrix.config.exp-name }} + recipe-fingerprint: ${{ matrix.config['recipe-fingerprint'] || '' }} isl: '0' osl: '0' max-model-len: '0' @@ -713,6 +718,7 @@ jobs: secrets: inherit with: exp-name: ${{ matrix.config.exp-name }} + recipe-fingerprint: ${{ matrix.config['recipe-fingerprint'] || '' }} isl: ${{ matrix.config.isl }} osl: ${{ matrix.config.osl }} max-model-len: ${{ matrix.config.max-model-len }} @@ -762,6 +768,7 @@ jobs: secrets: inherit with: exp-name: ${{ matrix.config.exp-name }} + recipe-fingerprint: ${{ matrix.config['recipe-fingerprint'] || '' }} runner: ${{ matrix.config.runner }} priority: ${{ matrix.config.priority }} queue-token: ${{ matrix.config['queue-token'] }} @@ -814,6 +821,7 @@ jobs: secrets: inherit with: exp-name: ${{ matrix.config.exp-name }} + recipe-fingerprint: ${{ matrix.config['recipe-fingerprint'] || '' }} isl: ${{ matrix.config.isl }} osl: ${{ matrix.config.osl }} max-model-len: ${{ matrix.config.max-model-len }} @@ -879,6 +887,7 @@ jobs: secrets: inherit with: exp-name: ${{ matrix.config.exp-name }} + recipe-fingerprint: ${{ matrix.config['recipe-fingerprint'] || '' }} isl: '0' osl: '0' max-model-len: '0' diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 3662dd9541..fb6241781a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -44,6 +44,44 @@ A full benchmark sweep is expensive GPU time, and the runners are shared by ever - **This reduces CI queue time for everyone.** Each reused merge frees hours of GPU runner time for other PRs, so please prefer the reuse path over merging without it. A green sweep alone is not enough. The `/reuse-sweep-run` comment must be on record (the sign-off verification checks for it), otherwise `main` silently re-runs the full sweep. - `utils/merge_with_reuse.sh ` is the supported merge path. It posts the command, syncs the branch with `main`, waits for checks, and squash-merges. See the [workflows README](.github/workflows/README.md#reusing-an-approved-pr-full-sweep) for eligibility details. +## Adding points to the latest curve with `append-only` + +When a PR only adds generated points to an existing curve, mark every new changelog +entry with `append-only: true`. Additions may introduce new concurrency values or new +recipe variants, such as another tensor-parallelism value. Sweep setup compares the +generated matrices at the base and head revisions, runs only the newly added points, +and emits metadata that lets InferenceX-app extend the most recent matching curve +instead of presenting the partial run as a separate curve. + +This mode is intentionally narrow, but it is not based on a file allowlist. Supporting +code, benchmark scripts, launchers, and other files may change when their behavioral +effect is exclusive to the newly appended points named by the changelog. No changed +benchmark path may execute for or alter an existing point. Every selected config and +scenario must already exist, and every point generated at the base revision must +remain present with the same recipe. The head may contain any additional generated +recipes or points inside that scope, including new topology or other recipe dimensions; +the sweep schedules the generated set difference. Additions must use the same non-null +image and belong to an existing dashboard visual series. Each generated recipe carries +a deterministic fingerprint so two distinct recipes at the same concurrency remain +distinct database points without splitting the visual curve. Removing or modifying an +existing point, or changing shared logic that can affect one, is rejected. Append-only +entries cannot be mixed with regular entries or eval-selection modifiers in the same +sweep. The matrix validator enforces the additive generated-matrix invariant; the human +and AI reviewers must inspect the complete diff and verify behavioral isolation. The +mechanical comparison renders each config revision with its own generator, validation +code, and runner metadata. Launcher and benchmark-script changes still rely on +complete-diff review because matrix equality alone cannot prove their runtime +control-flow isolation. + +```yaml +- config-keys: + - dsv4-fp4-b300-vllm-mtp + description: + - "Add TP8 at concurrency 12 and 16 to the existing curve" + pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXX + append-only: true +``` + ## AMD cluster: never leave root-owned files in runner workspaces Multi-node benchmarks on the AMD MI355X TW cluster submit Slurm jobs whose containers often run as **root**. If those containers write files (typically `benchmark_logs/logs/slurm_job-*`) into the GitHub Actions runner workspace and the job is **cancelled** before teardown runs, the root-owned directories are stranded. The runner user cannot delete them, so `actions/checkout` fails with: diff --git a/benchmarks/benchmark_lib.sh b/benchmarks/benchmark_lib.sh index 8cc894940f..26bde59696 100644 --- a/benchmarks/benchmark_lib.sh +++ b/benchmarks/benchmark_lib.sh @@ -1220,6 +1220,7 @@ _write_lm_eval_meta_json() { "framework": "${fw:-unknown}", "precision": "${prec:-unknown}", "spec_decoding": "${SPEC_DECODING:-}", + "recipe_fingerprint": "${RECIPE_FINGERPRINT:-}", "tp": ${TP:-1}, "pp": ${PP_SIZE:-1}, "dcp_size": ${DCP_SIZE:-1}, diff --git a/docs/PR_REVIEW_CHECKLIST.md b/docs/PR_REVIEW_CHECKLIST.md index a264c1e650..59f923c815 100644 --- a/docs/PR_REVIEW_CHECKLIST.md +++ b/docs/PR_REVIEW_CHECKLIST.md @@ -28,6 +28,7 @@ As a PR reviewer and CODEOWNER, I have reviewed this and have: - [ ] Verified that every single-node vLLM/SGLang recipe in this PR is documented in the official [vLLM recipes](https://recipes.vllm.ai/) and/or the [SGLang cookbook](https://docs.sglang.io/cookbook/intro): - [ ] I linked the corresponding upstream PR in the [vLLM recipe repo](https://github.com/vllm-project/recipes) or [SGLang repo](https://github.com/sgl-project/sglang/tree/main/docs_new) and verified that it is **MERGED** before this InferenceX PR merges. An opened, draft, or closed-without-merge upstream PR does not satisfy this requirement. If the matching recipe was already published, I linked the published recipe/cookbook page in the additional detail section below. - [ ] Verified that this PR does not patch the inference engine or serving stack — the pinned image must run as shipped. This covers .patch files / git apply / patch, inline patches embedded in benchmark scripts (e.g. a python3/sed heredoc that rewrites installed engine sources before serving), in-place edits of site-packages, monkey-patching, overwriting container files, and installing forked/rebuilt engine wheels on top of the pinned image. The only exception is a patch covered by a filled-out waiver at [docs/waiver/](https://github.com/SemiAnalysisAI/InferenceX/tree/main/docs/waiver)`.md` — named after the PR that introduces the patch and filed in that same PR, stating what is patched, why the unmodified upstream image cannot run this benchmark, the upstream PR/issue link, and the removal plan — which I have linked below in the additional detail section. +- [ ] If this PR uses `append-only: true`, verified that it only adds generated points or recipe variants inside a selected existing config/scenario and existing same-image visual curve: every previously generated point remains present with the same recipe, no prior point is removed or rerun, and every benchmark-affecting change in the complete diff can affect only the corresponding newly appended points (never an existing point), regardless of which file contains it. - [ ] If any of the above criteria cannot reasonably be satisfied, I have provided additional reasoning below. ### Additional detail section: @@ -42,4 +43,3 @@ Signed: `FILL_IN_GITHUB_USERNAME` image - diff --git a/utils/agentic/aggregation/process_agentic_result.py b/utils/agentic/aggregation/process_agentic_result.py index 5d13fa0f1b..c0d8d70ce1 100644 --- a/utils/agentic/aggregation/process_agentic_result.py +++ b/utils/agentic/aggregation/process_agentic_result.py @@ -216,6 +216,7 @@ def build_agg( "hw": os.environ.get("RUNNER_TYPE", ""), "conc": int(os.environ.get("CONC", "0")), "image": os.environ.get("IMAGE", ""), + "recipe_fingerprint": os.environ.get("RECIPE_FINGERPRINT", ""), "model": os.environ.get("MODEL", ""), "infmax_model_prefix": os.environ.get("MODEL_PREFIX", ""), "framework": framework, diff --git a/utils/agentic/aggregation/test_process_agentic_result.py b/utils/agentic/aggregation/test_process_agentic_result.py index 7f5b0395bf..a567e9aced 100644 --- a/utils/agentic/aggregation/test_process_agentic_result.py +++ b/utils/agentic/aggregation/test_process_agentic_result.py @@ -343,6 +343,7 @@ def _run_processor( "KV_OFFLOADING": "none", "RUNNER_TYPE": "b200-x4", "IMAGE": "test/image:0.1", + "RECIPE_FINGERPRINT": "b" * 64, "SPEC_DECODING": "none", "DISAGG": "false", "IS_MULTINODE": "false", @@ -373,6 +374,7 @@ def test_processor_emits_nested_request_and_server_metrics(tmp_path: Path): result_dir = _write_fixture(tmp_path) output_dir = tmp_path / "out" agg = _run_processor(result_dir, output_dir) + assert agg["recipe_fingerprint"] == "b" * 64 missing = AGG_TOP_LEVEL_KEYS - set(agg.keys()) assert not missing, f"agg JSON missing top-level keys: {sorted(missing)}" assert not (_flat_request_keys(result_dir) & set(agg.keys())) diff --git a/utils/matrix_logic/validation.py b/utils/matrix_logic/validation.py index f95c1e63d7..eceed9a456 100644 --- a/utils/matrix_logic/validation.py +++ b/utils/matrix_logic/validation.py @@ -78,6 +78,7 @@ class Fields(Enum): CONC = 'conc' MAX_MODEL_LEN = 'max-model-len' EXP_NAME = 'exp-name' + RECIPE_FINGERPRINT = 'recipe-fingerprint' DISAGG = 'disagg' SCENARIO_TYPE = 'scenario-type' @@ -175,6 +176,11 @@ class SingleNodeMatrixEntry(BaseModel): run_eval: bool = Field(alias=Fields.RUN_EVAL.value) eval_only: bool = Field(alias=Fields.EVAL_ONLY.value, default=False) router: Optional[ComponentMetadata] = None + recipe_fingerprint: Optional[str] = Field( + default=None, + alias=Fields.RECIPE_FINGERPRINT.value, + pattern=r"^[0-9a-f]{64}$", + ) @model_validator(mode='after') def validate_single_node_topology(self): @@ -245,6 +251,11 @@ class MultiNodeMatrixEntry(BaseModel): kv_p2p_transfer: Optional[str] = Field( default=None, alias=Fields.KV_P2P_TRANSFER.value, min_length=1 ) + recipe_fingerprint: Optional[str] = Field( + default=None, + alias=Fields.RECIPE_FINGERPRINT.value, + pattern=r"^[0-9a-f]{64}$", + ) @model_validator(mode='after') def validate_worker_hardware_pair(self): @@ -295,6 +306,11 @@ class SingleNodeAgenticMatrixEntry(BaseModel): # omit them, and exclude_none keeps them out of dumped benchmark output. run_eval: Optional[bool] = Field(default=None, alias=Fields.RUN_EVAL.value) eval_only: Optional[bool] = Field(default=None, alias=Fields.EVAL_ONLY.value) + recipe_fingerprint: Optional[str] = Field( + default=None, + alias=Fields.RECIPE_FINGERPRINT.value, + pattern=r"^[0-9a-f]{64}$", + ) @model_validator(mode='after') def validate_kv_offload_fields(self): @@ -341,6 +357,11 @@ class MultiNodeAgenticMatrixEntry(BaseModel): run_eval: Optional[bool] = Field(default=None, alias=Fields.RUN_EVAL.value) eval_only: Optional[bool] = Field(default=None, alias=Fields.EVAL_ONLY.value) eval_conc: Optional[int] = Field(default=None, alias=Fields.EVAL_CONC.value) + recipe_fingerprint: Optional[str] = Field( + default=None, + alias=Fields.RECIPE_FINGERPRINT.value, + pattern=r"^[0-9a-f]{64}$", + ) @model_validator(mode='after') def validate_worker_hardware_pair(self): @@ -875,6 +896,14 @@ class ChangelogEntry(BaseModel): pr_link: str = Field(alias="pr-link") evals_only: bool = Field(alias="evals-only", default=False) all_evals: bool = Field(alias="all-evals", default=False) + append_only: bool = Field( + alias="append-only", + default=False, + description=( + "Run only generated points or recipe variants added while preserving " + "every existing generated point, then append them to the latest curve" + ), + ) eval_min_prefill_ep: Optional[int] = Field( alias="eval-min-prefill-ep", default=None, ge=1, description=( @@ -887,6 +916,17 @@ class ChangelogEntry(BaseModel): description="Restrict to specific scenario types (e.g., ['fixed-seq-len', 'agentic-coding'])" ) + @model_validator(mode="after") + def validate_append_only_mode(self): + """Append-only entries are throughput deltas, never eval-only requests.""" + if self.append_only and ( + self.evals_only or self.all_evals or self.eval_min_prefill_ep is not None + ): + raise ValueError( + "append-only cannot be combined with eval selection fields" + ) + return self + class ChangelogMetadata(BaseModel): """Pydantic model for validating changelog metadata structure.""" diff --git a/utils/process_changelog.py b/utils/process_changelog.py index 91b276ba2f..6149d8fba7 100644 --- a/utils/process_changelog.py +++ b/utils/process_changelog.py @@ -1,8 +1,14 @@ import argparse +import copy +import hashlib import json import re import subprocess +import tempfile from collections import defaultdict +from contextlib import contextmanager +from dataclasses import dataclass +from pathlib import Path import yaml from constants import GENERATE_SWEEPS_PY_SCRIPT, MASTER_CONFIGS @@ -16,6 +22,13 @@ SCENARIO_TYPES = ("fixed-seq-len", "agentic-coding") +@dataclass(frozen=True) +class GenerationInputs: + config_files: list[str] + generator_script: str + runner_config: str + + def _freeze_config_value(value): """Convert JSON-shaped config values into deterministic hashable values.""" if isinstance(value, dict): @@ -143,6 +156,247 @@ def get_config_keys_from_master( return list(resolved_keys) +@contextmanager +def generation_inputs_at_ref(ref: str): + """Materialize config and generator inputs from one repository revision.""" + with tempfile.TemporaryDirectory(prefix="inferencex-append-only-") as temp_dir: + files_result = subprocess.run( + [ + "git", + "ls-tree", + "-r", + "--name-only", + ref, + "--", + "utils/matrix_logic", + *MASTER_CONFIGS, + "configs/runners.yaml", + ], + capture_output=True, + check=True, + text=True, + ) + repo_paths = files_result.stdout.splitlines() + required_paths = { + *MASTER_CONFIGS, + "configs/runners.yaml", + GENERATE_SWEEPS_PY_SCRIPT, + } + missing_paths = required_paths - set(repo_paths) + if missing_paths: + raise ValueError( + f"append-only base revision is missing generation inputs: " + f"{sorted(missing_paths)}" + ) + + for repo_path in repo_paths: + result = subprocess.run( + ["git", "show", f"{ref}:{repo_path}"], + capture_output=True, + check=True, + ) + destination = Path(temp_dir) / repo_path + destination.parent.mkdir(parents=True, exist_ok=True) + destination.write_bytes(result.stdout) + + yield GenerationInputs( + config_files=[str(Path(temp_dir) / path) for path in MASTER_CONFIGS], + generator_script=str(Path(temp_dir) / GENERATE_SWEEPS_PY_SCRIPT), + runner_config=str(Path(temp_dir) / "configs/runners.yaml"), + ) + + +def _matrix_curve_key(entry: dict) -> tuple: + """Identify one curve while deliberately excluding point-level fields.""" + return tuple( + sorted( + (key, _freeze_config_value(value)) + for key, value in entry.items() + if key not in {"conc", "exp-name", "recipe-fingerprint"} + ) + ) + + +def recipe_fingerprint(entry: dict) -> str: + """Hash the generated recipe independently of point-level concurrency/name.""" + recipe = { + key: value + for key, value in entry.items() + if key not in {"conc", "exp-name", "recipe-fingerprint"} + } + canonical = json.dumps( + recipe, + sort_keys=True, + separators=(",", ":"), + ensure_ascii=True, + ) + return hashlib.sha256(canonical.encode("utf-8")).hexdigest() + + +def _matrix_visual_series_key(entry: dict) -> tuple: + """Identify the App curve that an appended recipe must already belong to.""" + is_agentic = entry.get("scenario-type") == "agentic-coding" + kv_offloading = entry.get("kv-offloading", "none") + offload_mode = "off" if kv_offloading in (None, "", "none") else "on" + prefill = entry.get("prefill") or {} + decode = entry.get("decode") or {} + return ( + entry.get("model"), + entry.get("model-prefix"), + entry.get("precision"), + entry.get("framework"), + entry.get("runner"), + bool(entry.get("disagg", False)), + "agentic_traces" if is_agentic else "single_turn", + None if is_agentic else entry.get("isl"), + None if is_agentic else entry.get("osl"), + offload_mode, + "" if is_agentic else entry.get("spec-decoding", "none"), + prefill.get("hardware"), + decode.get("hardware"), + ) + + +def _matrix_concurrencies(entry: dict) -> tuple[int, ...]: + conc = entry.get("conc") + if isinstance(conc, int): + return (conc,) + if isinstance(conc, list) and conc and all(isinstance(value, int) for value in conc): + return tuple(conc) + raise ValueError(f"append-only matrix entry has invalid concurrency value: {conc!r}") + + +def append_only_delta(base_entries: list[dict], head_entries: list[dict]) -> list[dict]: + """Return only newly added points, rejecting any existing-point mutation. + + Generated matrix rows are the runtime contract. Grouping them without ``conc`` + and ``exp-name`` lets an existing recipe gain concurrency while also permitting + entirely new recipe variants. Every base recipe and concurrency must remain in + the head unchanged; the returned delta is therefore strictly additive. + """ + base_groups: dict[tuple, set[int]] = defaultdict(set) + head_groups: dict[tuple, set[int]] = defaultdict(set) + for entry in base_entries: + base_groups[_matrix_curve_key(entry)].update(_matrix_concurrencies(entry)) + for entry in head_entries: + head_groups[_matrix_curve_key(entry)].update(_matrix_concurrencies(entry)) + + if not base_groups: + raise ValueError("append-only requires an existing curve in the base revision") + + removed_curves = base_groups.keys() - head_groups.keys() + if removed_curves: + raise ValueError( + "append-only may not remove or modify existing generated recipes" + ) + + for key, base_concurrencies in base_groups.items(): + removed_points = base_concurrencies - head_groups[key] + if removed_points: + raise ValueError( + "append-only may not remove existing concurrency points: " + f"{sorted(removed_points)}" + ) + + delta: list[dict] = [] + emitted_concurrencies: dict[tuple, set[int]] = defaultdict(set) + for entry in head_entries: + key = _matrix_curve_key(entry) + added = head_groups[key] - base_groups.get(key, set()) + conc = entry.get("conc") + if isinstance(conc, int): + if conc in added and conc not in emitted_concurrencies[key]: + delta.append(entry) + emitted_concurrencies[key].add(conc) + continue + added_in_source_order = [] + for value in conc: + if value in added and value not in emitted_concurrencies[key]: + added_in_source_order.append(value) + emitted_concurrencies[key].add(value) + if added_in_source_order: + delta_entry = copy.deepcopy(entry) + delta_entry["conc"] = added_in_source_order + delta.append(delta_entry) + + if not delta: + raise ValueError("append-only did not add any generated points") + + base_images_by_series: dict[tuple, set[str | None]] = defaultdict(set) + for entry in base_entries: + base_images_by_series[_matrix_visual_series_key(entry)].add( + entry.get("image") + ) + for entry in delta: + series_key = _matrix_visual_series_key(entry) + base_images = base_images_by_series.get(series_key, set()) + image = entry.get("image") + if image is None or base_images != {image}: + raise ValueError( + "append-only additions must belong to an existing visual curve " + "with one unchanged non-null image" + ) + return delta + + +def validate_append_only_scope( + base_master: dict, + head_master: dict, + selected_config_scenarios: dict[str, set[str]], +) -> None: + """Reject edits outside selected existing configs and scenarios. + + Changes inside an explicitly selected scenario are checked semantically by + ``append_only_delta`` after generating the complete base and head matrices. + This permits arbitrary additive recipe variants while ensuring every existing + generated point remains unchanged and present. + """ + selected_configs = selected_config_scenarios.keys() + all_keys = base_master.keys() | head_master.keys() + unrelated_changes = [ + key + for key in all_keys + if key not in selected_configs and base_master.get(key) != head_master.get(key) + ] + if unrelated_changes: + raise ValueError( + "append-only PR changed configs not selected by its changelog entry: " + f"{sorted(unrelated_changes)}" + ) + + for config, allowed_scenarios in selected_config_scenarios.items(): + base_config = base_master[config] + head_config = head_master[config] + base_scenarios = base_config.get("scenarios", {}) + head_scenarios = head_config.get("scenarios", {}) + if base_scenarios.keys() != head_scenarios.keys(): + raise ValueError( + f"append-only added or removed a scenario in config {config!r}" + ) + + unselected_scenarios = base_scenarios.keys() - allowed_scenarios + base_top_level = { + key: value for key, value in base_config.items() if key != "scenarios" + } + head_top_level = { + key: value for key, value in head_config.items() if key != "scenarios" + } + if unselected_scenarios and base_top_level != head_top_level: + raise ValueError( + "append-only changed config-wide fields that can affect scenarios " + f"outside its changelog scope: {config!r}" + ) + + for scenario in base_scenarios: + if scenario not in allowed_scenarios: + if base_scenarios[scenario] != head_scenarios[scenario]: + raise ValueError( + "append-only changed a scenario outside its changelog scope: " + f"{config!r} / {scenario!r}" + ) + continue + + def main(): parser = argparse.ArgumentParser() parser.add_argument("--base-ref", type=str, required=True) @@ -171,6 +425,17 @@ def main(): if not changelog_data: raise ValueError("No valid YAML entries found in the changelog additions.") + parsed_entries = [ChangelogEntry.model_validate(entry) for entry in changelog_data] + has_append_only = any(entry.append_only for entry in parsed_entries) + if has_append_only and not all(entry.append_only for entry in parsed_entries): + raise ValueError( + "append-only entries cannot share a sweep with regular changelog entries" + ) + if has_append_only and (args.all_evals or args.evals_only): + raise ValueError( + "append-only sweeps cannot use all-evals or evals-only modifiers" + ) + final_results = { "single_node": defaultdict(list), "multi_node": defaultdict(list), @@ -194,13 +459,37 @@ def main(): master_config = load_config_files(MASTER_CONFIGS) resolved_entries = [] - for entry_data in changelog_data: - entry = ChangelogEntry.model_validate(entry_data) + for entry in parsed_entries: all_configs = get_config_keys_from_master( entry.config_keys, master_config ) resolved_entries.append((entry, all_configs)) + base_inputs_context = None + base_inputs = None + if has_append_only: + base_inputs_context = generation_inputs_at_ref(args.base_ref) + base_inputs = base_inputs_context.__enter__() + base_master = load_config_files(base_inputs.config_files) + selected_config_scenarios: dict[str, set[str]] = defaultdict(set) + for entry, configs in resolved_entries: + for config in configs: + selected_config_scenarios[config].update( + entry.scenario_type or SCENARIO_TYPES + ) + selected_configs = selected_config_scenarios.keys() + missing_from_base = selected_configs - base_master.keys() + if missing_from_base: + raise ValueError( + "append-only requires every selected config to exist in the base " + f"revision; missing: {sorted(missing_from_base)}" + ) + validate_append_only_scope( + base_master, + master_config, + selected_config_scenarios, + ) + # Process all-evals entries first so their broader eval matrix wins when # the same config appears in multiple changelog entries. resolved_entries.sort(key=lambda item: not item[0].all_evals) @@ -230,7 +519,7 @@ def main(): benchmark_groups[unseen_scenarios].append(config) for scenarios, benchmark_configs in benchmark_groups.items(): - base_cmd = [ + head_cmd = [ "python3", GENERATE_SWEEPS_PY_SCRIPT, "test-config", @@ -238,21 +527,45 @@ def main(): *benchmark_configs, "--config-files", *MASTER_CONFIGS, + "--runner-config", + "configs/runners.yaml", "--no-evals", ] if scenarios != SCENARIO_TYPES: - base_cmd.extend(["--scenario-type", *scenarios]) + head_cmd.extend(["--scenario-type", *scenarios]) try: result = subprocess.run( - base_cmd, + head_cmd, capture_output=True, text=True, check=True, ) + head_results = json.loads(result.stdout) + if entry.append_only: + base_cmd = head_cmd.copy() + base_cmd[1] = base_inputs.generator_script + config_files_index = base_cmd.index("--config-files") + 1 + base_cmd[ + config_files_index:config_files_index + len(MASTER_CONFIGS) + ] = base_inputs.config_files + runner_config_index = base_cmd.index("--runner-config") + 1 + base_cmd[runner_config_index] = base_inputs.runner_config + base_result = subprocess.run( + base_cmd, + capture_output=True, + text=True, + check=True, + ) + head_results = append_only_delta( + json.loads(base_result.stdout), head_results + ) except subprocess.CalledProcessError as e: print(e.stderr) raise - all_benchmark_results.extend(json.loads(result.stdout)) + all_benchmark_results.extend(head_results) + + if entry.append_only: + continue eval_groups = defaultdict(list) for config in all_configs: @@ -299,10 +612,14 @@ def main(): ) all_eval_results.extend(entry_eval_results) + if base_inputs_context is not None: + base_inputs_context.__exit__(None, None, None) + if args.trim_conc: all_benchmark_results = trim_conc(all_benchmark_results) for result in all_benchmark_results: + result["recipe-fingerprint"] = recipe_fingerprint(result) if result.get("scenario-type") == "agentic-coding": if result.get("prefill") is not None: final_results["multi_node"]["agentic"].append(result) diff --git a/utils/process_result.py b/utils/process_result.py index a8bdc8cca6..ec7fa2693c 100644 --- a/utils/process_result.py +++ b/utils/process_result.py @@ -123,6 +123,7 @@ def record_power_internal_error( isl = base_env['ISL'] osl = base_env['OSL'] image = base_env['IMAGE'] +recipe_fingerprint = os.environ.get('RECIPE_FINGERPRINT', '') with open(f'{result_filename}.json') as f: bmk_result = json.load(f) @@ -137,6 +138,7 @@ def record_power_internal_error( 'precision': precision, 'spec_decoding': spec_decoding, 'disagg': disagg, + 'recipe_fingerprint': recipe_fingerprint, 'isl': int(isl), 'osl': int(osl), } diff --git a/utils/test_process_changelog.py b/utils/test_process_changelog.py index dfe677014e..252457590f 100644 --- a/utils/test_process_changelog.py +++ b/utils/test_process_changelog.py @@ -3,9 +3,45 @@ import json import subprocess import sys +from contextlib import nullcontext +from pathlib import Path from types import SimpleNamespace import process_changelog +from matrix_logic.generate_sweep_configs import generate_test_config_sweep +from matrix_logic.validation import validate_master_config + + +def _fixed_matrix_row( + conc, + *, + image="vllm/vllm-openai:v0.16.0", + tp=8, + duration=None, +): + return { + "image": image, + "model": "deepseek-ai/DeepSeek-V4-Pro", + "model-prefix": "dsv4", + "precision": "fp4", + "framework": "vllm", + "spec-decoding": "mtp", + "runner": "cluster:b300-nv", + "isl": 8192, + "osl": 1024, + "tp": tp, + "pp": 1, + "dcp-size": 1, + "pcp-size": 1, + "ep": 8, + "dp-attn": True, + "conc": conc, + "max-model-len": 10240, + "exp-name": f"dsv4_tp{tp}_conc{conc}", + "disagg": False, + "run-eval": False, + "eval-only": False, + } | ({"duration": duration} if duration is not None else {}) def _scenario_values(command): @@ -15,6 +51,21 @@ def _scenario_values(command): return command[index:] +def test_recipe_fingerprint_reaches_all_e2e_benchmark_jobs(): + workflow = (Path(__file__).parents[1] / ".github/workflows/e2e-tests.yml").read_text() + + assert workflow.count("uses: ./.github/workflows/benchmark") == 8 + assert workflow.count("recipe-fingerprint: ${{ matrix.config") == 8 + + +def test_recipe_fingerprint_disambiguates_result_and_artifact_names(): + repo_root = Path(__file__).parents[1] + for template_name in ("benchmark-tmpl.yml", "benchmark-multinode-tmpl.yml"): + template = (repo_root / ".github/workflows" / template_name).read_text() + assert 'RECIPE_FINGERPRINT: ${{ inputs.recipe-fingerprint }}' in template + assert 'recipe-${RECIPE_FINGERPRINT:0:16}' in template + + def test_trim_conc_supports_nested_backend_metadata(): common = { "model": "moonshotai/Kimi-K3", @@ -59,6 +110,406 @@ def test_config_key_expansion_is_deterministic_and_deduplicated(): assert result == ["config-b", "config-a"] +def test_append_only_delta_keeps_only_new_single_node_points(): + base = [_fixed_matrix_row(4), _fixed_matrix_row(8)] + head = [*base, _fixed_matrix_row(12)] + + delta = process_changelog.append_only_delta(base, head) + + assert [entry["conc"] for entry in delta] == [12] + + +def test_append_only_delta_slices_multinode_concurrency_lists(): + common = { + "image": "lmsysorg/sglang:v0.5.7", + "model": "deepseek-ai/DeepSeek-V4-Pro", + "model-prefix": "dsv4", + "precision": "fp4", + "framework": "dynamo-sglang", + "conc": [8, 16], + "exp-name": "dsv4-disagg", + } + + delta = process_changelog.append_only_delta( + [common], + [{**common, "conc": [8, 16, 24]}], + ) + + assert delta == [{**common, "conc": [24]}] + + +def test_append_only_delta_deduplicates_new_single_node_points(): + base = [_fixed_matrix_row(4)] + head = [base[0], _fixed_matrix_row(8), _fixed_matrix_row(8)] + + delta = process_changelog.append_only_delta(base, head) + + assert [entry["conc"] for entry in delta] == [8] + + +def test_append_only_delta_deduplicates_multinode_concurrency_lists(): + common = { + "image": "lmsysorg/sglang:v0.5.7", + "model": "deepseek-ai/DeepSeek-V4-Pro", + "framework": "dynamo-sglang", + "conc": [8, 16], + "exp-name": "dsv4-disagg", + } + + delta = process_changelog.append_only_delta( + [common], + [{**common, "conc": [8, 16, 24, 24]}], + ) + + assert delta == [{**common, "conc": [24]}] + + +def test_append_only_delta_rejects_image_changes(): + base = [_fixed_matrix_row(4)] + head = [ + _fixed_matrix_row(4, image="vllm/vllm-openai:v0.16.1"), + _fixed_matrix_row(8, image="vllm/vllm-openai:v0.16.1"), + ] + + try: + process_changelog.append_only_delta(base, head) + except ValueError as error: + assert "remove or modify" in str(error) + else: + raise AssertionError("image mutation should reject append-only mode") + + +def test_append_only_delta_allows_new_parallelism_with_its_points(): + base = [ + _fixed_matrix_row(1, tp=4), + _fixed_matrix_row(4, tp=4), + _fixed_matrix_row(8, tp=4), + ] + head = [ + *base, + _fixed_matrix_row(12, tp=8), + _fixed_matrix_row(16, tp=8), + ] + + delta = process_changelog.append_only_delta(base, head) + + assert [(entry["tp"], entry["conc"]) for entry in delta] == [ + (8, 12), + (8, 16), + ] + + +def test_append_only_delta_allows_any_new_recipe_while_preserving_old_recipe(): + base = [_fixed_matrix_row(4, duration=3600)] + head = [*base, _fixed_matrix_row(6, duration=300)] + + delta = process_changelog.append_only_delta(base, head) + + assert [(entry["duration"], entry["conc"]) for entry in delta] == [(300, 6)] + + +def test_append_only_delta_rejects_head_only_image_variant(): + base = [_fixed_matrix_row(4)] + head = [ + *base, + _fixed_matrix_row(8, image="vllm/vllm-openai:v0.16.1", tp=16), + ] + + try: + process_changelog.append_only_delta(base, head) + except ValueError as error: + assert "unchanged non-null image" in str(error) + else: + raise AssertionError("an append cannot fork the target curve's image") + + +def test_recipe_fingerprint_ignores_concurrency_and_experiment_name(): + first = _fixed_matrix_row(4) + second = _fixed_matrix_row(16) + + assert process_changelog.recipe_fingerprint(first) == ( + process_changelog.recipe_fingerprint(second) + ) + + +def test_recipe_fingerprint_changes_for_any_recipe_variant(): + base = _fixed_matrix_row(4, tp=4, duration=3600) + changed_parallelism = _fixed_matrix_row(4, tp=8, duration=3600) + changed_duration = _fixed_matrix_row(4, tp=4, duration=300) + + fingerprints = { + process_changelog.recipe_fingerprint(entry) + for entry in (base, changed_parallelism, changed_duration) + } + + assert len(fingerprints) == 3 + + +def test_append_only_delta_rejects_removed_parallelism_recipe(): + tp4 = _fixed_matrix_row(4, tp=4) + tp8 = _fixed_matrix_row(8, tp=8) + + try: + process_changelog.append_only_delta([tp4, tp8], [tp4]) + except ValueError as error: + assert "remove or modify" in str(error) + else: + raise AssertionError("removing a parallelism recipe should reject append-only mode") + + +def test_append_only_delta_rejects_modified_existing_recipe(): + base = [_fixed_matrix_row(4, duration=3600)] + head = [_fixed_matrix_row(4, duration=300)] + + try: + process_changelog.append_only_delta(base, head) + except ValueError as error: + assert "remove or modify" in str(error) + else: + raise AssertionError("modifying an existing recipe should reject append-only mode") + + +def test_append_only_delta_rejects_removed_existing_point(): + base = [_fixed_matrix_row(4), _fixed_matrix_row(8)] + head = [_fixed_matrix_row(8), _fixed_matrix_row(12)] + + try: + process_changelog.append_only_delta(base, head) + except ValueError as error: + assert "remove existing concurrency" in str(error) + else: + raise AssertionError("removing an existing point should reject append-only mode") + + +def test_append_only_scope_defers_selected_scenario_changes_to_matrix_comparison(): + base = { + "test-config": { + "image": "vllm/vllm-openai:v0.16.0", + "scenarios": { + "agentic-coding": { + "duration": 3600, + "search-space": [{"tp": 8, "conc-list": [1, 4]}], + } + }, + } + } + head = { + "test-config": { + "image": "vllm/vllm-openai:v0.16.0", + "scenarios": { + "agentic-coding": { + "duration": 1800, + "search-space": [{"tp": 8, "conc-list": [1, 4, 8]}], + } + }, + } + } + process_changelog.validate_append_only_scope( + base, head, {"test-config": {"agentic-coding"}} + ) + + +def test_append_only_scope_allows_additive_top_level_restructuring(): + router_a = {"name": "router-a", "version": "1"} + router_b = {"name": "router-b", "version": "2"} + base = { + "test-config": { + "image": "img", + "model": "m", + "model-prefix": "m", + "precision": "fp4", + "framework": "vllm", + "runner": "b200", + "multinode": False, + "router": router_a, + "scenarios": { + "fixed-seq-len": [ + { + "isl": 8192, + "osl": 1024, + "search-space": [{"tp": 4, "conc-list": [1, 4, 8]}], + } + ] + }, + } + } + head = json.loads(json.dumps(base)) + head["test-config"].pop("router") + search_space = head["test-config"]["scenarios"]["fixed-seq-len"][0][ + "search-space" + ] + search_space[0]["router"] = router_a + search_space.append( + {"tp": 8, "conc-list": [12, 16], "router": router_b} + ) + + validate_master_config(base) + validate_master_config(head) + args = SimpleNamespace( + config_keys=["test-config"], + seq_lens=None, + conc=None, + scenario_type=["fixed-seq-len"], + runner_node_filter=None, + ) + base_rows = generate_test_config_sweep(args, base) + head_rows = generate_test_config_sweep(args, head) + + process_changelog.validate_append_only_scope( + base, head, {"test-config": {"fixed-seq-len"}} + ) + delta = process_changelog.append_only_delta(base_rows, head_rows) + + assert [(row["tp"], row["conc"], row["router"]) for row in delta] == [ + (8, 12, router_b), + (8, 16, router_b), + ] + + +def test_append_only_scope_rejects_global_change_with_unselected_scenario(): + base = { + "test-config": { + "router": {"name": "dynamo-router", "version": "0.8.1"}, + "scenarios": { + "fixed-seq-len": {"search-space": [{"tp": 4, "conc-list": [1]}]}, + "agentic-coding": {"search-space": [{"tp": 4, "conc-list": [1]}]}, + }, + } + } + head = { + "test-config": { + "router": {"name": "dynamo-router", "version": "0.8.2"}, + "scenarios": base["test-config"]["scenarios"], + } + } + + try: + process_changelog.validate_append_only_scope( + base, head, {"test-config": {"fixed-seq-len"}} + ) + except ValueError as error: + assert "config-wide fields" in str(error) + else: + raise AssertionError("global changes may not affect an unselected scenario") + + +def test_append_only_scope_rejects_changes_to_unselected_scenario(): + base = { + "test-config": { + "scenarios": { + "fixed-seq-len": {"search-space": [{"tp": 4, "conc-list": [1]}]}, + "agentic-coding": {"search-space": [{"tp": 4, "conc-list": [1]}]}, + } + } + } + head = { + "test-config": { + "scenarios": { + "fixed-seq-len": {"search-space": [{"tp": 4, "conc-list": [1]}]}, + "agentic-coding": { + "search-space": [{"tp": 4, "conc-list": [1, 4]}] + }, + } + } + } + + try: + process_changelog.validate_append_only_scope( + base, head, {"test-config": {"fixed-seq-len"}} + ) + except ValueError as error: + assert "outside its changelog scope" in str(error) + else: + raise AssertionError("unselected scenario changes should reject append-only mode") + + +def test_append_only_scope_allows_range_to_list_expansion(): + base = { + "test-config": { + "image": "vllm/vllm-openai:v0.16.0", + "scenarios": { + "fixed-seq-len": { + "search-space": [{"tp": 8, "conc-start": 4, "conc-end": 64}], + } + }, + } + } + head = { + "test-config": { + "image": "vllm/vllm-openai:v0.16.0", + "scenarios": { + "fixed-seq-len": { + "search-space": [{"tp": 8, "conc-list": [4, 16, 32, 64]}], + } + }, + } + } + process_changelog.validate_append_only_scope( + base, head, {"test-config": {"fixed-seq-len"}} + ) + + +def test_append_only_main_runs_only_added_points_and_skips_evals( + monkeypatch, + capsys, +): + added_yaml = """ +- config-keys: + - test-config + description: + - Add one concurrency point without rerunning the curve + pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/1 + append-only: true +""" + base_rows = [_fixed_matrix_row(4)] + head_rows = [*base_rows, _fixed_matrix_row(8)] + commands = [] + + monkeypatch.setattr(process_changelog, "get_added_lines", lambda *_: added_yaml) + monkeypatch.setattr( + process_changelog, + "generation_inputs_at_ref", + lambda *_: nullcontext( + process_changelog.GenerationInputs( + config_files=["base-nvidia.yaml", "base-amd.yaml"], + generator_script="base-generate-sweep-configs.py", + runner_config="base-runners.yaml", + ) + ), + ) + monkeypatch.setattr( + process_changelog, + "load_config_files", + lambda _: {"test-config": {"image": "vllm/vllm-openai:v0.16.0"}}, + ) + + def fake_run(command, **kwargs): + commands.append(command) + rows = base_rows if "base-nvidia.yaml" in command else head_rows + return SimpleNamespace(stdout=json.dumps(rows)) + + monkeypatch.setattr(subprocess, "run", fake_run) + monkeypatch.setattr(sys, "argv", [ + "process_changelog.py", + "--base-ref", "base", + "--head-ref", "head", + "--changelog-file", "perf-changelog.yaml", + ]) + + process_changelog.main() + + output = json.loads(capsys.readouterr().out) + assert [row["conc"] for row in output["single_node"]["8k1k"]] == [8] + assert len(output["single_node"]["8k1k"][0]["recipe-fingerprint"]) == 64 + assert output["evals"] == [] + assert output["changelog_metadata"]["entries"][0]["append-only"] is True + assert len(commands) == 2 + assert commands[0][1] == process_changelog.GENERATE_SWEEPS_PY_SCRIPT + assert commands[1][1] == "base-generate-sweep-configs.py" + assert commands[0][commands[0].index("--runner-config") + 1] == "configs/runners.yaml" + assert commands[1][commands[1].index("--runner-config") + 1] == "base-runners.yaml" + + def test_all_evals_skips_benchmarks_and_uses_all_evals_generator_flag( monkeypatch, capsys, diff --git a/utils/test_process_result.py b/utils/test_process_result.py index 4d5219010f..209f8b91fe 100644 --- a/utils/test_process_result.py +++ b/utils/test_process_result.py @@ -52,6 +52,7 @@ def base_env_vars(): "DISAGG": "false", "MODEL_PREFIX": "dsr1", "IMAGE": "test-image", + "RECIPE_FINGERPRINT": "a" * 64, } @@ -215,6 +216,7 @@ def test_single_node_processing(self, tmp_path, sample_benchmark_result, single_ assert output_data["isl"] == 1024 assert output_data["osl"] == 1024 assert output_data["disagg"] is False + assert output_data["recipe_fingerprint"] == "a" * 64 # Verify single-node specific fields assert output_data["is_multinode"] is False