Skip to content

feat(inference): lm-eval-harness based accuracy evaluation + vllm suite integration - #268

Open
atnair-amd wants to merge 9 commits into
dev/dtnifrom
atnair/accuracy-harness
Open

feat(inference): lm-eval-harness based accuracy evaluation + vllm suite integration #268
atnair-amd wants to merge 9 commits into
dev/dtnifrom
atnair/accuracy-harness

Conversation

@atnair-amd

@atnair-amd atnair-amd commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

Jira: AIMVT-269

Summary

Adds an lm-eval-harness based accuracy evaluation capability, opt-in per suite via config.json.

  • Config schema (accuracy_config.py): AccuracyTask/AccuracyConfig (pydantic, extra=forbid) hold task selection only — id/task/num_fewshot/metadata/include_path/num_concurrent/apply_chat_template/gen_kwargs. Rejects duplicate task ids.
  • Result parsing (lm_eval_parsing.py): project() auto-discovers real numeric metrics from an lm-eval results.json payload and flattens them into a single dict, keyed task.metric__filter.
  • Command construction + orchestration (lm_eval_job.py): builds the lm_eval CLI invocation (local-completions vs local-chat-completions, model_args, gen_kwargs, metadata) and runs it via orch.exec_on_head against an already-live inference server, then locates and parses the resulting results*.json.
  • Lifecycle stage (inference_suite_lifecycle.py::test_accuracy_eval): shared, opt-in pytest stage — skips cleanly if a prior stage failed or no accuracy tasks are configured, records per-task results via lifecycle.record, and gates each task independently against threshold.json's accuracy.<task_id> block when enforce_thresholds is set (record-only otherwise).
  • Suite wiring: accuracy: AccuracyConfig field added to both VariantConfig (vllm) and InferenceXAtomVariantConfig (inferencex_atom), defaulting to empty so existing configs are unaffected. test_accuracy_eval imported into both suites' test modules and inserted into collection order right after the perf-metric stage.

Not included in this PR: the long_isl_osl_stress custom lm-eval task (its gibberish/repetition scoring logic needs its own design pass — deferred).

Test plan

  • ./.cvs_venv/bin/python -m pytest cvs/lib/inference/unittests/ --cluster_file /dev/null --config_file /dev/null — 249 passed (2 pre-existing, unrelated failures in test_inferencing_config_loader.py persist with or without this branch)
  • Verified vllm/inferencex_atom suite modules and conftest.py collection ordering import cleanly with test_accuracy_eval inserted

@atnair-amd atnair-amd changed the title feat(inference): add AccuracyTask/AccuracyConfig schema for accuracy harness feat(inference): lm-eval-harness based accuracy evaluation for vllm/inferencex_atom suites Jul 22, 2026
@atnair-amd atnair-amd self-assigned this Jul 24, 2026
@atnair-amd
atnair-amd marked this pull request as ready for review July 24, 2026 19:33
…harness

First unit of the lm-eval-harness accuracy evaluation system: the
config.json-side task selection schema, split from threshold/gating
values which will be joined in at runtime by a later unit.
Second unit of the accuracy harness: pure JSON -> {scalar: float}
projector for lm-eval-harness results.json payloads. Walks every
numeric metric with no per-task registry, so group tasks (e.g.
RULER's per-seq-length metrics) fall out of the same walk.
…tom suites

Adds the accuracy field to both suites' VariantConfig classes (defaulting to
an empty AccuracyConfig so existing configs load unchanged), imports the
shared test_accuracy_eval lifecycle stage into each suite's test module, and
places it in collection order right after the perf-metric stage.
- exclude the top-level "accuracy" threshold key from sweep-cell coverage
  checks (was tripping the extra-key/typo detector); delegate
  vllm_config_loader's inline check to the shared validator
- gate accuracy threshold evaluation on enforce_thresholds, matching
  test_metric's existing record-only convention
- route chat-template tasks to /v1/chat/completions instead of always
  using /v1/completions
- wrap exec_on_head tuple-unpack and JSON parse failures in
  run_accuracy_tasks as RuntimeError instead of letting raw
  ValueError/JSONDecodeError propagate
- select the newest results*.json by mtime instead of an arbitrary find
  ordering, guarding against stale results from a prior run
…e tasks

