Skip to content

Commit 46d38c7

Browse files
GopalakrishnanNGopalakrishnan Nallasamy
andauthored
Benchmark CLI follow-ups: clarify --backend help and include evaluator config in eval cache key (microsoft#2552)
## Describe your changes Follow-ups to microsoft#2420 addressing the remaining (non-blocking) review comments. **`olive benchmark` `--backend` help text** — Reworded to clarify it applies to lm-eval evaluation (not just "ONNX model evaluation"): `ort`/`ortgenai` require ONNX input, `ortgenai` additionally requires GenAI-packaged model assets (e.g. `genai_config.json`), and `auto` infers the backend from the model type. **Evaluation cache key now includes the evaluator config** — `Engine._evaluate_model` previously keyed the evaluation cache on `model_id` + accelerator only, so re-running the same model with a different `--backend` (i.e. `model_class`), or different lm-eval `tasks`/`limit`/`batch_size`/metrics, would silently reuse a stale cached result. The key now folds in `hash_dict(evaluator_config.to_json())`, so any change to the evaluation parameters produces a distinct cache entry. The key is internal to `_evaluate_model` (only `_load_evaluation`/`_cache_evaluation` consume it); footprints still record the raw `model_id`. **Tests** — Added `model_class`-absent backward-compat assertions to `test_benchmark_command_hfmodel`/`test_benchmark_command_onnxmodel`, and a new `test_evaluate_model_cache_key_includes_evaluator_config` engine test (same config → cache hit; different config → cache miss). ## Checklist before requesting a review - [x] Add unit tests for this change. - [x] Make sure all tests can pass. - [ ] Update documents if necessary. - [x] Lint and apply fixes to your code by running `lintrunner -a` - [x] Is this a user-facing change? If yes, give a description of this change to be included in the release notes. - `olive benchmark` no longer reuses a cached evaluation result when re-running the same model with a different `--backend` or different lm-eval parameters; the `--backend` help text was also clarified. ## (Optional) Issue link Follow-up to microsoft#2420. Co-authored-by: Gopalakrishnan Nallasamy <gopalakrishnan.nallasamy@microsoft.com>
1 parent 1e76021 commit 46d38c7

4 files changed

Lines changed: 66 additions & 3 deletions

File tree

olive/cli/benchmark.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,11 @@ def register_subcommand(parser: ArgumentParser):
7373
type=str,
7474
default="auto",
7575
choices=["auto", "ort", "ortgenai"],
76-
help="Backend for ONNX model evaluation. Use 'auto' to infer backend from model type.",
76+
help=(
77+
"Backend for lm-eval model evaluation. 'ort' and 'ortgenai' require ONNX input; "
78+
"'ortgenai' additionally requires GenAI-packaged model assets (e.g., genai_config.json). "
79+
"'auto' infers backend from model type."
80+
),
7781
)
7882

7983
add_logging_options(sub_parser)

