Repository navigation
feat: add NPU RMSNorm pilot and shared benchmark evaluation - #338
Bounty-hunter wants to merge 17 commits into
Conversation
Add frozen task packages, independent CPU oracles and profiling, with bindings to the existing Workbench and offline SkillEval verifier. Include research decisions, reproducible integration checks and explicit qualification limits. Agent-assisted-by: Codex (implementation and documentation) Signed-off-by: dengyunyang <584797741@qq.com>
Dong1017
added
the
area:workbench
ChatGPT Codex Connector
Dong1017
left a comment
Dong1017
left a comment
There was a problem hiding this comment.
Scoped draft review of the new CPU benchmark measurement path against #252 and the campaign no-output-replay rule in #30. P2 benchmark-validity concern is attached inline: the precheck and timed samples reuse the same input object/content, so a memoizing candidate can appear faster without performing the operation on each call. Please address or explicitly constrain this before treating timing as optimization evidence. Static source review only; no tests, model runs, hardware measurements, or acceptance decision made here. The initial task subset remains subject to the recorded owner decision.
| if profiler: | ||
| profiler.enable() | ||
| started = time.perf_counter_ns() | ||
| output = compute(x, eps=oracle["eps"]) |
There was a problem hiding this comment.
P2 — measured calls can become cached-output replay. The precheck at line 94 already invokes compute(x), and this loop passes the same x object and values to every warmup/sample. A candidate with a process-local id(x)/content → output memo can pass every value/input-preservation assertion while most timed calls only return the stored output; the reported suite_latency_ms then measures replay, contrary to the campaign's no-final-output-caching boundary (#30). Suggest preparing a frozen, paired varying input sequence outside the timing boundary (same sequence for baseline and candidate) and validating each invocation; add a stateful memoizing-candidate negative regression. Public development workloads still cannot establish hostile-code resistance or held-out generalization.
There was a problem hiding this comment.
Addressed in ce84506e7106c4fc48fafdfab55d2ea2f9dffd50 with numpy-axis1-v2. Inputs/oracles are prepared before candidate invocation and retained for the complete action; each timed input has distinct bytes from precheck/warmup/other samples. Both arms and all consumers share the same deterministic per-call sequence. Each sample is bound to an input hash. The invariant zero case remains correctness-only, so the new score covers five varying workloads; old task/grader identities and evidence are preserved.
Regression controls confirm exact id/content memo implementations have no cache hits in the sequence, while shape/layout output replay is rejected without a metric. The fix does not claim hostile-code resistance. Forty focused tests and related CLI/SkillEval tests passed; actual Workbench and offline verifier execution retained 300 samples with matching planned and actual inputs. Statistical resolution remains unqualified.
Fix and validation · RSI consumer boundaries. Please re-review; this response does not mark the thread resolved or claim maintainer acceptance.
There was a problem hiding this comment.
The specific cached-output replay finding is addressed in ac03ae23; this is a scoped source confirmation, not approval of the whole draft.
- Inputs and independent oracles are prepared before candidate calls. All input objects stay alive, and later inputs must have distinct bytes; the zero case is correctness-only and five varying cases contribute timing.
- The new regressions require both identity-keyed and content-keyed memoization to miss, and reject stale shape/layout output replay without a usable performance metric. Per-sample input hashes and the frozen task/grader bindings are retained.
- @Bounty-hunter remains the implementer. Full-PR review, v2 timing/noise qualification and the recorded parent scope decision remain open; this confirmation establishes neither reliable performance ordering nor optimizer/skill benefit.
Evidence and limits
Assessed head: ac03ae23ce0bb5864a55c2c97d5f3d8d3b550d39; API base and actual main: 663f3b327998526a13aa31e72ad729025c18983c.
Read the current task contract, NumPy evaluator, worker, consumer/verifier bindings, both task manifests and the regression definitions, including input preparation and memo/replay controls. This is not a completed review of all 42 changed files.
The Python 3.11 hosted job reports Ruff success and 3,584 passed / 13 skipped / 284 subtests. It checked out merge 396688581967ea85d8edf2f7f169925546c5e207 (current head into the main above); the API comparison returned zero file differences from the standalone head. Draft qualification was skipped. I did not rerun tests or independently inspect the author's local 300-sample archive. No model/device execution or scope acceptance occurred.
Agent-assisted-by: Codex (validation fixture and documentation) Signed-off-by: dengyunyang <584797741@qq.com>
Document existing RSI consumers and remaining end-to-end acceptance. Agent-assisted-by: Codex (implementation, review and validation) Signed-off-by: dengyunyang <584797741@qq.com>
Agent-assisted-by: Codex (documentation) Signed-off-by: dengyunyang <584797741@qq.com>
Agent-assisted-by: Codex (review, implementation and documentation) Signed-off-by: dengyunyang <584797741@qq.com>
Agent-assisted-by: Codex (implementation, validation and documentation) Signed-off-by: dengyunyang <584797741@qq.com>
Agent-assisted-by: Codex (review) Signed-off-by: dengyunyang <584797741@qq.com>
Skill evaluationPLANNED: not executed; there are no task outcomes yet.
Author verification steps (nonpaid; not yet run)
Evidence and logs · Retention: 30 days. Plan bindings, budget, frozen exam and advisory detailsAdvisory: obtain missing evidenceRetained evidence availability: planned / unexecuted. No approval or merge authorization is issued.
Model/settings:
Automatic execution / cumulative PR budget: {"reason":"Trusted configuration or pricing missing, invalid or expired","remaining_usd":"unknown","reserved_or_unresolved_usd":"unknown","spent_or_legacy_usd":"unknown","state":"blocked"}Provider/settings: Uncovered / unsupported paths (76)
|
Agent-assisted-by: Codex (validation) Signed-off-by: dengyunyang <584797741@qq.com>
Agent-assisted-by: Codex (validation and documentation) Signed-off-by: dengyunyang <584797741@qq.com>
Dong1017
left a comment
Dong1017
left a comment
There was a problem hiding this comment.
Incremental review at fd0f396c of the formal artifact integration since ac03ae23: one P2 correctness blocker, attached inline. The new built-in-only adapter can resolve a same-named working-directory task instead of the packaged task, and its freeze/revalidation path preserves that substitution.
The earlier cached-output replay finding remains addressed; I am not reopening or duplicating it. I inspected the new adapter, reused task/sandbox contracts, controller/run-trial/local-capture/report changes, regression definitions and CI wiring. Current Ready CI succeeds: the dedicated Linux controls report 49 passed with strict sandbox availability enabled; the 3.11 full check reports 3,631 passed, 18 skipped and 284 subtests. Actual checkout 98156741 has zero file differences from this head. Those tests do not cover cwd task shadowing.
Static source review plus existing hosted logs only; no local candidate/test execution, model or hardware work. Bounty-hunter retains the correction. Correctness-only limits, performance/skill-benefit gaps and #252's outstanding acceptance decision remain separate; this review does not accept the broader goal.
| def task_descriptor(name: str) -> dict: | ||
| if name not in builtin_tasks(): | ||
| raise ValueError("Only built-in shared CPU benchmark tasks are supported") | ||
| package = load_task(name) |
There was a problem hiding this comment.
P2 — resolve formal tasks from the packaged resource, not the working directory. Checking that name occurs in builtin_tasks() does not ensure that load_task(name) loads that built-in package: contract.load_task first prefers any existing Path(name) directory. validate_task(), starter() and verify() repeat the same lookup, so they all agree on a shadow rather than detect it.
For a concrete source-trace case, run from the InferRSI checkout with a local softmax-cpu/ copy of the packaged task and change only its oracle.atol to 1. Generate the descriptor through task_descriptor('softmax-cpu'). The manifest validator permits that positive finite tolerance. prepare() copies this same package, freezes its checkout-relative paths, and load_plan() compares against the same shadow, so the paired path also accepts it. A compute returning np.zeros_like(x) then satisfies all four output checks for every input: true softmax values lie in [0,1], so np.allclose(0, expected, atol=1) accepts them. The canonical packaged files need not change. This is static tracing/arithmetic, not a claim that I executed the candidate.
This makes a built-in-only, independently frozen correctness task depend on the caller's cwd and allows an incorrect artifact to pass under an accidentally or deliberately substituted rubric. It is separate from the documented public-suite/protocol-spoofing limits. Please use an explicit packaged-resource/built-in-only loader throughout the formal adapter and its prepare() dependency collection, while retaining path loading for the trusted Workbench API. Add a regression with a same-named cwd task directory that proves the canonical oracle/hash is used or the shadow is rejected.
There was a problem hiding this comment.
The specific cwd-shadowing finding is addressed at 6308268fc6d1fbf90e9ab97b5da8af79152f39ea (fix introduced in 00c41b24). This clears this finding only.
load_builtin_task()resolves the packaged resource and checks its manifest ID. The formal descriptor, starter and parent verifier now usetask_package(); prepare copies and freezes dependencies through that same canonical loader. Revalidation compares the retained package hash and starter against the canonical descriptor. Trusted Workbench path loading remains available.- The new regressions cover a same-named
softmax-cpu/cwd directory withatol=1, canonical descriptor/starter/oracle selection, prepare/dependency freezing and later revalidation. The real sandbox control rejects the zero-output implementation despite that shadow. The formal CPU path also rejects the new NPU adapter before execution. - I read the exact-head source and dedicated Linux control log: 54 passed, zero skips, with
INFERRSI_REQUIRE_BENCHMARK_SANDBOX=1and NumPy 2.2.6. The 3.11 full check reports 3,649 passed / 20 skipped / 284 subtests. CI checked out merge6f1506bc; its tree has zero file differences from this head. I did not execute code locally.
The separate NPU action/total-timeout finding remains open and is not superseded by this reply. This is not whole-PR approval, device qualification, performance/skill-benefit evidence, or scope acceptance; Bounty-hunter retains the implementation.
Agent-assisted-by: Codex (documentation and scope review) Signed-off-by: dengyunyang <584797741@qq.com>
Agent-assisted-by: Codex (source research and task design) Signed-off-by: dengyunyang <584797741@qq.com>
Bounty-hunter
changed the title
Keep reviewed path loading available to Workbench while resolving formal descriptors, starters, oracle checks and prepare dependencies only from packaged task resources. Reject unsupported device adapters before the NumPy sandbox. Agent-assisted-by: Codex (implementation and tests) Signed-off-by: dengyunyang <584797741@qq.com>
Define the affine device task and qualification flow; retain CPU results as historical software evidence and keep NPU SkillEval isolation separate. Agent-assisted-by: Codex (documentation) Signed-off-by: dengyunyang <584797741@qq.com>
Execute trusted candidates on an allocated NPU with an independent scalar oracle, synchronized eager timing, separate device profiling and Workbench A/A and reference controls. Agent-assisted-by: Codex (implementation and review) Signed-off-by: dengyunyang <584797741@qq.com>
A no-op comparison may fail either its speed gate or correctness. Accept the no-op control only after successful final quality validation, and compare its actual environment with every other result. Agent-assisted-by: Codex (implementation and review) Signed-off-by: dengyunyang <584797741@qq.com>
Match all action, trial and final validation identities against the pre-execution freeze, including incorrect controls stored outside the task package. Agent-assisted-by: Codex (implementation and review) Signed-off-by: dengyunyang <584797741@qq.com>
Dong1017
left a comment
Dong1017
left a comment
There was a problem hiding this comment.
Scoped incremental static review of the new rmsnorm-npu pilot in PR #338 (head 6308268f), against the execution limits recorded in Issue #337. One P2 execution-budget contract gap, attached inline: the 180-second action timeout is applied to standalone check/profile/negative controls but not to the A/A or reference Workbench trials. This can leave an individual trial running until the 8-minute comparison deadline; the script also does not itself enforce the separately stated 20-minute pilot envelope. Please bind the per-action and total deadlines or identify and verify the outer enforcement, with a timeout-control regression. This is a source/contract finding, not a hardware result. I inspected the new NPU adapter/driver, RunSpec, the Workbench runner/external/local executor chain and relevant tests. The earlier canonical-task shadowing review remains separate and is not repeated. No NPU/model/test execution or acceptance decision was performed.
| comparisons = {} | ||
| # Same implementation in both arms is the explicit no-op/A-A control. | ||
| for name, path in (("aa", baseline), ("reference", candidate)): | ||
| spec = task.runspec(path, repo=args.repo, output_dir=output / "results" / name, |
There was a problem hiding this comment.
P2 — the frozen per-action timeout is not applied to paired NPU trials. Issue #337's predeclared pilot says --timeout-seconds 180, two Workbench comparisons capped at 8 minutes each, and a 20-minute total runtime. Here args.timeout_seconds is passed only to run_action() for check/profile/negative controls (lines 96–116); task.runspec() does not carry it. run_experiment() passes its whole-comparison deadline to execute_trial() → execute_command(), which calls process.wait(timeout=deadline-now). Thus a first stalled A/A or reference trial may occupy almost the entire 480-second comparison instead of stopping at 180 seconds; two comparisons plus all standalone controls can also exceed 20 minutes unless an independently enforced outer deadline exists. That changes the frozen resource/lifecycle budget, particularly on a shared NPU allocation. Please cap each Workbench trial at min(comparison_remaining, action_timeout, pilot_remaining) (or implement equivalent tested enforcement), and include a deliberately nonterminating synthetic command in a CPU-only timeout regression. If a scheduler supplies the global bound, record its enforceable receipt and distinguish that from this script's guarantees.
Publish the NPU workflow, all timing samples and scoped keep decision; distinguish the trusted device pilot from formal artifact isolation and Agent efficacy. Agent-assisted-by: Codex (documentation and evidence review) Signed-off-by: dengyunyang <584797741@qq.com>
Bounty-hunter
changed the title
This PR adds an Ascend NPU RMSNorm pilot to InferRSI's shared engineering tasks and validates the existing Workbench path on a real Ascend 910B1. The user explicitly selected NPU for the pilot; the earlier CPU tasks remain historical/software controls.
Draft, current head
a91b913e7a71ba2d8460ea3b8f2b8964de7dafe1. NPU execution and hosted source checks ran on6308268fc6d1fbf90e9ab97b5da8af79152f39ea; subsequent changes only document that evidence. Maintainer acceptance and the original reviewer's re-review remain pending. No merge or issue closure is requested.Read first: 中文实测与决定 · NPU workflow/interface · all 800 samples, identities and controls · four deliverables.
Delivered behavior
rmsnorm-npu/torch-npu-rmsnorm-v1:compute(x, weight, *, eps)on contiguous 2D float32 NPU tensors, nonuniform weights, independent scalar double/math.fsum oracle, input/weight metadata and content checks, no CPU fallback.check/profile/measure/compareoperations dispatch to the NPU worker. Timing covers eager dispatch and device completion synchronization; setup, transfers, oracle and output validation are outside timing. Profiling is separate and requires actual Ascend Hardware kernel events.tools.npu_benchmark_validatefreezes policy and implementation identities, then uses the existing Workbench for A/A and reference AB/BA. All action/trial/final-validation identities and actual device/CANN/runtime records must match. Incorrect outputs never supply usable metrics.Four issues and their roles
Executed NPU evidence
Policy was declared before execution. One scheduled Ascend 910B1, CANN 9.1.0, Python 3.12.13, torch 2.10.0+cpu and torch_npu 2.10.0.post4. Both correct implementations passed 375 assertions each. Six wrong controls were rejected without metrics. An additional stateful control passed all 25 initial assertions and was then rejected at the first varying warmup. Actual profile records show six composed compute kernels per selected input versus one RmsNorm kernel for the fused reference.
[1,4096][64,4096]Each comparison used four AB/BA pairs; all 800 timing samples are retained. The predeclared checks passed, including both final quality validations, fixed actual environment and frozen identities. Main qualification and supplemental replay checks exited 0 and released the allocation. The fused reference is an existing vendor operator, not a newly invented kernel. These are synchronized eager-call measurements including dispatch/synchronization, not pure kernel times, statistical significance or service gains.
Software validation and review
Source-head ready/review CI: Python 3.10 and 3.11 each 3649 passed / 20 skipped / 284 subtests passed; both installed-package checks passed. Formal CPU artifact controls: 54 passed / 0 skipped, including the new actual sandbox cwd-shadow test. The checked-out merge
6f1506bcb366c10836991701f8beaf7ac89e9a11and tested head have the same tree5c22811e28f1c8c0182852916f43804c18b35edf. Hosted CI has no NPU; device evidence is the separate run above.Local NPU contract regressions ended at 14 passed / 1 device test skipped. Initial new fixture encoding/output-directory failures were corrected and rerun. Source review also found and corrected mutable-shape validation, final-quality gating for A/A, and missing comparison to initially frozen control identities. Early API probes with nonzero exits remain documented and are not timing samples.
The original reviewer has not yet accepted the canonical-loader fix; CHANGES_REQUESTED remains a review state. All real NPU execution uses reviewed trusted source. Formal SkillEval NPU isolation, real old/new-skill Agent efficacy, multi-round search policy, provider generation, skill_eval_improve and model/service qualification remain unestablished. Public development tasks are not holdout. The existing general Workbench remains usable for user targets outside the built-in task library.
Refs #252
Refs #334
Refs #335
Refs #336
Refs #337
Base:
663f3b327998526a13aa31e72ad729025c18983c. DCO-signed commits retain Agent-assisted-by attribution. Knowledge authorship declaration: not applicable; protected knowledge/policy paths are unchanged.Earlier CPU evidence
The previous CPU delivery and evidence remain preserved at their original revisions. They are historical/software controls, not this NPU pilot's qualification. Current scope and review-fix status are stated above.