fix(agentx): bump AIPerf to v1.0.5 and propagate MI355X failures - #2572
Conversation
Signed-off-by: Cam Quilici <cjquilici@gmail.com>
There was a problem hiding this comment.
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.
Summary
utils/aiperffromagentx-v1.0.4toagentx-v1.0.5Behavior
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
srunwith a successfulscancel, masking the benchmark failure from GitHub Actions. It now preserves thesrunresult, performs cleanup, and exits with that result.Evidence: TP2 c1, TP4 c1, AIPerf #41.
Validation
bash -n runners/launch_mi355x-amds.shandgit diff --checkpassed