build_lm_eval_cmd switched the model backend/endpoint to local-chat-completions
for tasks with apply_chat_template=True but never passed lm-eval's own
--apply_chat_template flag, so lm-eval sent plain-string prompts and the
chat-completions client asserted on every request. Verified on a live 2N
DeepSeek-R1 accuracy run where this aborted mmlu_pro and blocked all
subsequent tasks in the same test_accuracy_eval invocation.
@atnair-amd
atnair-amd force-pushed the atnair/accuracy-harness branch from 788cf53 to fccabeb Compare July 27, 2026 21:31
Scope this feature to vllm only until it's been validated against ATOM
hardware. Removes the accuracy field from InferenceXAtomVariantConfig,
the test_accuracy_eval import/collection-order entry in the ATOM suite,
and the corresponding ATOM-specific unit tests. The shared
lm_eval_job.py/lm_eval_parsing.py/accuracy_config.py machinery and
test_accuracy_eval itself are untouched -- vllm's wiring is unaffected.
Each accuracy task now gets its own pytest node (test_accuracy_eval[<id>])
instead of one collapsed row covering every configured task, matching the
test_metric/test_gpu_metric per-metric row convention. Task nodes are gated
independently: a run failure or threshold violation in one task no longer
sets the shared lifecycle.failed flag, so sibling tasks still execute rather
than being skipped by a prior task's outcome. pytest_generate_tests
parametrizes accuracy_task from config.json's accuracy.tasks, including the
empty-list case (auto-skips a single node, same UX as before).
@atnair-amd atnair-amd changed the title feat(inference): lm-eval-harness based accuracy evaluation for vllm/inferencex_atom suites feat(inference): lm-eval-harness based accuracy evaluation + vllm suite integration Jul 27, 2026
@solaiys

solaiys commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Review (head 66e57af0, base dev/dtni)

Regular review — blocking issues only.

Blocking issues

No blocking issues found.

What I checked:

  • Command construction / injection (lm_eval_job.py) — every lm_eval arg is shlex.quoted, and model_args/gen_kwargs are joined into a single quoted token; the find/cat follow-ups quote task_out_dir and result_path. No shell-injection or interpolation gaps. exec_on_head single-result unpacking is guarded ((x,) = out.values() wrapped with a clear RuntimeError), and a missing results*.json is treated as a run failure.
  • Result parsing (lm_eval_parsing.py) — pure function; skips alias/non-real numbers (bool/NaN excluded), flattens numeric metrics only. No crash paths.
  • Config schema (accuracy_config.py) — Pydantic v2 _Forbid; mutable defaults ({}/[]) are per-instance-copied by Pydantic (not a shared-mutable-state bug), and duplicate task ids are rejected. The new accuracy field defaults to empty via default_factory, so existing configs are unaffected.
  • Threshold refactor (vllm_config_loader.py → shared validate_thresholds_cover_sweep) — no regression: import warnings was removed from vllm_config_loader.py and it has no remaining warnings. use, while the shared function imports/uses warnings. New gated_gpu_metrics param defaults to None (backward-compatible for other callers); vllm passes GATED_GPU_METRICS to preserve prior gpu gating. accuracy is excluded from sweep-cell coverage via NON_SWEEP_THRESHOLD_KEYS.
  • Lifecycle stage (test_accuracy_eval) — skips on prior failure / missing task, records per-task independently (doesn't set the shared lifecycle.failed), and only gates via evaluate_all when enforce_thresholds is set (record-only otherwise). vllm collection order/parametrization (pytest_generate_tests empty-list auto-skip, conftest rank) is consistent.
  • Concurrency — no new threads/async; per-task output dirs are isolated (output_dir/<task.id>), so no shared-file races in normal (non-xdist) collection.

Non-blocking note (not a merge blocker, but worth confirming)

The PR description states the accuracy field was added to InferenceXAtomVariantConfig and test_accuracy_eval was "imported into both suites' test modules." The diff only wires the vllm suite — the inferencex_atom suite (inferencex_atom_single.py / _shared.py / conftest.py) has no accuracy references, and no inferencex_atom config/suite file is changed. This isn't a bug in the code that's present (nothing breaks; accuracy is simply unavailable for inferencex_atom), but the described scope for inferencex_atom isn't delivered here — worth confirming whether that was intended for this PR or a follow-up.

Reviewed 1 PR: 0 blocking issues — clean (with one scope-accuracy note).

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.

2 participants