Repository navigation
OperatorX: add Ascend queued GEMM reproducer / OperatorX：新增昇腾排队计时 GEMM 复现工具 - #4440
Conversation
新增用于 Ascend GEMM 测量的独立复现工具，包含排队计时、数值校验、可恢复日志和 FP8 L0A 容量预检查。
将 FP8 预检查绑定到已验证的逻辑设备，并拒绝不一致的硬件身份报告。
将示例输出目录放在 /tmp，避免在共享工作目录中创建基准产物。
|
Claude finished @Oseltamivir's task in 8m 59s —— View job Review
No blocking issues found. Two 🟡 warnings posted inline:
Perf changelog: not applicable, and I did not run What I verified rather than assumed, since most of this cannot run in CI:
I could not execute The fail-closed posture throughout is the right call for a measurement tool — journal fingerprinting refusing resume across changed plan/source/hardware, the two-idle-observation + ECC/PCIe gate, foreign-process detection invalidating an otherwise- |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| if ( | ||
| not 1 <= args.warmups <= 10000 | ||
| or not 10 <= args.samples <= 10000 | ||
| or not 0 <= args.warmup_ms <= 10000 | ||
| ): | ||
| parser.error("Invalid warmup/sample configuration") |
There was a problem hiding this comment.
🟡 WARNING: The CLI advertises a wide timing range, but time_op hard-rejects everything except exactly (10, 5, 25).
operatorx_queued_timing.py:12 raises ValueError("Queued protocol requires 10 samples, 5 warmups, 25 ms ramp") for any other combination, so --samples 20 (or --warmups 4, or --warmup-ms 50) passes argparse here, passes run_case's own 10 <= samples <= 10000 check, and only fails deep inside _run_native.
Why it matters: by then the controller has already run the preflight subprocess, taken the board lock, imported torch/torch_npu, set the device and generated inputs (the bundled BF16 example is 49152×7168×6144). The failure surfaces as a per-case status: "error", which sets failed = True and breaks the loop — so the operator burns a full device bring-up and a journal entry to learn that an argparse-accepted value is unsupported.
Fix: reject it at parse time, where the message is immediate and unambiguous.
| if ( | |
| not 1 <= args.warmups <= 10000 | |
| or not 10 <= args.samples <= 10000 | |
| or not 0 <= args.warmup_ms <= 10000 | |
| ): | |
| parser.error("Invalid warmup/sample configuration") | |
| if (args.samples, args.warmups, args.warmup_ms) != (10, 5, 25): | |
| parser.error( | |
| "Queued protocol requires --samples 10 --warmups 5 --warmup-ms 25" | |
| ) |
| c++ -std=c++17 -O2 -shared -fPIC \ | ||
| "$ASCEND_SCRIPTS/operatorx_fp8_l0_guard.cpp" -ldl \ | ||
| -o "$ASCEND_RESULTS/liboperatorx_fp8_l0_guard.so" |
There was a problem hiding this comment.
🟡 WARNING: The documented build command omits -pthread, but the guard uses std::mutex/std::lock_guard (operatorx_fp8_l0_guard.cpp:29,110,124,137,155,171) and the test harness that validates this exact source does pass it (operatorx/tests/test_ascend_l0_guard.py:102).
Why it matters: on toolchains whose glibc still keeps the pthread symbols in libpthread (pre-2.34 — e.g. CentOS/EulerOS 7/8 images, which are common for CANN hosts), linking a shared object that uses std::mutex without -pthread resolves the locking primitives to the libc no-op stubs instead of erroring. The guard then runs with non-functional mutexes while aclrtLaunchKernelWithHostArgs mutates g_keys/g_last from runtime threads, so the decision JSON that operatorx_fp8_l0_preflight.py:177 reads back can be torn or stale — a silently wrong L0A verdict rather than a build failure. Building the shipped artifact differently from the tested one is the part worth closing.
Fix: match the harness invocation.
| c++ -std=c++17 -O2 -shared -fPIC \ | |
| "$ASCEND_SCRIPTS/operatorx_fp8_l0_guard.cpp" -ldl \ | |
| -o "$ASCEND_RESULTS/liboperatorx_fp8_l0_guard.so" | |
| c++ -std=c++17 -O2 -shared -fPIC -pthread \ | |
| "$ASCEND_SCRIPTS/operatorx_fp8_l0_guard.cpp" -ldl \ | |
| -o "$ASCEND_RESULTS/liboperatorx_fp8_l0_guard.so" |
| args.expect_pci, | ||
| **keywords, | ||
| ) | ||
| foreign = [row for row in parsed["processes"] if row["pid"] not in own_pids] |
There was a problem hiding this comment.
🔴 Operators who run the controller inside a container (a separate PID namespace) have every run aborted on the first case: the controller's own worker process is flagged as foreign. own_pids is just {os.getpid()} (execute_case), but when npu-smi's process table has no container-PID column, operatorx_npu_health._processes records each row's host PID; a containerized controller's os.getpid() differs from that host PID, so the live monitor marks the case "contaminated". Fix: match the controller's process against npu-smi rows by both host and container PIDs (or a stable non-PID identity), so the worker is never treated as foreign in any PID namespace.
Why this was flagged
operatorx/scripts/ascend/operatorx_batch_campaign.py:316 sets own_pids={os.getpid()} and passes it to process_snapshot (line 137) and the monitor (operatorx_campaign.py:324). operatorx_npu_health._processes (lines 104-117) sets a row's pid to the host PID (cells[1]) when the table lacks the 'Process id in container' column, and to the container PID only when that column exists. When the controller runs in a container PID namespace and npu-smi reports host PIDs, the controller's own device-holding process appears with a PID not equal to os.getpid(). process_snapshot then collects it into foreign and raises "Foreign NPU process detected", and the per-case monitor appends it to state['issues'], which _invalidate turns into status 'contaminated' (lines 428-433), stopping the run. On bare metal with matching namespaces this does not fire. Because the whole harness is new there is no base behavior; the consequence is that the first real case aborts rather than measuring. The identity check at process_snapshot validates board/chip/PCI, not process ownership, so it does not prevent the misclassification.
Verification: execute_case sets own_pids = {os.getpid()} (operatorx_batch_campaign.py:316). In operatorx_npu_health._processes, when that column is missing, pid stays int(cells[1]) (line 107). The monitor records the controller's own row as foreign_processes, and lines 426-433 downgrade the result to "contaminated".
| if ( | ||
| not 1 <= args.warmups <= 10000 | ||
| or not 10 <= args.samples <= 10000 | ||
| or not 0 <= args.warmup_ms <= 10000 | ||
| ): |
There was a problem hiding this comment.
🟡 (optional) Operators can pass --samples 20, --warmups 10 or another value the controller accepts, and then every case fails after full input generation. The controller and both workers accept samples in [10,10000], warmups in [1,10000] and warmup_ms in [0,10000]. time_op raises unless the values are exactly (10, 5, 25) (operatorx_queued_timing.py:12-13). Fix: reject anything except samples=10, warmups=5, warmup_ms=25 in the controller's argparse validation, before the preflight and torch import. Alternatively, drop these flags and record the fixed protocol in config, while keeping the config[key] fields that validate_worker_result compares.
Why this was flagged
An operator runs operatorx_batch_campaign.py run --samples 20 (or --warmups 10, or --warmup-ms 50). The values pass the range check at operatorx_batch_campaign.py:627-631. They also pass the worker range checks at operatorx_ascend_worker.py:734-749 and operatorx_fp8_block_worker.py:640-647. The controller then runs the npu-smi preflight, imports torch_npu and creates the journal with that config in its fingerprint. The first case allocates inputs and passes the before-timing FP64 check. Then queued_timing.time_op raises ValueError("Queued protocol requires 10 samples, 5 warmups, 25 ms ramp") at operatorx_queued_timing.py:12-13. The case is recorded as error and the campaign stops with exit 1. The run directory's journal fingerprint now carries the unusable config, so a corrected rerun needs a new run directory. This harness is new and has no base behaviour; no safeguard catches the mismatch early, because the CLI and worker ranges are wider than what the timing helper accepts.
Verification: Trigger: an operator passes any timing value other than the fixed 10/5/25, for example --samples 20. The range checks at operatorx_batch_campaign.py:627-631 are effectively dead validation, because queued_timing.time_op raises unless the values are (10, 5, 25) (operatorx_queued_timing.py:12-13). The failure is deterministic and repeats for every case in the campaign.
| self.assertEqual(t["queued_wall_us_per_call"], t["queued_batch_wall_us"] / 10) | ||
| self.assertNotIn("synchronized_wall_samples_us", t) |
There was a problem hiding this comment.
🟡 nit (optional): Two assertions here break the AGENTS.md test-quality rules. Line 74 recomputes the implementation's own batch_wall_us / 10 formula, which is tautological. Line 75 asserts that an unrelated key is absent, which pins structure rather than behaviour. Fix: delete both, or assert queued_wall_us_per_call against a value worked out by hand from the mocked perf_counter_ns ticks.
Why this was flagged
AGENTS.md 'Test quality' forbids tautological expectations that compute the expected value with the same formula as the implementation. operatorx_queued_timing.py sets queued_wall_us_per_call = batch_wall_us / 10, and test line 74 checks exactly that expression against the returned dict. That assertion cannot fail on any regression in how wall time is measured. Line 75's assertNotIn("synchronized_wall_samples_us", t) checks for the absence of a key the code never produces, so it covers no behaviour. Mocked clock ticks of 10,000,000 ns give a deterministic batch wall time, so an independently derived expected value is available.
Verification: At operatorx/scripts/ascend/operatorx_queued_timing.py:65 the implementation sets "queued_wall_us_per_call": batch_wall_us / 10. Test line 74 asserts t["queued_wall_us_per_call"] == t["queued_batch_wall_us"] / 10, which is the same formula. AGENTS.md:94 forbids this. The assertion holds whatever batch_wall_us is, including a wrong measurement window. Nothing breaks at runtime.
| return code, details | ||
|
|
||
| def test_original_failure_capture_exceeds_declared_capacity(self): | ||
| self.assertEqual(len(self.original), 160) |
There was a problem hiding this comment.
🟡 nit (optional): self.assertEqual(len(self.original), 160) checks the size of the checked-in fixture file, not guard behavior. AGENTS.md forbids tests that pin checked-in data or test fixtures. Fix: drop that assertion and keep the decoder assertions on code and l0a_bytes.
Why this was flagged
test_original_failure_capture_exceeds_declared_capacity at operatorx/tests/test_ascend_l0_guard.py:192-196 first asserts the byte length of operatorx/tests/fixtures/ascend_fp8_tiling.bin. That length is a property of the checked-in fixture, not of the guard. AGENTS.md "Test quality" items 4 and 7 forbid pinning checked-in data and testing fixtures. Recapturing the fixture with a different trailing DFX length would fail this test even though the guard's behavior is unchanged. test_exact_declared_struct_and_extra_dfx_bytes_agree already covers the length variants through the real decoder.
Verification: nit. At operatorx/tests/test_ascend_l0_guard.py:193, self.assertEqual(len(self.original), 160) checks the byte length of the checked-in fixture operatorx/tests/fixtures/ascend_fp8_tiling.bin before any guard code runs, so it tests the fixture, not the guard. AGENTS.md "Test quality" forbids "Tests of fixtures" (item 7). Nothing breaks at runtime.
| EXECUTION_MODE = "persistent_worker_process" | ||
|
|
||
|
|
||
| def worker_script(args): |
There was a problem hiding this comment.
🟡 nit (optional): Most new harness modules use untyped function signatures; the existing operatorx code and AGENTS.md ("Python uses typed signatures") call for typed ones. sweep:^\s*def \w+\(.*\):$ Examples: def worker_script(args): here, and def time_op(npu, fn, cache, config): at operatorx_queued_timing.py:10. Fix: annotate parameters and return types, as prepare.py and fp8_plan.py already do.
Why this was flagged
AGENTS.md line 29 says "Follow the nearest existing pattern. Python uses typed signatures". Existing operatorx modules annotate every function, for example operatorx/core/op_registry.py:11 def register(spec: OpSpec) -> None: and operatorx/runners/common/timing.py:26. Several new files in this PR declare functions with no annotations: operatorx_batch_campaign.py, operatorx_campaign.py, operatorx_ascend_worker.py, operatorx_fp8_block_worker.py, operatorx_npu_health.py, operatorx_queued_timing.py and operatorx_fp8_l0_preflight.py. This is a convention mismatch, not a runtime failure.
Verification: nit. The new harness modules are merged with unannotated function signatures, against the repository's stated style rule. AGENTS.md:29 says: "Follow the nearest existing pattern. Python uses typed signatures and strict Pydantic schemas." One site is operatorx/scripts/ascend/operatorx_batch_campaign.py:34 def worker_script(args):. Nothing breaks at runtime.
Adds the standalone Ascend GEMM harness used for the 950PR measurements: queued ten-sample timing, numerical checks, resumable journals, and a prelaunch FP8 L0A guard. It accepts OperatorX testlists without changing the NVIDIA/AMD runners or CI pools.
The guard is specific to the tested CANN 9.1 FP8 kernel ABI and skips unknown tilings. The NPU write-based launch shield and eager execution do not claim CUDA sleep/graph parity. Usage and protocol.
Validated the packaged CLI on Ascend950PR_9579 / CANN 9.1.0: three unquantized cases and one FP8 case passed correctness and resume checks. The BF16 peak reached 376.93 TFLOPS; the 68 KiB FP8 tile was skipped before dispatch.
AI model disclosure
The exact deployed model/version identifiers could not be verified in this runtime. Codex prepared the implementation, tests, documentation and PR. Earlier agent-assisted measurement and guard code is included; those contributing agents' exact model/version identifiers could not be verified either.
PR checklist and change type
Type of Change
Checklist
python -m infx.matrix.changed --base origin/mainlists affected config keys, I have appended one new entry covering them to the physical end ofinferencex-e2e/perf-changelog.yaml; if it lists none, I have added none. I have not edited historical entriesinferencex-e2e/perf-changelog.yamlentry, it carries exactly one primary sweep label (a maintainer applies it on fork PRs):full-sweep-fail-fast(recommended),full-sweep-enabled, ornon-canary-full-sweep-enabled. Optional modifiersall-evals,evals-only, andagentx-fastrequire a primary label; the last two block reuse while applied.OWNER/MEMBER/COLLABORATOR) has commented/use <run_id>(or the legacy/reuse-sweep-run) on this PR. Do this only once there is a final full sweep that is all green with evals passing, since after this comment the primary sweep label will no longer automatically kick off new sweeps. Remove and re-add the primary sweep label to force a new sweep.Changelog, sweep-label and reuse conditions are not applicable: this standalone tool changes no existing E2E configuration or scheduled benchmark point.
中文
新增用于 950PR 测量的独立 Ascend GEMM 复现工具，包含十次采样的排队计时、数值校验、可恢复日志和执行前 FP8 L0A 容量检查。支持读取 OperatorX 测试列表，不修改 NVIDIA/AMD 运行器或 CI 资源池。
容量检查仅支持已验证的 CANN 9.1 FP8 内核 ABI；无法识别的分块会被跳过。NPU 写操作构成的启动屏障与即时执行模式，不声称与 CUDA sleep 或图执行完全等价。使用方式和协议见上方链接。
已在 Ascend950PR_9579 / CANN 9.1.0 上验证打包后的命令行：三个非量化用例和一个 FP8 用例通过数值校验及恢复检查，BF16 峰值达到 376.93 TFLOPS；需要 68 KiB L0A 的 FP8 分块在执行前被跳过。
AI 披露：当前运行环境无法核实具体模型及版本。Codex 完成实现、测试、文档和 PR；其中也包含此前由其他 agent 辅助编写的测量与容量检查代码，这些 agent 的具体模型及版本同样无法核实。
改动类型为新功能和文档更新。已完成本地测试；由于不改变已有端到端配置或计划中的基准点，changelog、sweep 标签及复用要求不适用。