olive/engine/engine.py

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
from olive.cache import CacheConfig, OliveCache
1515
from olive.common.config_utils import validate_config
1616
from olive.common.constants import DEFAULT_WORKFLOW_ID, LOCAL_INPUT_MODEL_ID
17+
from olive.common.utils import hash_dict
1718
from olive.engine.config import FAILED_CONFIG, INVALID_CONFIG, PRUNED_CONFIGS, RunPassConfig
1819
from olive.engine.footprint import Footprint, FootprintNodeMetric
1920
from olive.engine.output import WorkflowOutput
@@ -791,8 +792,16 @@ def _evaluate_model(
791792
else:
792793
model_id_with_accelerator = model_id
793794

795+
# include a hash of the evaluator config in the cache key so that changing the
796+
# evaluation parameters (e.g. metrics, lm-eval tasks/limit/batch_size, or the
797+
# ort/ortgenai backend) does not silently reuse a stale result cached for the
798+
# same model and accelerator.
799+
eval_cache_key = model_id_with_accelerator
800+
if evaluator_config is not None:
801+
eval_cache_key = f"{model_id_with_accelerator}-{hash_dict(evaluator_config.to_json())[:8]}"
802+
794803
# load evaluation from cache if it exists
795-
signal = self._load_evaluation(model_id_with_accelerator)
804+
signal = self._load_evaluation(eval_cache_key)
796805
if signal is not None:
797806
logger.debug("Loading evaluation from cache ...")
798807
# footprint evaluation
@@ -810,7 +819,7 @@ def _evaluate_model(
810819
signal = self.target.evaluate_model(model_config, evaluator_config, accelerator_spec)
811820

812821
# cache evaluation
813-
self._cache_evaluation(model_id_with_accelerator, signal)
822+
self._cache_evaluation(eval_cache_key, signal)
814823

815824
# footprint evaluation
816825
self.footprint.record(

test/cli/test_cli.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -864,6 +864,7 @@ def test_benchmark_command_hfmodel(_, mock_run, tmp_path):
864864
assert config["evaluators"]["evaluator"]["batch_size"] == 8
865865
assert config["evaluators"]["evaluator"]["max_length"] == 1024
866866
assert config["evaluators"]["evaluator"]["limit"] == 16
867+
assert "model_class" not in config["evaluators"]["evaluator"]
867868
assert mock_run.call_count == 1
868869

869870

@@ -902,6 +903,7 @@ def test_benchmark_command_onnxmodel(mock_run, tmp_path):
902903
assert config["evaluators"]["evaluator"]["batch_size"] == 8
903904
assert config["evaluators"]["evaluator"]["max_length"] == 1024
904905
assert config["evaluators"]["evaluator"]["limit"] == 16
906+
assert "model_class" not in config["evaluators"]["evaluator"]
905907
assert mock_run.call_count == 1
906908

907909

test/engine/test_engine.py

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -446,6 +446,54 @@ def test_run_evaluate_input_model(self, mock_local_system_init, tmpdir):
446446
result_data = json.load(f)
447447
assert MetricResult.model_validate(result_data).to_json() == output_model.metrics_value
448448

449+
def test_evaluate_model_cache_key_includes_evaluator_config(self, tmp_path):
450+
# setup: two evaluator configs that differ only in the evaluation parameters
451+
# (here the accuracy goal, standing in for e.g. lm-eval tasks/limit or the
452+
# ort/ortgenai backend). They must not share a cached evaluation result for
453+
# the same model and accelerator.
454+
metric = get_accuracy_metric(AccuracySubType.ACCURACY_SCORE, goal_value=0.99)
455+
evaluator_config = OliveEvaluatorConfig(metrics=[metric])
456+
other_evaluator_config = OliveEvaluatorConfig(
457+
metrics=[get_accuracy_metric(AccuracySubType.ACCURACY_SCORE, goal_value=0.5)]
458+
)
459+
options = {
460+
"cache_config": {
461+
"cache_dir": tmp_path,
462+
"clean_cache": True,
463+
"clean_evaluation_cache": True,
464+
},
465+
"search_strategy": None,
466+
"evaluator": evaluator_config,
467+
}
468+
metric_result_dict = {
469+
joint_metric_key(metric.name, sub_metric.name): {
470+
"value": 0.998,
471+
"priority": sub_metric.priority,
472+
"higher_is_better": sub_metric.higher_is_better,
473+
}
474+
for sub_metric in metric.sub_types
475+
}
476+
477+
engine = Engine(**options)
478+
engine.target = MagicMock()
479+
engine.target.evaluate_model.return_value = MetricResult.model_validate(metric_result_dict)
480+
engine.cache.prepare_resources_for_local = MagicMock(side_effect=lambda config: config)
481+
482+
model_config = get_pytorch_model_config()
483+
model_id = "model_1"
484+
485+
# first evaluation runs and caches the result
486+
engine._evaluate_model(model_config, model_id, evaluator_config, DEFAULT_CPU_ACCELERATOR)
487+
assert engine.target.evaluate_model.call_count == 1
488+
489+
# identical evaluator config -> cache hit, no re-evaluation
490+
engine._evaluate_model(model_config, model_id, evaluator_config, DEFAULT_CPU_ACCELERATOR)
491+
assert engine.target.evaluate_model.call_count == 1
492+
493+
# different evaluator config for the same model and accelerator -> cache miss
494+
engine._evaluate_model(model_config, model_id, other_evaluator_config, DEFAULT_CPU_ACCELERATOR)
495+
assert engine.target.evaluate_model.call_count == 2
496+
449497
@patch("olive.systems.local.LocalSystem")
450498
def test_run_no_pass(self, mock_local_system_init, tmp_path):
451499
# setup

0 commit comments

Comments
 (0)