Skip to content

Add an optional AISBench service benchmark adapter - #326

Draft
jiangkuaixue123 wants to merge 1 commit into
InferMatrixLab:mainfrom
jiangkuaixue123:codex/aisbench-external-evaluator
Draft

jiangkuaixue123 wants to merge 1 commit into
InferMatrixLab:mainfrom
jiangkuaixue123:codex/aisbench-external-evaluator

Conversation

@jiangkuaixue123

Copy link
Copy Markdown
Collaborator

Governing issue and delivery slice

Refs #289

This draft proposes the docs-and-example option for maintainer review. AISBench is installed separately. A campaign invokes it through the existing baseline, candidate and validation commands. The example targets AISBench v3.1-20261013-master (5a63d936e40dd04addfcffb2401f84cd0e53bb46).

Changed artifacts and evidence

  • examples/aisbench_adapter.py starts each child with a fresh working directory, verifies one timestamped report, and publishes output-token throughput to INFERRSI_RESULT_PATH. Performance trials must report positive throughput, complete request coverage and zero failures.
  • For accuracy, the example verifies the frozen reference hash, model/dataset identity and complete, unique prediction coverage before comparing the reported score with the floor. A measured regression returns 1. Missing or malformed evidence returns 2.
  • --validation-inconclusive-exit-code lets the campaign opt into the latter quality outcome while retaining the subprocess return code. The option applies only to validation. Campaigns that omit the option keep their existing behavior. The guide demonstrates the distinction between AISBench’s performance mode and InferRSI's own run mode.

Checks and dependencies

  • Linux/aarch64, Python 3.11.6 and Git 2.47.3. Full CPU suite passed with 3,428 tests and 292 subtests passing, and 13 optional/platform skips. Draft smoke (8 cases) and Ruff passed.
  • Ran the pinned AISBench CLI against an isolated synthetic HTTP service. A successful performance report mapped to the throughput metric. One injected request failure produced a native report with nonzero throughput and a zero child exit status; the adapter rejected the measurement.
  • Ran three concurrent InferRSI campaigns using the same synthetic Git checkout. Complete, regressed and insufficient accuracy evidence produced validation exits 0, 1 and 2 and overall verdicts PASS, FAIL and INCONCLUSIVE. Source integrity passed in each case.

Remaining work and issue closure

The choice between a docs/example integration and a packaged extra remains with the maintainers of #289. This submission requests design and code review. It makes no engine performance or model quality claim.

Run AISBench through the existing external commands and read reports from an isolated trial directory. Verify prediction coverage and the frozen reference before using an accuracy result. Let campaigns designate a validation exit code for insufficient evidence while preserving the original process status.

Agent-assisted-by: Codex (integration and command contract)
Agent-assisted-by: Codex / inferrsi_isolation (adapter and regression tests)
Agent-assisted-by: Codex / official_reference (documentation and integration verification)
Signed-off-by: jiangkuaixue123 <jiangxiaozhou111@163.com>
@Dong1017 Dong1017 added the area:harness Harness module: profile, A/B loop, quality gates, evidence reports label Nov 3, 2026 — with ChatGPT Codex Connector

@Dong1017 Dong1017 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The optional AISBench adapter is coherent within its documented scope at ad3c6ca2; adoption and packaging remain an open maintainer decision in the governing proposal.

  • The adapter binds a fresh report directory, rejects failed/incomplete performance requests, and checks frozen reference bytes plus unique prediction coverage before interpreting accuracy. The supported report paths/keys and missing-prediction behavior were cross-checked against the pinned AISBench source.
  • The opt-in exit-code setting affects validation only: the selected nonzero code becomes INCONCLUSIVE, a measured regression remains FAIL, and subprocess status/code are retained. The default behavior is preserved when the option is absent.
  • Current hosted software checks pass; the inspected 3.11 job reports 3,428 passed, 13 skipped and 292 passing subtests. Synthetic software controls do not establish engine performance or model quality.
  • jiangkuaixue123 retains the contribution; existing maintainers retain the docs/example versus optional-extra decision, with benchmark coordination already directed to Bounty-hunter. This COMMENT does not accept the proposal or grant merge permission.
