Repository navigation
Add an optional AISBench service benchmark adapter - #326
jiangkuaixue123 wants to merge 1 commit into
Conversation
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
left a comment
There was a problem hiding this comment.
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.
|
Review at head
|
Dong1017
left a comment
There was a problem hiding this comment.
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] |
There was a problem hiding this comment.
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.
|
Coverage review at head
Reviewed commit range: |
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.pystarts each child with a fresh working directory, verifies one timestamped report, and publishes output-token throughput toINFERRSI_RESULT_PATH. Performance trials must report positive throughput, complete request coverage and zero failures.--validation-inconclusive-exit-codelets 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
PASS,FAILandINCONCLUSIVE. 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.