Skip to content

fix(agentx): bump AIPerf to v1.0.5 and propagate MI355X failures - #2572

Merged
cquil11 merged 1 commit into
mainfrom
agent/bump-aiperf-v1.0.5
Aug 12, 2026
Merged

fix(agentx): bump AIPerf to v1.0.5 and propagate MI355X failures#2572
cquil11 merged 1 commit into
mainfrom
agent/bump-aiperf-v1.0.5

Conversation

@cquil11

@cquil11 cquil11 commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • bump utils/aiperf from agentx-v1.0.4 to agentx-v1.0.5
  • propagate the single-node MI355X benchmark exit code after Slurm cleanup

Behavior

AIPerf now allows a three-minute sparse tail in one-hour AgentX profiles while still failing when neither TTFT nor inter-token latency reaches 95% of the profile. This fixes healthy c1 profiles rejected at 97.2% coverage.

The MI355X launcher previously followed a failing container srun with a successful scancel, masking the benchmark failure from GitHub Actions. It now preserves the srun result, performs cleanup, and exits with that result.

Evidence: TP2 c1, TP4 c1, AIPerf #41.

Validation

  • AIPerf: 103 focused tests, Ruff/format, and all pre-commit hooks passed
  • InferenceX: bash -n runners/launch_mi355x-amds.sh and git diff --check passed
  • no inference-server, workload, routing, or performance configuration changes

Signed-off-by: Cam Quilici <cjquilici@gmail.com>

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM — straightforward fix that preserves the srun exit code across cleanup.

Extended reasoning...

Overview

This PR touches only runners/launch_mi355x-amds.sh (CI runner script for single-node MI355X benchmarks) and bumps the utils/aiperf submodule pointer to agentx-v1.0.5. The shell change captures $? immediately after the benchmark srun invocation into benchmark_rc, then exits with that value after the existing scancel and gpucore cleanup steps run. Previously the script's final exit status was implicitly that of scancel, which silently masked real benchmark failures from GitHub Actions (as shown by the linked TP2/TP4 c1 job evidence).

Security risks

None. This is CI/cluster orchestration shell code with no user input, auth, or credential handling involved. The submodule bump only changes a benchmark client's latency-tail tolerance logic in a separate repo, not anything in this repo's trust boundary.

Level of scrutiny

Low. This is a narrowly scoped, mechanical bug fix in a non-production CI launcher script — exactly the kind of self-contained change (capture exit code, propagate it) that doesn't need design-level input. The multi-node branch of the same script already uses a similar pattern, so this brings the single-node branch in line.

Other factors

bash -n and git diff --check were reported as passing, and the change is easy to read end-to-end in the diff. The bug-hunting pass found no issues, and the one candidate quality nit raised (reimplementing a trap-based pattern from a sibling launcher) was ruled out as not a real bug. No outstanding review comments exist on this PR.

@cquil11
cquil11 merged commit fe433b1 into main Aug 12, 2026
7 checks passed
@cquil11
cquil11 deleted the agent/bump-aiperf-v1.0.5 branch August 12, 2026 12:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

1 participant