From 90c1303303696db2d4e6981b25209962ca8ab793 Mon Sep 17 00:00:00 2001 From: Horde Date: Sun, 2 Aug 2026 22:40:44 +0000 Subject: [PATCH 1/2] Complete the L40S runner stability qualification The 2026-07-28 qualification run collected only 57 of 135 samples and produced no verdict. Two independent faults caused the shortfall, and both are fixed here. Most Newton tasks crashed on `from newton.solvers import SolverNotifyFlags`. Staging source was 145 commits behind develop while the CI image was built from recent develop, where that symbol had been replaced by ModelFlags. Merging develop into staging removes the mismatch. One allocation died partway through because five parallel allocations shared a single Warp JIT cache directory and raced on creating it. Each allocation now derives its own cache root from the run id, attempt, and run label. Separately, a crashed benchmark could exit zero while FPS regressions were advisory, so execution health is now always enforced. To keep that from failing PRs for reasons outside their control, a crash whose log shows the image missing a symbol the source pins is reported as a stale CI image and stays advisory. Only packages the image installs qualify; a missing Isaac Lab symbol is still a real defect. The qualification workflow no longer promotes samples to the rolling baseline or chains a second gate run. It measures the pool and reports a verdict. --- .../perf-smoke-runner-stability.yaml | 60 +++++------ tools/perf_smoke_test/aggregate.py | 72 +++++++++++-- tools/perf_smoke_test/environment_skew.py | 85 +++++++++++++++ tools/perf_smoke_test/seed_baselines.py | 15 ++- .../test/test_aggregate_reseed.py | 100 ++++++++++++++++-- .../test/test_environment_skew.py | 68 ++++++++++++ .../test/test_runner_stability.py | 12 ++- .../test/test_seed_ancestry.py | 9 ++ 8 files changed, 362 insertions(+), 59 deletions(-) create mode 100644 tools/perf_smoke_test/environment_skew.py create mode 100644 tools/perf_smoke_test/test/test_environment_skew.py diff --git a/.github/workflows/perf-smoke-runner-stability.yaml b/.github/workflows/perf-smoke-runner-stability.yaml index 8f6780868219..d4ebe3d61690 100644 --- a/.github/workflows/perf-smoke-runner-stability.yaml +++ b/.github/workflows/perf-smoke-runner-stability.yaml @@ -3,11 +3,11 @@ # # SPDX-License-Identifier: BSD-3-Clause -# One-time staging qualification for the L40S runner pool. The workflow waits -# for the normal gate and baseline seeder from the same push to finish, then fans -# the commit out to five independent allocations. Each allocation collects three -# FPS samples per bucket without publishing baselines. The final CPU job combines -# all 15 samples and fails unless the pool meets the predeclared stability policy. +# One-time staging qualification for the L40S runner pool. The workflow waits for +# the commit's normal gate run to pass and for the L40 pool to go quiet, then fans +# the commit out to five independent allocations so per-runner and run-to-run FPS +# spread can be measured separately. Qualification only reports a verdict; it +# never writes baselines. name: Performance Smoke - L40S Runner Stability @@ -20,8 +20,9 @@ on: permissions: actions: read - # Reusable workflows cannot elevate their caller's token. The seeder requests - # write access for its normal publish mode, while this caller forces dry-run. + # The seeder this workflow calls declares contents: write, and a caller cannot + # grant a reusable workflow more than it holds. The stability allocations run + # with dry_run: true, so nothing is actually written. contents: write concurrency: @@ -30,66 +31,53 @@ concurrency: jobs: wait_for_quiet_pool: - name: Wait for sibling performance workflows - if: ${{ github.event_name == 'push' }} + name: Wait for initial gate and quiet pool runs-on: ubuntu-latest - timeout-minutes: 350 + timeout-minutes: 180 permissions: actions: read contents: read steps: - - name: Wait for the gate and seeder from this push + - name: Wait for the initial gate and competing L40 work env: GH_TOKEN: ${{ github.token }} run: | set -euo pipefail - REQUIRED_WORKFLOWS=( - "Performance Smoke Test" - "Performance Smoke - Seed Baselines" - ) L40_POOL_WORKFLOWS=( "Performance Smoke Test" - "Performance Smoke - Seed Baselines" "Perf Smoke — Publish CI Image" "Perf Smoke — Auto Era Roll" ) - DEADLINE=$((SECONDS + 20400)) + DEADLINE=$((SECONDS + 10200)) QUIET_POLLS=0 while (( SECONDS < DEADLINE )); do RUNS="$(gh run list \ --repo "${GITHUB_REPOSITORY}" \ --limit 100 \ - --json headSha,workflowName,status)" - PREREQUISITES_READY=true - for workflow_name in "${REQUIRED_WORKFLOWS[@]}"; do - MATCH_COUNT="$(jq --arg name "${workflow_name}" \ - --arg sha "${GITHUB_SHA}" \ - '[.[] | select(.workflowName == $name and .headSha == $sha)] | length' <<<"${RUNS}")" - ACTIVE_COUNT="$(jq --arg name "${workflow_name}" \ - --arg sha "${GITHUB_SHA}" \ - '[.[] | select(.workflowName == $name and .headSha == $sha and .status != "completed")] | length' \ - <<<"${RUNS}")" - if [ "${MATCH_COUNT}" -eq 0 ] || [ "${ACTIVE_COUNT}" -ne 0 ]; then - PREREQUISITES_READY=false - break - fi - done + --json conclusion,headSha,workflowName,status)" + GATE_CONCLUSION="$(jq -r --arg sha "${GITHUB_SHA}" \ + '[.[] | select(.workflowName == "Performance Smoke Test" and .headSha == $sha and .status == "completed")] + | first | .conclusion // ""' <<<"${RUNS}")" + if [ -n "${GATE_CONCLUSION}" ] && [ "${GATE_CONCLUSION}" != "success" ]; then + echo "::error::Initial pinned-image gate concluded ${GATE_CONCLUSION}." + exit 1 + fi ACTIVE_POOL_RUNS=0 for workflow_name in "${L40_POOL_WORKFLOWS[@]}"; do ACTIVE_COUNT="$(jq --arg name "${workflow_name}" \ '[.[] | select(.workflowName == $name and .status != "completed")] | length' <<<"${RUNS}")" ACTIVE_POOL_RUNS=$((ACTIVE_POOL_RUNS + ACTIVE_COUNT)) done - if [ "${PREREQUISITES_READY}" = true ] && [ "${ACTIVE_POOL_RUNS}" -eq 0 ]; then + if [ "${GATE_CONCLUSION}" = "success" ] && [ "${ACTIVE_POOL_RUNS}" -eq 0 ]; then QUIET_POLLS=$((QUIET_POLLS + 1)) if [ "${QUIET_POLLS}" -ge 2 ]; then - echo "Sibling workflows completed and the L40 pool stayed quiet; starting qualification." + echo "Initial gate passed and the L40 pool stayed quiet; starting qualification." exit 0 fi echo "L40 pool is quiet; confirming for one more polling interval..." else QUIET_POLLS=0 - echo "Waiting for sibling workflows and other L40 workloads to finish..." + echo "Waiting for the initial gate and other L40 workloads to finish..." fi sleep 60 done @@ -99,7 +87,7 @@ jobs: stability_sample: name: Runner allocation ${{ matrix.allocation }} needs: [wait_for_quiet_pool] - if: ${{ always() && (needs.wait_for_quiet_pool.result == 'success' || needs.wait_for_quiet_pool.result == 'skipped') }} + if: ${{ needs.wait_for_quiet_pool.result == 'success' }} strategy: fail-fast: false max-parallel: 5 diff --git a/tools/perf_smoke_test/aggregate.py b/tools/perf_smoke_test/aggregate.py index 2f4fac698ccb..c17305a7b8d6 100644 --- a/tools/perf_smoke_test/aggregate.py +++ b/tools/perf_smoke_test/aggregate.py @@ -30,6 +30,7 @@ update_baselines_git, ) from contracts import BenchResult # noqa: E402 +from environment_skew import DependencySkew, detect_dependency_skew # noqa: E402 from gate_config import BASELINE_PUSH_RETRIES, MIN_BASELINE_SAMPLES, load_gate_config # noqa: E402 from gate_types import FpsMeanThreshold, OracleVerdict # noqa: E402 from gpu_identity import canonical_gpu_model, gpu_model_config_keys # noqa: E402 @@ -160,6 +161,18 @@ def _runtime_context(bench_result: BenchResult) -> tuple[str, str]: return str(gpu_name or "N/A"), runtime or "N/A" +def _skewed_rows(rows: list[tuple]) -> list[tuple[str, str, DependencySkew]]: + """Return ``(task_id, backend, skew)`` for failures caused by a stale CI image.""" + skewed = [] + for result, bench_result in rows: + if result.verdict != OracleVerdict.HARD_FAILURE: + continue + skew = detect_dependency_skew(bench_result.stdout_tail) + if skew is not None: + skewed.append((result.task_id, result.backend, skew)) + return skewed + + def _row_explanation(result) -> str: """Explain one verdict in reviewer-facing language.""" if result.verdict == OracleVerdict.HARD_FAILURE: @@ -227,16 +240,41 @@ def _build_technical_table(rows: list[tuple]) -> str: return "\n".join(lines) +def _build_stale_image_section(skewed: list[tuple[str, str, DependencySkew]]) -> str: + """Explain that a stale CI image, not the PR, caused these failures.""" + packages = sorted({skew.package for _, _, skew in skewed}) + affected = "\n".join(f"- `{task_id}` ({backend}): {skew.describe()}" for task_id, backend, skew in skewed) + return ( + "### Stale CI image\n\n" + f"The prebuilt CI image does not match this PR's pinned {' and '.join(packages)} version, so the " + "tasks below crashed before producing any FPS. This reflects the image, not the change under review, " + "so these results are advisory and do not fail the check.\n\n" + f"{affected}\n\n" + "This resolves itself once the CI image is rebuilt for the current dependency pins. Re-run the gate " + "after the next image publish to get real numbers for these tasks." + ) + + def _build_summary_markdown(rows: list[tuple], *, blocking: bool) -> str: """Build reviewer-first Markdown for the sticky PR comment.""" counts = {verdict: 0 for verdict in OracleVerdict} for result, _ in rows: counts[result.verdict] += 1 + skewed = _skewed_rows(rows) + skewed_keys = {(task_id, backend) for task_id, backend, _ in skewed} + unexplained_failures = sum( + 1 + for result, _ in rows + if result.verdict == OracleVerdict.HARD_FAILURE and (result.task_id, result.backend) not in skewed_keys + ) + if not rows: overall = "❌ No benchmark results were produced" - elif counts[OracleVerdict.HARD_FAILURE]: + elif unexplained_failures: overall = "❌ One or more benchmarks failed before producing usable performance data" + elif skewed: + overall = "⚠️ The CI image is stale for this PR, so some tasks could not be measured" elif counts[OracleVerdict.BLOCK]: overall = "🚫 One or more blocking-level performance regressions were detected" elif counts[OracleVerdict.WARN]: @@ -269,10 +307,14 @@ def _build_summary_markdown(rows: list[tuple], *, blocking: bool) -> str: else: gpu_name, runtime = "N/A", "N/A" - return "\n\n".join( + sections = [ + f"### Overall result\n\n**{overall}**\n\n{count_summary}\n\n{mode}", + f"### Run context\n\n- **GPU:** {gpu_name}\n- **Runtime:** {runtime}", + ] + if skewed: + sections.append(_build_stale_image_section(skewed)) + sections.extend( ( - f"### Overall result\n\n**{overall}**\n\n{count_summary}\n\n{mode}", - f"### Run context\n\n- **GPU:** {gpu_name}\n- **Runtime:** {runtime}", "### How to read this\n\n" "Start with **BLOCK** and **HARD FAILURE**, then review any **WARN** rows.\n\n" "- **✅ PASS:** no meaningful slowdown was detected.\n" @@ -292,6 +334,7 @@ def _build_summary_markdown(rows: list[tuple], *, blocking: bool) -> str: f"{_build_technical_table(rows)}\n\n", ) ) + return "\n\n".join(sections) def _write_github_output(**values) -> None: @@ -407,7 +450,14 @@ def main() -> int: if oracle_result.verdict == OracleVerdict.BLOCK: has_block = True elif oracle_result.verdict == OracleVerdict.HARD_FAILURE: - has_hard_failure = True + # A crash caused by the image lacking a symbol this source pins is a + # property of the image, not of the change under test, so it is + # reported loudly but never fails the PR. + skew = detect_dependency_skew(bench_result.stdout_tail) + if skew is None: + has_hard_failure = True + else: + print(f"[aggregate] {task_id}/{backend}: stale CI image; {skew.describe()}") if ( allow_update @@ -526,11 +576,13 @@ def main() -> int: if baseline_update_failed: return 1 - if blocking: - if has_block: - return 1 - if has_hard_failure: - return 2 + # Benchmark execution health is never advisory. A crash, missing result, or + # invalid benchmark must fail even while FPS regressions are being rolled out + # in advisory mode. + if has_hard_failure: + return 2 + if blocking and has_block: + return 1 return 0 diff --git a/tools/perf_smoke_test/environment_skew.py b/tools/perf_smoke_test/environment_skew.py new file mode 100644 index 000000000000..17729790164d --- /dev/null +++ b/tools/perf_smoke_test/environment_skew.py @@ -0,0 +1,85 @@ +# Copyright (c) 2022-2026, The Isaac Lab Project Developers (https://github.com/isaac-sim/IsaacLab/blob/main/CONTRIBUTORS.md). +# All rights reserved. +# +# SPDX-License-Identifier: BSD-3-Clause + +"""Detection of source-versus-image dependency skew in a failed benchmark. + +The gate bind-mounts Isaac Lab source over a prebuilt CI image, but the image +supplies the installed third-party packages (Newton, Warp, Isaac Sim). Between a +dependency-pin change landing on ``develop`` and the next image publish, a PR's +source can reference a symbol the installed package does not have yet, which +crashes every affected task before any FPS is measured. + +That crash says nothing about the PR's performance, so it must not read as a +performance failure. This module recognizes the crash signature so the gate can +report a stale image and stay advisory for the affected tasks instead. + +Only packages the *image* installs are eligible. A missing symbol in Isaac Lab's +own source is a genuine defect in the change under test and still fails. +""" + +from __future__ import annotations + +import re +from dataclasses import dataclass + +# Packages installed into the CI image rather than bind-mounted from the PR. +IMAGE_PROVIDED_PACKAGES: frozenset[str] = frozenset( + { + "carb", + "isaacsim", + "mujoco", + "mujoco_warp", + "newton", + "omni", + "pxr", + "warp", + } +) + +# Python spells "this name is not in the installed package" three ways. +_MISSING_NAME_PATTERNS: tuple[re.Pattern[str], ...] = ( + re.compile(r"ImportError: cannot import name ['\"](?P\w+)['\"] from ['\"](?P[\w.]+)['\"]"), + re.compile(r"ModuleNotFoundError: No module named ['\"](?P[\w.]+)['\"]"), + re.compile(r"AttributeError: module ['\"](?P[\w.]+)['\"] has no attribute ['\"](?P\w+)['\"]"), +) + + +@dataclass(frozen=True) +class DependencySkew: + """One detected mismatch between the PR's source and the image's packages.""" + + package: str + module: str + symbol: str | None + + def describe(self) -> str: + """Return a one-line reviewer-facing description of the mismatch.""" + if self.symbol: + return f"`{self.module}` in the CI image has no `{self.symbol}`" + return f"`{self.module}` is not installed in the CI image" + + +def detect_dependency_skew(log_text: str | None) -> DependencySkew | None: + """Return the dependency skew a benchmark log indicates, if any. + + Args: + log_text: Captured benchmark output, typically ``BenchResult.stdout_tail``. + + Returns: + The detected mismatch, or ``None`` when the log shows no missing symbol + from an image-provided package. + """ + if not log_text: + return None + for pattern in _MISSING_NAME_PATTERNS: + match = pattern.search(log_text) + if match is None: + continue + module = match.group("module") + package = module.split(".", 1)[0] + if package not in IMAGE_PROVIDED_PACKAGES: + continue + return DependencySkew(package=package, module=module, symbol=match.groupdict().get("symbol")) + return None diff --git a/tools/perf_smoke_test/seed_baselines.py b/tools/perf_smoke_test/seed_baselines.py index 93255c3a7739..d13160bd04ff 100755 --- a/tools/perf_smoke_test/seed_baselines.py +++ b/tools/perf_smoke_test/seed_baselines.py @@ -394,6 +394,17 @@ def _cache_bucket_path(cache_root: Path, target_branch: str, commit: str, task_i ) +def _cache_run_name() -> str: + """Return an allocation-isolated cache directory name for this invocation.""" + run_id = os.environ.get("GITHUB_RUN_ID", f"local-{os.getpid()}") + run_attempt = os.environ.get("GITHUB_RUN_ATTEMPT", "0") + run_label = os.environ.get("PERF_SMOKE_RUN_LABEL", "").strip() + name = f"run-{run_id}-attempt-{run_attempt}" + if run_label: + name += f"-{_safe_path_component(run_label)}" + return name + + def _cleanup_run_dir(run_dir: Path) -> None: """Remove a disposable directory for one seeder invocation.""" if not run_dir.exists(): @@ -665,9 +676,7 @@ def main() -> int: else: seed_src_dir = _create_seed_source_dir() atexit.register(_cleanup_run_dir, seed_src_dir) - run_id = os.environ.get("GITHUB_RUN_ID", f"local-{os.getpid()}") - run_attempt = os.environ.get("GITHUB_RUN_ATTEMPT", "0") - cache_run = f"run-{run_id}-attempt-{run_attempt}" + cache_run = _cache_run_name() jit_cache_root = workdir / "jit-cache" / "seed" / cache_run kit_cache_root = workdir / "kit-cache" / "seed" / cache_run atexit.register(_cleanup_run_dir, jit_cache_root) diff --git a/tools/perf_smoke_test/test/test_aggregate_reseed.py b/tools/perf_smoke_test/test/test_aggregate_reseed.py index b279773b6847..cb833426867a 100644 --- a/tools/perf_smoke_test/test/test_aggregate_reseed.py +++ b/tools/perf_smoke_test/test/test_aggregate_reseed.py @@ -32,7 +32,7 @@ _RUNTIME_HASH = "runtime-a" -def _bench_result(*, fps: float | None = 100.0, info_present: bool = True) -> BenchResult: +def _bench_result(*, fps: float | None = 100.0, info_present: bool = True, stdout_tail: str = "") -> BenchResult: launch_config = { "task_id": "Isaac-Cartpole-Direct", "backend": "physx", @@ -52,6 +52,7 @@ def _bench_result(*, fps: float | None = 100.0, info_present: bool = True) -> Be backend_key="physx", preset="default", was_retried=False, + stdout_tail=stdout_tail, perf_smoke_test_info_present=info_present, raw_fps_mean=fps, raw_fps_std=1.0 if fps is not None else None, @@ -95,10 +96,14 @@ def _seed_flat_baseline(baselines_dir: Path, count: int) -> None: ) -def _run_aggregate(tmp_path: Path, monkeypatch, bench_result: BenchResult, baseline_count: int) -> dict[str, str]: +def _run_aggregate( + tmp_path: Path, monkeypatch, bench_result: BenchResult, baseline_count: int +) -> tuple[int, dict[str, str]]: artifacts_dir = tmp_path / "artifacts" baselines_dir = tmp_path / "baselines" output_file = tmp_path / "gh_output.txt" + gate_config = tmp_path / "gate_config.json" + gate_config.write_text('{"blocking": false}', encoding="utf-8") _write_artifact(artifacts_dir, bench_result) if baseline_count: _seed_flat_baseline(baselines_dir, baseline_count) @@ -119,9 +124,11 @@ def _run_aggregate(tmp_path: Path, monkeypatch, bench_result: BenchResult, basel str(baselines_dir), "--allow_baseline_update", "false", + "--gate_config", + str(gate_config), ], ) - aggregate.main() + exit_code = aggregate.main() outputs: dict[str, str] = {} if output_file.exists(): @@ -129,38 +136,115 @@ def _run_aggregate(tmp_path: Path, monkeypatch, bench_result: BenchResult, basel if "=" in line: key, _, value = line.partition("=") outputs[key] = value - return outputs + return exit_code, outputs def test_reseed_flagged_when_no_baseline(tmp_path, monkeypatch) -> None: """A valid measurement with zero matching samples opens a bucket that needs reseeding.""" - outputs = _run_aggregate(tmp_path, monkeypatch, _bench_result(fps=100.0), baseline_count=0) + exit_code, outputs = _run_aggregate(tmp_path, monkeypatch, _bench_result(fps=100.0), baseline_count=0) + assert exit_code == 0 assert outputs.get("reseed_tasks") == "Isaac-Cartpole-Direct" assert outputs.get("reseed_min_samples") == str(MIN_BASELINE_SAMPLES) def test_reseed_flagged_when_window_insufficient(tmp_path, monkeypatch) -> None: """Fewer than MIN_BASELINE_SAMPLES matching samples still counts as under-filled.""" - outputs = _run_aggregate(tmp_path, monkeypatch, _bench_result(fps=100.0), baseline_count=MIN_BASELINE_SAMPLES - 1) + exit_code, outputs = _run_aggregate( + tmp_path, monkeypatch, _bench_result(fps=100.0), baseline_count=MIN_BASELINE_SAMPLES - 1 + ) + assert exit_code == 0 assert outputs.get("reseed_tasks") == "Isaac-Cartpole-Direct" def test_reseed_not_flagged_when_bucket_full(tmp_path, monkeypatch) -> None: """A fully populated bucket must not trigger a reseed.""" - outputs = _run_aggregate(tmp_path, monkeypatch, _bench_result(fps=100.0), baseline_count=MIN_BASELINE_SAMPLES) + exit_code, outputs = _run_aggregate( + tmp_path, monkeypatch, _bench_result(fps=100.0), baseline_count=MIN_BASELINE_SAMPLES + ) + assert exit_code == 0 assert "reseed_tasks" not in outputs def test_reseed_not_flagged_without_valid_measurement(tmp_path, monkeypatch) -> None: """A crashed run (no measurement) must not reseed even with an empty bucket.""" - outputs = _run_aggregate(tmp_path, monkeypatch, _bench_result(fps=None, info_present=False), baseline_count=0) + exit_code, outputs = _run_aggregate( + tmp_path, monkeypatch, _bench_result(fps=None, info_present=False), baseline_count=0 + ) + + assert exit_code == 2 + assert "reseed_tasks" not in outputs + + +_STALE_IMAGE_LOG = "ImportError: cannot import name 'SolverNotifyFlags' from 'newton.solvers'" + + +def test_crash_from_stale_image_does_not_fail_the_gate(tmp_path, monkeypatch) -> None: + """An image missing a symbol this source pins is not the PR's fault, so it stays advisory.""" + bench_result = _bench_result(fps=None, info_present=False, stdout_tail=_STALE_IMAGE_LOG) + exit_code, outputs = _run_aggregate(tmp_path, monkeypatch, bench_result, baseline_count=0) + + assert exit_code == 0 assert "reseed_tasks" not in outputs +def test_stale_image_is_explained_in_the_sticky_comment() -> None: + """Reviewers are told the image is stale rather than left reading a bare crash.""" + result = SimpleNamespace( + task_id="Isaac-Cartpole-Direct", + backend="newton", + verdict=OracleVerdict.HARD_FAILURE, + measured_fps=None, + baseline_fps=None, + regression_pct=None, + baseline_sample_count=0, + threshold_source="no_baseline", + hard_floor_fps=None, + failure_phase="import", + was_retried=False, + note=None, + crossed_thresholds=[], + ) + bench_result = _bench_result(fps=None, info_present=False, stdout_tail=_STALE_IMAGE_LOG) + + summary = aggregate._build_summary_markdown([(result, bench_result)], blocking=True) + + assert "### Stale CI image" in summary + assert "The CI image is stale for this PR" in summary + assert "advisory and do not fail the check" in summary + assert "no `SolverNotifyFlags`" in summary + # The failure is surfaced, just not attributed to the change under review. + assert "failed before producing usable performance data" not in summary + + +def test_genuine_crash_still_fails_and_is_not_excused() -> None: + """A crash with no image-skew signature keeps its hard-failure framing.""" + result = SimpleNamespace( + task_id="Isaac-Cartpole-Direct", + backend="newton", + verdict=OracleVerdict.HARD_FAILURE, + measured_fps=None, + baseline_fps=None, + regression_pct=None, + baseline_sample_count=0, + threshold_source="no_baseline", + hard_floor_fps=None, + failure_phase="runtime", + was_retried=False, + note=None, + crossed_thresholds=[], + ) + bench_result = _bench_result(fps=None, info_present=False, stdout_tail="RuntimeError: CUDA out of memory") + + summary = aggregate._build_summary_markdown([(result, bench_result)], blocking=True) + + assert "### Stale CI image" not in summary + assert "failed before producing usable performance data" in summary + + def test_sticky_summary_explains_results_in_reviewer_language() -> None: """The sticky comment leads with actionable guidance and keeps diagnostics available.""" result = SimpleNamespace( diff --git a/tools/perf_smoke_test/test/test_environment_skew.py b/tools/perf_smoke_test/test/test_environment_skew.py new file mode 100644 index 000000000000..423bdec23176 --- /dev/null +++ b/tools/perf_smoke_test/test/test_environment_skew.py @@ -0,0 +1,68 @@ +# Copyright (c) 2022-2026, The Isaac Lab Project Developers (https://github.com/isaac-sim/IsaacLab/blob/main/CONTRIBUTORS.md). +# All rights reserved. +# +# SPDX-License-Identifier: BSD-3-Clause + +"""Tests for telling a stale CI image apart from a genuine defect in the PR.""" + +from __future__ import annotations + +import sys +from pathlib import Path + +_GATE_DIR = Path(__file__).resolve().parents[1] +if str(_GATE_DIR) not in sys.path: + sys.path.insert(0, str(_GATE_DIR)) + +from environment_skew import detect_dependency_skew # noqa: E402 + +# Verbatim from the 2026-07-28 staging run, where source pinning a newer Newton +# met an image built before SolverNotifyFlags was replaced by ModelFlags. +_SOLVER_NOTIFY_FLAGS_LOG = """ +Traceback (most recent call last): + File "/workspace/isaaclab/source/isaaclab_newton/isaaclab_newton/physics/newton_manager.py", line 21, in + from newton.solvers import SolverNotifyFlags +ImportError: cannot import name 'SolverNotifyFlags' from 'newton.solvers' +""" + + +def test_missing_newton_symbol_is_reported_as_stale_image() -> None: + """The crash that broke the last qualification run must be recognized.""" + skew = detect_dependency_skew(_SOLVER_NOTIFY_FLAGS_LOG) + + assert skew is not None + assert skew.package == "newton" + assert skew.symbol == "SolverNotifyFlags" + assert "no `SolverNotifyFlags`" in skew.describe() + + +def test_missing_image_module_is_reported_as_stale_image() -> None: + """A package the image should provide but does not is also image skew.""" + skew = detect_dependency_skew("ModuleNotFoundError: No module named 'newton.solvers.kamino'") + + assert skew is not None + assert skew.package == "newton" + assert skew.symbol is None + assert "is not installed" in skew.describe() + + +def test_missing_module_attribute_is_reported_as_stale_image() -> None: + """Warp exposing no such attribute means the installed Warp predates the pin.""" + skew = detect_dependency_skew("AttributeError: module 'warp' has no attribute 'sparse_matmul'") + + assert skew is not None + assert skew.package == "warp" + assert skew.symbol == "sparse_matmul" + + +def test_missing_isaaclab_symbol_is_not_excused() -> None: + """Isaac Lab source is mounted from the PR, so its own broken import is a real defect.""" + log = "ImportError: cannot import name 'ArticulationCfg' from 'isaaclab.assets'" + + assert detect_dependency_skew(log) is None + + +def test_clean_log_reports_no_skew() -> None: + """A run that never raised an import error is not skew.""" + assert detect_dependency_skew("Step Frametimes: 3.1 3.0 3.2") is None + assert detect_dependency_skew(None) is None diff --git a/tools/perf_smoke_test/test/test_runner_stability.py b/tools/perf_smoke_test/test/test_runner_stability.py index e0f520017bd7..90d5f96c32f2 100644 --- a/tools/perf_smoke_test/test/test_runner_stability.py +++ b/tools/perf_smoke_test/test/test_runner_stability.py @@ -326,11 +326,12 @@ def test_staging_workflow_fans_out_complete_independent_evidence() -> None: assert trigger["push"]["paths"] == [".github/workflows/perf-smoke-runner-stability.yaml"] assert "workflow_dispatch" not in trigger assert workflow["permissions"]["contents"] == "write" + wait_job = workflow["jobs"]["wait_for_quiet_pool"] + assert "needs" not in wait_job assert wait_job["permissions"]["contents"] == "read" wait_step = wait_job["steps"][0] assert "Performance Smoke Test" in wait_step["run"] - assert "Performance Smoke - Seed Baselines" in wait_step["run"] assert "Perf Smoke — Publish CI Image" in wait_step["run"] assert "Perf Smoke — Auto Era Roll" in wait_step["run"] assert "QUIET_POLLS" in wait_step["run"] @@ -346,7 +347,8 @@ def test_staging_workflow_fans_out_complete_independent_evidence() -> None: assert "${{ matrix.allocation }}" in sample_job["with"]["concurrency_group"] assert sample_job["secrets"] == {"NGC_API_KEY": "${{ secrets.NGC_API_KEY }}"} - qualify_steps = workflow["jobs"]["qualify"]["steps"] + qualify_job = workflow["jobs"]["qualify"] + qualify_steps = qualify_job["steps"] report_step = next(step for step in qualify_steps if step.get("name") == "Build qualification report") assert "--require_ready" in report_step["run"] assert "--minimum_distinct_runners 3" in report_step["run"] @@ -354,6 +356,12 @@ def test_staging_workflow_fans_out_complete_independent_evidence() -> None: assert download_step["continue-on-error"] is True assert any(step.get("name") == "Report artifact download failure" for step in qualify_steps) assert any(step.get("name") == "Fail on artifact download error" for step in qualify_steps) + + # Qualification only measures the pool. It must not publish baselines or chain + # another gate run, so a bad verdict can never contaminate the rolling window. + assert qualify_job["permissions"]["contents"] == "read" + assert all("baseline" not in step.get("run", "").lower() for step in qualify_steps) + assert not any(job.get("uses") == "./.github/workflows/perf-smoke-test.yaml" for job in workflow["jobs"].values()) assert seeder["concurrency"]["group"] == "${{ inputs.concurrency_group || 'perf-smoke-seed' }}" seed_step = next( step for step in seeder["jobs"]["seed"]["steps"] if step.get("name") == "Seed baselines from commit history" diff --git a/tools/perf_smoke_test/test/test_seed_ancestry.py b/tools/perf_smoke_test/test/test_seed_ancestry.py index 369b35c25086..448a174b5f04 100644 --- a/tools/perf_smoke_test/test/test_seed_ancestry.py +++ b/tools/perf_smoke_test/test/test_seed_ancestry.py @@ -146,6 +146,15 @@ def test_prepare_jit_cache_opens_task_cache(tmp_path: Path) -> None: assert cache_dir.stat().st_mode & 0o777 == 0o777 +def test_cache_run_name_isolates_stability_allocations(monkeypatch) -> None: + """Parallel allocations must not share writable JIT and Kit cache trees.""" + monkeypatch.setenv("GITHUB_RUN_ID", "123") + monkeypatch.setenv("GITHUB_RUN_ATTEMPT", "2") + monkeypatch.setenv("PERF_SMOKE_RUN_LABEL", "runner-allocation-4") + + assert seed_baselines._cache_run_name() == "run-123-attempt-2-runner-allocation-4" + + def test_only_failing_camera_backends_use_container_local_jit_cache() -> None: """Only camera backends that failed on the host mount bypass cache reuse.""" camera_task = "Isaac-Reorient-Cube-Shadow-Camera-Benchmark-Direct" From 28a0170b28f688a36f38b47091ee57eeebbb496c Mon Sep 17 00:00:00 2001 From: Horde Date: Mon, 3 Aug 2026 16:57:57 +0000 Subject: [PATCH 2/2] Scope stability qualification to the Newton buckets The 2026-07-28 run collected clean evidence for the four PhysX-only buckets and none for the five that load Newton, which all crashed on `from newton.solvers import SolverNotifyFlags`. Re-running the full matrix would spend half the pool time re-measuring buckets that already worked. Restrict both the sampling and the qualification to the Newton-touching backends, cutting the run from 135 to 75 samples. The seeder already accepted a backend allowlist; runner_stability now takes the same list so the expected scope matches what was collected, instead of reporting the PhysX buckets as missing evidence and failing closed. An unknown backend key is rejected rather than silently dropped, since a typo would otherwise narrow the scope and yield a weaker verdict that still reads as qualified. --- .../perf-smoke-runner-stability.yaml | 13 +++++ tools/perf_smoke_test/runner_stability.py | 32 +++++++++++- .../test/test_runner_stability.py | 52 +++++++++++++++++++ 3 files changed, 96 insertions(+), 1 deletion(-) diff --git a/.github/workflows/perf-smoke-runner-stability.yaml b/.github/workflows/perf-smoke-runner-stability.yaml index d4ebe3d61690..c5b695a57189 100644 --- a/.github/workflows/perf-smoke-runner-stability.yaml +++ b/.github/workflows/perf-smoke-runner-stability.yaml @@ -8,6 +8,10 @@ # the commit out to five independent allocations so per-runner and run-to-run FPS # spread can be measured separately. Qualification only reports a verdict; it # never writes baselines. +# +# Scope is the Newton-touching buckets. The PhysX-only buckets already produced +# clean evidence on 2026-07-28; these five crashed on a Newton import and are the +# ones still lacking a verdict. Halving the matrix also halves the pool time. name: Performance Smoke - L40S Runner Stability @@ -29,6 +33,11 @@ concurrency: group: perf-smoke-runner-stability-${{ github.ref }} cancel-in-progress: false +env: + # Every backend_key whose benchmark loads Newton. Covers five task/backend + # buckets, since `newton` applies to both Cartpole and Velocity-Flat-G1. + STABILITY_BACKENDS: "newton,newton_rtx_renderer,newton_newton_renderer,physx_newton_renderer" + jobs: wait_for_quiet_pool: name: Wait for initial gate and quiet pool @@ -102,6 +111,9 @@ jobs: commit_count: "1" samples_per_commit: "3" tasks: "__ALL_TASKS__" + # Literal because `with:` cannot read the `env` context. Kept in sync with + # STABILITY_BACKENDS by test_staging_workflow_qualifies_only_newton_buckets. + backends: "newton,newton_rtx_renderer,newton_newton_renderer,physx_newton_renderer" target_branch: perf-smoke/develop-staging strict_ancestry: true dry_run: true @@ -156,6 +168,7 @@ jobs: done python3 tools/perf_smoke_test/runner_stability.py \ "${RECORD_ARGS[@]}" \ + --backends "${STABILITY_BACKENDS}" \ --gpu_model l40s \ --expected_target_branch perf-smoke/develop-staging \ --expected_commit "${GITHUB_SHA}" \ diff --git a/tools/perf_smoke_test/runner_stability.py b/tools/perf_smoke_test/runner_stability.py index 71904b6eeda9..cc9a1075ab3a 100644 --- a/tools/perf_smoke_test/runner_stability.py +++ b/tools/perf_smoke_test/runner_stability.py @@ -124,6 +124,30 @@ def _noise_floor_for_task(task: TaskConfig, gpu_model: str) -> float: return 0.0 +def select_backends(tasks: list[TaskConfig], backends: str) -> list[TaskConfig]: + """Narrow the qualification scope to an explicit backend allowlist. + + Args: + tasks: Every task/backend bucket configured for the gate. + backends: Comma-separated ``backend_key`` allowlist; empty keeps all. + + Returns: + The buckets whose backend is in the allowlist. + + Raises: + ValueError: If a requested backend matches no configured bucket, which + would otherwise silently shrink the scope and weaken the verdict. + """ + requested = {backend.strip() for backend in backends.split(",") if backend.strip()} + if not requested: + return tasks + configured = {task.backend_key for task in tasks} + unknown = sorted(requested - configured) + if unknown: + raise ValueError(f"Unknown backend_key(s) {unknown}; configured backends are {sorted(configured)}") + return [task for task in tasks if task.backend_key in requested] + + def configured_scope( tasks: list[TaskConfig], gpu_model: str, @@ -589,6 +613,11 @@ def _parse_args() -> argparse.Namespace: parser = argparse.ArgumentParser(description=__doc__) parser.add_argument("--records", required=True, action="append", type=Path) parser.add_argument("--tasks_config", type=Path, default=Path(__file__).with_name("tasks.json")) + parser.add_argument( + "--backends", + default="", + help="Comma-separated backend_key allowlist to qualify (empty = every configured backend).", + ) parser.add_argument("--gpu_model", default="l40s") parser.add_argument("--expected_target_branch") parser.add_argument("--expected_commit") @@ -606,7 +635,8 @@ def main() -> int: """Run runner-pool stability qualification.""" args = _parse_args() records = _load_records(args.records) - expected, noise_floors = configured_scope(load_tasks(args.tasks_config), args.gpu_model) + tasks = select_backends(load_tasks(args.tasks_config), args.backends) + expected, noise_floors = configured_scope(tasks, args.gpu_model) report, markdown = build_report( records, expected_buckets=expected, diff --git a/tools/perf_smoke_test/test/test_runner_stability.py b/tools/perf_smoke_test/test/test_runner_stability.py index 90d5f96c32f2..78d33dfe50c2 100644 --- a/tools/perf_smoke_test/test/test_runner_stability.py +++ b/tools/perf_smoke_test/test/test_runner_stability.py @@ -312,6 +312,58 @@ def test_report_states_scope_and_decision() -> None: assert "runner-1" in markdown +def test_backend_allowlist_narrows_the_qualified_scope() -> None: + """Only the requested backends are qualified, so unrelated buckets are not demanded.""" + tasks = runner_stability.load_tasks(_GATE_DIR / "tasks.json") + + selected = runner_stability.select_backends(tasks, "newton,physx_newton_renderer") + + assert {task.backend_key for task in selected} == {"newton", "physx_newton_renderer"} + assert len(selected) < len(tasks) + + +def test_empty_backend_allowlist_keeps_every_bucket() -> None: + """An empty allowlist must not silently narrow the scope.""" + tasks = runner_stability.load_tasks(_GATE_DIR / "tasks.json") + + assert runner_stability.select_backends(tasks, "") == tasks + + +def test_unknown_backend_is_rejected_rather_than_silently_dropped() -> None: + """A typo would otherwise shrink the scope and produce a weaker verdict unnoticed.""" + tasks = runner_stability.load_tasks(_GATE_DIR / "tasks.json") + + with pytest.raises(ValueError, match="Unknown backend_key"): + runner_stability.select_backends(tasks, "newton,nwton") + + +def test_staging_workflow_qualifies_only_newton_buckets() -> None: + """The sampled backends and the qualified backends must be the same set.""" + repo_root = Path(__file__).resolve().parents[3] + workflow = yaml.safe_load( + (repo_root / ".github/workflows/perf-smoke-runner-stability.yaml").read_text(encoding="utf-8") + ) + + declared = workflow["env"]["STABILITY_BACKENDS"] + sampled = workflow["jobs"]["stability_sample"]["with"]["backends"] + assert sampled == declared, "the seeder's backend list drifted from STABILITY_BACKENDS" + + report_step = next( + step for step in workflow["jobs"]["qualify"]["steps"] if step.get("name") == "Build qualification report" + ) + assert '--backends "${STABILITY_BACKENDS}"' in report_step["run"] + + # Exactly the buckets that crashed on 2026-07-28, and no PhysX-only bucket. + tasks = runner_stability.select_backends(runner_stability.load_tasks(_GATE_DIR / "tasks.json"), declared) + assert {(task.task_id, task.backend_key) for task in tasks} == { + ("Isaac-Cartpole-Direct", "newton"), + ("Isaac-Velocity-Flat-G1", "newton"), + ("Isaac-Reorient-Cube-Shadow-Camera-Benchmark-Direct", "newton_rtx_renderer"), + ("Isaac-Reorient-Cube-Shadow-Camera-Benchmark-Direct", "newton_newton_renderer"), + ("Isaac-Reorient-Cube-Shadow-Camera-Benchmark-Direct", "physx_newton_renderer"), + } + + def test_staging_workflow_fans_out_complete_independent_evidence() -> None: """One staging merge automatically gathers and qualifies five allocations.""" repo_root = Path(__file__).resolve().parents[3]