Repository navigation
feat: add shared engineering evaluation and dual-NPU MoE task - #338
Bounty-hunter wants to merge 25 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 (84)
|
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
|
Review at head
|
Keep profiling optional, distinguish existing task tools from pending real-Agent integration, and preserve scoped evidence and unresolved review findings. Agent-assisted-by: Codex (documentation and scope review) Signed-off-by: dengyunyang <584797741@qq.com>
Bounty-hunter
changed the title
|
Incremental review This delta: the 12 files under Still open:
Static doc review of the delta only; I did not run the NPU tooling. |
Remove the CPU RMSNorm package and its dedicated historical probe. Keep Softmax regression coverage, preserve source-pinned experiment records, and retain the NPU task unchanged. Agent-assisted-by: Codex (implementation, test migration, and documentation) Signed-off-by: dengyunyang <584797741@qq.com>
|
Incremental review This delta looks consistent:
Still open:
Static review of the delta only; I did not run the tests or the NPU tooling. |
Remove issue-specific plans and large historical dumps from the current tree, retain durable task and artifact handoff guidance, and drop obsolete path attributes and issue-specific utility wording. Historical records remain available at their original revision. Agent-assisted-by: Codex (documentation cleanup and independent review) Signed-off-by: dengyunyang <584797741@qq.com>
|
Incremental review This delta:
Still open:
Static review of the delta only; I did not run the tests or the NPU tooling. |
Signed-off-by: dengyunyang <584797741@qq.com>
Signed-off-by: dengyunyang <584797741@qq.com>
Signed-off-by: dengyunyang <584797741@qq.com>
Bounty-hunter
changed the title
|
Incremental review This delta: adds the Findings / questions:
Static review of the delta only; I did not run the tests, the NPU tooling or the MoE evaluator. |
Signed-off-by: dengyunyang <584797741@qq.com>
Dong1017
left a comment
Dong1017
left a comment
There was a problem hiding this comment.
Scoped follow-up at 2eed487a to Amy's CI question: the evidence-inventory count is now repaired, but both current-head full checks still fail in detached-worker cleanup. The new diagnostics identify a process-discovery failure before any worker is signalled; the P2 below needs correction. The previously reported malformed-field classification and RMSNorm timeout findings remain separate and are not cleared by this review.
CI evidence and scope
- Inspected head:
2eed487a1edcac30a3a32b5a504bc7b013e9616b; incremental commit only changes inventory totals and assertion diagnostics. - Python 3.10 log and Python 3.11 log: each reports 1 failed, 3,682 passed, 19 skipped. The sole failure is
test_deadline_cleans_detached_workers_but_preserves_unrelated_process, withverified: false,signalled_pids: [], and permission denied reading a same-user/proc/.../environ. - Read the affected cleanup code, test and hosted logs. No local execution, hardware/model run, or new spending.
| fields = (entry / "stat").read_text().rsplit(")", 1)[1].split() | ||
| if fields[0] == "Z": | ||
| continue | ||
| if marker in (entry / "environ").read_bytes().split(b"\0"): |
There was a problem hiding this comment.
P2: An unreadable same-user process prevents cleanup of the invocation's workers. _members() scans every same-UID process, and a permission error reading any environ propagates before it returns its accumulated matches. cleanup() therefore reaches its outer exception handler without signalling even the marked workers it could inspect. Both current-head Linux jobs reproduce this after the launcher times out: verified=false, signalled_pids=[], with /proc/.../environ permission denied. This can leave detached workers running and makes the standard Linux qualification fail. Scope discovery to an inspectable invocation-owned process set, and ensure a partial scan still attempts cleanup of positively identified workers. Preserve an inconclusive result when absence cannot be established; do not merely ignore unreadable entries and report verified success. Add coverage for an unreadable unrelated same-user process.
There was a problem hiding this comment.
Scoped follow-up at 8f3aebd2e1359d11d8bcdd034f5dacedf9a1f12d: the original hosted-CI failure is cleared, but this P2 is only partially fixed.
Both Python 3.10 and 3.11 now report 3,688 passed, 19 skipped. Skipping strictly older processes is sound for the fresh marker. However, an unreadable same-tick or newer process still prevents cleanup of workers already positively identified earlier in the scan.
I reproduced this with a synthetic procfs and mocked os.kill (no real signals): driver start tick 100, readable exact-marker worker PID 990001 at tick 101, then an unrelated same-UID process whose environ raises PermissionError. With the unrelated process at tick 100 or 101, cleanup() returns verified=false, signalled_pids=[], and the mock receives zero calls. At tick 99, it skips that unrelated environment and attempts to signal the known worker.
_members() still throws away its accumulated matches when the later read raises; cleanup() catches the exception outside the signalling loop. The new unreadable-process test has no owned worker, so it does not cover this consequence.
Please preserve discovered matches alongside scan errors, attempt cleanup of the PID/start-time-verified owned workers, and retain an inconclusive result for unreadable entries. Add the mixed owned-worker/unreadable-process case. This is the remaining part of the original finding, separate from Amy's malformed-field and whole-pilot timeout findings.
|
Incremental review This delta bumps the evidence inventory totals in Findings:
Static review of the delta plus the CI logs; I did not run the tests or the MoE evaluator. |
Signed-off-by: dengyunyang <584797741@qq.com>
|
Incremental review This delta addresses item 1 of that review in a reasonable way:
Remaining items:
Static review of the delta plus surrounding source only; I did not run the tests or any NPU tooling. |
Structural question: two task libraries, two entry styles, two identity freezesReviewed at
The MoE addition is documented as a deliberate separation, not an oversight: 1. Two parallel content libraries with the same file roles
Both hold the same kind of thing: a frozen task manifest, a baseline, a correctness oracle, a license and provenance. The stated distinguishing axis is single-operation versus distributed layer. That reads like a property a task declares, not a reason for a parallel tree — and Two costs are visible in this diff rather than hypothetical. The practical consequence: an agent has to know which library a task lives in before it can choose the invocation. For a task library that is the thing you most want to avoid. 2. Each entry point verifies something different, and the differences are not visible from the imports
Its imports are These two are not interchangeable: What is worth checking is that reusing a shared execution helper conveys no upper-layer guarantee. Per entry point, as read at this head:
That table is the useful artifact, and it is also the point: filling it per module produces the wrong answer, which is how I got the original claim wrong. It has to be filled per entry point. Nothing here says any of these entry points needs source-snapshot adjudication — 3. Three low-cost suggestionsNone of these requires restructuring what is already here:
4. What this comment does not claim
|
This PR gives engineering optimization candidates a shared task definition, independent correctness checks, reproducible measurements and retained source identities. Callers supply actual implementations; existing InferRSI execution paths run them and retain the evidence used for subsequent optimization decisions.
Draft. This update also publishes the two-NPU MoE forward task and the six-session experiment previously kept in local experiment storage. Historical device results and qualification of the newly packaged local runner are reported separately.
Delivered behavior
check / measure / profile / grade / plan / compareoperations reuse Workbench; incorrect implementations cannot provide usable timing scores.python -m tools.moe_benchmarkruns a supplied candidate inside a caller-managed two-NPU allocation using the existing external executor. No machine address, physical card assignment, SSH configuration or Agent provider is embedded. It is a separate distributed scenario, not the single-operation TaskPackage adapter.How to use and review
Agent launch, knowledge selection, candidate search and repetition policy remain caller-owned. Profiling is optional. This PR does not turn an arbitrary external project into a validated task or add automatic model/session orchestration.
Validation
src,testsandtools.8f3aebd2e1359d11d8bcdd034f5dacedf9a1f12d: Python 3.10 and 3.11 each 3,688 passed, 19 skipped, 284 subtests passed. Both clean installed-wheel checks and strict Linux artifact controls passed. The real detached-worker cleanup regression ran on Linux; scoped PID/start-time and same-tick/unreadable-process controls are included.test_all_actual_revision_bytes_retained_before_checksalso fails on the unchanged runtime/test baseline. The passing Linux suite does not imply full Windows qualification.Historical NPU RMSNorm controls retain their original tested revision and 800 timing samples: correct implementations pass, wrong/replayed outputs are rejected, same-code controls are not selected as an optimization, and an existing fused reference reduces synchronized call latency under the recorded workloads. This was a reference-control experiment, not Agent-discovered code.
The MoE comparison used GPT-6.1 Sol, GPT-5.6 Sol and GPT-6 Astra with knowledge on/off, fresh sequential sessions, eight exploratory rounds each and an independent final. There are 46 correct exploratory candidates, two retained candidate failures and six passing final checks. Costs include failed attempts and cached input; Astra ON experienced substantial queueing. Each condition has one trajectory. Results do not establish a general model/knowledge advantage, and across-session speedup ratios are not direct candidate-to-candidate timings.
Those historical MoE runs used the existing
external.execute_trialat frozen InferRSI source6ed93bb41f7db9dafb420dcb6f8705f1864bc45c, with experiment-local orchestration. They did not execute this PR's single-operation TaskPackage/SkillEval adapter, nor the newly packaged wrapper. No new NPU run is represented by the publication checks.Remaining work
Refs #252
Refs #336
Refs #337