Evidence and scope
  • Head: ad3c6ca27e6e658b08b6d9940d5a5ef8682384be; API base: 2baebf6ebb815ebf07d21c41e3b3754858c0798e; assessed current main: ebdbf5af00c94f228b504a3f4d813aad0b63840c.
  • Read all nine changed paths and traced CLI → RunSpec validation → evaluation verdict, plus adapter report/coverage validation and regression definitions.
  • Ready run, including inspected 3.11 job. The hosted checkout is a synthetic main/head merge; no standalone-tree equivalence is claimed.
  • Main's hardware-only 103-path update touches none of this PR's nine files, CLI/evaluation interfaces or test definitions. Earlier Ready evidence remains applicable to that unchanged adapter scope; the current-main combination was not run here.
  • AISBench pin: 5a63d936e40dd04addfcffb2401f84cd0e53bb46. Inspected the performance writer, metric calculator, accuracy prediction writer and evaluator padding logic. The guide correctly leaves installed version/config contents, endpoint identity and campaign acceptance with the owner.
  • Static source and existing hosted evidence only: no local suite, AISBench installation/service run, model/device work or workflow action by this maintenance session. The author's synthetic HTTP/campaign runs remain attributed reports.
  • Contribution ownership is publicly acknowledged; scope acceptance remains pending. No new implementation authority or owner is assigned here.

Copy link
Copy Markdown
Collaborator

Review at head ad3c6ca2 (draft): not approving yet.

  1. Adoption decision pending: whether InferRSI adopts and packages an optional AISBench adapter is still an open maintainer decision in [RFC] Integrate AISBench as an optional benchmark tool #289. Please link that decision before this is reviewed for merge.
  2. Not run on CI: the PR is a draft, so the Ready checks have not run at this head. Please mark it ready (after [RFC] Integrate AISBench as an optional benchmark tool #289 is decided) and attach the test results.
  3. Question: is the adapter exercised against a real AISBench installation anywhere (version, command, result), or only with fixtures? Please record which, so the scope claim in the PR body can be checked.

@Dong1017 Dong1017 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Scoped incremental static review of PR #326 at head ad3c6ca2 (base 2baebf6e). One new P2 evidence-completeness gap is attached inline. The performance reader verifies that reported requests are all successful, but does not bind their total to the campaign's predeclared request count. This can admit a valid-looking throughput datapoint for a shortened workload, compromising A/B comparability. The prior reviews' CLI layering, quality evidence, and unresolved #289 adoption decision are not reopened. Checked the complete changed-file patch, adapter/test contract, external trial metric consumer, governing RFC, existing reviews and current workflow triggers. This is source-only feedback; no AISBench, test, service or device execution, no approval or adoption decision.

if (
any(type(x) is not int for x in counts)
or counts[0] <= 0
or counts != [counts[0], counts[0], 0]

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

P2 — reported success is not the frozen request coverage. This only establishes Total == Success > 0 and Failed == 0 within the report. In perf mode the adapter has no expected-request input, so a run configured for 100 requests that actually emits a successful 1-request report plus a positive throughput passes this check and contributes a normal A/B metric. The guide asks owners to freeze sample count, but that count is never compared here; baseline/candidate can therefore be measured over different effective workloads without an explicit evidence warning. Please bind each trial to the predeclared expected request count (e.g. an optional frozen --expected-requests or verified native run/config manifest), require exact coverage, and return evidence-insufficient when it differs. A CPU-only fixture with Total=Success=1, Failed=0, expected 100 should not yield a performance metric. This is a static contract finding, not a claim that AISBench itself has miscounted real requests.

Copy link
Copy Markdown
Collaborator

Coverage review at head ad3c6ca27e6e658b08b6d9940d5a5ef8682384be (base 2baebf6ebb815ebf07d21c41e3b3754858c0798e; draft). Re-checked against my earlier short-SHA note; no new commits since then. Not approving yet.

  1. Adoption decision still pending (#289): whether InferRSI adopts and packages an optional AISBench adapter remains an open maintainer decision. Please link that decision before this is reviewed for merge.
  2. P2 evidence-completeness gap (Dong1017, 2026-11-05) (examples/aisbench_adapter.py ~L92–100, discussion): perf_rate only checks Total == Success > 0 and Failed == 0 within the report. There is no binding to a predeclared expected request count, so a shortened successful workload can still yield a normal A/B throughput metric. Expected: bind each trial to a frozen expected request count (CLI flag or verified native run/config), require exact coverage, and treat a mismatch as evidence-insufficient.
  3. Merge state: mergeable_state=dirty against current main; please rebase once [RFC] Integrate AISBench as an optional benchmark tool #289 is decided and re-run applicable checks.
  4. Still a draft; Ready checks for this head have not been exercised as a ready PR. Please also record whether the adapter was exercised against a real AISBench installation (version, command, result) or only fixtures.

Reviewed commit range: 2baebf6ebb815ebf07d21c41e3b3754858c0798e … ad3c6ca27e6e658b08b6d9940d5a5ef8682384be.

@Dong1017 Dong1017 added the kind:task Bounded delivery task with a delegated slice label Nov 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:harness Harness module: profile, A/B loop, quality gates, evidence reports kind:task Bounded delivery task with a delegated slice

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants