Repository navigation
Conversation
…ance Extend inference-benchmark-design with an owner-local reference on clock and cache controls, within-run attribution, work-count signatures and timing traps. Candidate guidance; empirical validation not run. Agent-assisted-by: Codex CLI / gpt-6.1-sol [effort=xhigh] (author) Signed-off-by: BeatSeat <137616995+BeatSeat@users.noreply.github.com>
Agent-assisted-by: Codex CLI / gpt-6.1-sol [effort=xhigh] (author) Signed-off-by: BeatSeat <137616995+BeatSeat@users.noreply.github.com>
Skill evaluationPLANNED: not executed; there are no task outcomes yet.
Author verification steps (nonpaid; not yet run)
python tools/build_skill_index.py --source-revision 3671a98ad340fb19e6595d1c8d818c7e9ca2577a --checkThe declared source pin matches this head's canonical library bytes. The command has not run: retain its exit code and full drift/success output. It checks the generated projection, not claim truth or skill efficacy.
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:
Selection/task metadata (not a relevance verdict): {"adapter":"audit","basis":"trusted_path_mapping","matched_paths":["src/inferrsi/knowledge/data/skill_library/methods/inference-benchmark-design/SKILL.md","src/inferrsi/knowledge/data/skill_library/methods/inference-benchmark-design/metadata.json","src/inferrsi/knowledge/data/skill_library/methods/inference-benchmark-design/references/gpu-measurement-validity.md"],"tasks":[{"evidence_scope":"http_fixed_workload","id":"audit-threshold","request":"Audit the fixed input report using its contract. Compute the median of per-trial request means, including every request in each trial, and the percentage reduction relative to the baseline aggregate.","split":"holdout"}]}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"}Projection check selection (not an executed result): {"declared_in":"src/inferrsi/knowledge/data/repo_packs/inferrsi/knowledge/_provenance.json","source_revision":"3671a98ad340fb19e6595d1c8d818c7e9ca2577a","status":"ready"}Provider/settings: Uncovered / unsupported paths (60)
|
|
Thanks for the careful caveats and the two-step generation. I reviewed this against current
I also checked: the new reference is 175 lines; its SHA-256 I am not approving this head until the matrix placement is fixed. |
Dong1017
added
area:knowledge
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR review verdict — DO NOT PASS
Reviewed base cd8fcb452bd2c7969a7a6e2544aa1cb5f8ad5332 and head 8c0b894a2dc9b8572700ca3f80b119e75f2377db (draft).
Acting reviewer: cursor[bot] via standing InferRSI steward delegation. Binding formal review to current head.
Blockers before Approve / ready-for-review:
- Matrix placement. The new
docs/reference-matrix.mdrow for GPU measurement validity sits under## Task-specific benchmark selection (issue #60)(around L444–L451). The matching searchspace note is under the issue #60 heading as well. This is general GPU measurement methodology, not an #60 task-selection source. Move the matrix row and searchspace paragraph to a dedicated short section (same pattern as## Faster CosyVoice evaluation source context), then regenerate coverage/records sodocs/skill-coverage.mddoes not label it as an #60 source. - Rebase onto current main. Head diverges: ahead 2 / behind 1 vs
cd8fcb45(#453). Rebase, and if the source-revision commit changes, regenerate projections and update the declarationhead_sha. - Governing issue still Proposed. #455 has no recorded owner acceptance yet. Steward decision will be posted on #455; keep this draft until acceptance + the matrix/rebase fixes land.
Checked / not blocking alone: docs-only slice; candidate / empirical not_run preserved; Triton pin and do_bench flush description match prior notes; installed-count 923→924 is coherent with a single new reference. I did not re-download NVIDIA/PyTorch docs or re-run the installed-wheel check.
Re-request review after the blockers are addressed on a new head.
|
I re-checked after the steward decision on #455: the documentation-only slice is now Accepted (with conditions), and the goal-acceptance owner is Dong1017. That external blocker is cleared. This note is for the same head
The “require an explanation before adoption” wording should stay clearly candidate guidance, as accepted on #455. I am not approving this head. cursor[bot] also left CHANGES_REQUESTED / DO NOT PASS on the same head for these items. Please request a re-review once they are addressed (likely on a new head). |
Governing Issue and delivery slice
Refs #455
GPU E2E timings can move with clock, thermal, power and sharing conditions; profiler replay can introduce different execution conditions. The reference gives a procedure for retaining paired timings, checking conditional within-run attribution and Amdahl predictions, and connecting gains to work counters and a predicted profiler signature. It covers timing/cache/baseline/JIT pitfalls and keeps the four supplied numerical observations explicitly unverified.
The existing
inference-benchmark-designowner already defines comparable workloads, measurement integrity and evidence interpretation. Under the knowledge-base ownership contract, this is an owner-local reference extending that procedure; it does not need a separate skill identity or registry entry. Candidate state, empiricalnot_runand efficacynot_evaluatedremain explicit.Knowledge authorship declaration
Changed artifacts and evidence
methods/inference-benchmark-design/references/gpu-measurement-validity.md(175 lines); SKILL.md adds six lines_provenance.json, coverage view;tests/check_installed.pycount 924Canonical skill paths above are relative to
src/inferrsi/knowledge/data/skill_library/. The registry, unrelated skill records and generated navigation README remain unchanged.Checks and dependencies
All Python commands used the requester-specified interpreter and checkout
PYTHONPATH. Final Git-dependent checks used a temporary Git 2.49.1 build, with the repository tools unchanged and native--no-lazy-fetchsupport.tools/build_skill_index.py --source-revision 3671a98ad340fb19e6595d1c8d818c7e9ca2577a, then--check: passed, 80 records current (regenerated after the two-commit split; generated churn limited to the target record,_provenance.jsonand the coverage view).tools/build_skill_coverage.py, then--check: passed. Generated churn is limited to the target record, shared provenance and coverage view.python -m pytest tests/test_skill_catalog.py tests/test_skill_index.py tests/test_skill_library.py tests/test_skill_library_readers.py tests/test_skill_coverage_view.py tests/test_knowledge_*.py --basetemp <temporary-test-dir> -rs: 479 passed, 5 skipped, 112 subtests passed. The final head retains the same canonical/generated bytes; the later one-line installed-count adjustment was exercised directly below.python -m ruff check src tests: passed, including the installed-count adjustment.tests/check_installed.pyoutside the checkout with isolated imports and checkout-resource comparison: passed, all 924 resource files byte-identical. The installed normalized reader also retrieved the new reference byte-for-byte.read/resource, citation/line-count checks and retained public-document digest comparisons: passed.tools/knowledge_declaration_paths.py --base origin/main --head HEAD: passed; 7 protected paths, 0 policy paths, 3 generated paths (overlapping classifications), 2 other paths.assess()fromtools/check_knowledge_model_policy.pyagainst this PR body, final head, base-relative paths and trusted base policy: passed, declaration compliance only. No GitHub status was published; the remote CLI/status workflow was not run against an uncreated PR.git diff --check, clean-worktree check, DCO sign-off andAgent-assisted-bytrailer inspection on both commits: passed.Resolved validation failures: The initial generator invocation used system Git 2.43, which lacks
--no-lazy-fetch; final generation/checks used native Git 2.49.1. A broad-suite run found the paper source needed the normalizedidfield; it was corrected and the complete selected suite rerun. The first installed check exposed the expected resource-count increment; the fixture now expects 924 and the final-head check passes. Initial build-dependency downloads timed out; a bounded pinned-dependency retry succeeded.Skipped / not run: One browser test requires unavailable Playwright; two fresh-venv tests remain opt-in; two NTFS-junction cases are Windows-only. The separate noneditable wheel check above ran successfully. Full ready/nightly CI, independent review, GPU profiling, model quality, L1–L4 campaign execution and efficacy evaluation were not run.
Limits
3671a98a, the pinnedsource_revision), then records are regenerated from that commit and committed (8c0b894a). The source revision is an ancestor of the head, so no separate bundle is needed.Remaining work and Issue closure