Skip to content

[None][docs] drop deleted accuracy_eval/gen_server_config from disagg benchmark README - #18969

Open
imitater-dou wants to merge 1 commit into
NVIDIA:mainfrom
imitater-dou:docs/disagg-bench-drop-deleted-accuracy-eval-20260909
Open

[None][docs] drop deleted accuracy_eval/gen_server_config from disagg benchmark README#18969
imitater-dou wants to merge 1 commit into
NVIDIA:mainfrom
imitater-dou:docs/disagg-bench-drop-deleted-accuracy-eval-20260909

Conversation

@imitater-dou

@imitater-dou imitater-dou commented Sep 9, 2026

Copy link
Copy Markdown

What

In examples/disaggregated/slurm/benchmark/README.md:

  • Drop deleted Supporting scripts entries accuracy_eval.sh and gen_server_config.py.
  • Note that submit.py invokes lm_eval directly when accuracy evaluation is enabled.
  • Align the Accuracy Evaluation YAML sample with config.yaml / submit.py (per-task nested schema).
  • Mention run_benchmark_aiperf.sh alongside the other benchmark runners.

Why

How I checked

Verified on main @ 96a25c4:

  • accuracy_eval.sh / gen_server_config.py return 404 under examples/disaggregated/ and examples/disaggregated/slurm/benchmark/.
  • Directory listing includes run_benchmark_aiperf.sh, which the README omitted.
  • Live config.yaml uses nested accuracy.tasks.<name> with model / model_args_extra / extra_kwargs.
  • submit.py writes per-task logs under accuracy_eval_<task> and invokes lm_eval directly.

Dev Engineer Review

  • Updated examples/disaggregated/slurm/benchmark/README.md to match the current benchmark workflow.
  • Removed references to deleted accuracy_eval.sh and gen_server_config.py scripts.
  • Documented direct lm_eval execution through submit.py.
  • Updated the accuracy evaluation example to use nested per-task settings and gsm8k.
  • Added run_benchmark_aiperf.sh to the benchmark runner documentation.
  • No source-code or API changes are included.

QA Engineer Review

No test changes.

Per-File QA Perspective

  • examples/disaggregated/slurm/benchmark/README.md: Documentation-only change. Verify that the documented lm_eval command, nested accuracy configuration, per-task result directories, and run_benchmark_aiperf.sh usage match the current scripts and produce the expected benchmark outputs.

… benchmark README

Signed-off-by: ADou <ikun3.1415927@gmail.com>
@coderabbitai

coderabbitai Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 69065e78-4947-4440-a1fd-51f74eda463b

📥 Commits

Reviewing files that changed from the base of the PR and between 96a25c4 and 896918e.

📒 Files selected for processing (1)
  • examples/disaggregated/slurm/benchmark/README.md

Included review availability: Your plan provides up to 12 included reviews per hour; 7 remain after this review.


Walkthrough

The benchmark README now documents run_benchmark_aiperf.sh, direct lm_eval usage through submit.py, per-task accuracy settings, and task-specific result directories.

Changes

Benchmark documentation

Layer / File(s) Summary
Accuracy evaluation documentation
examples/disaggregated/slurm/benchmark/README.md
The README lists the updated benchmark scripts, documents nested per-task accuracy settings with gsm8k, describes task-specific result directories, and preserves the YAML code fence.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~3 minutes

Merge Risk: ⚪ Minimal · up to 89691

Benchmark documentation now reflects the current accuracy-evaluation workflow and runner scripts, with no remaining merge-readiness risk identified.

Suggested reviewers: bowenfu

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies a documentation update that removes references to deleted benchmark scripts. It uses the required ticket and type format and is concise.
Description check ✅ Passed The description explains what changed, why the changes were needed, and how the author verified the documentation against the current repository state. The testing details are appropriate for a docume…
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant