diff --git a/src/olmo_eval/cli/run/config.py b/src/olmo_eval/cli/run/config.py index 2e51981c2..d12f2db3d 100644 --- a/src/olmo_eval/cli/run/config.py +++ b/src/olmo_eval/cli/run/config.py @@ -69,6 +69,21 @@ def _merge_dict_into_list_items(items: list[Any], override: dict[str, Any], key_ _deep_merge(item, copy.deepcopy(override)) +def _overrides_provider_kind(overrides: list[str]) -> bool: + """Whether dotlist overrides explicitly set the harness provider kind.""" + for override in overrides: + if "=" not in override: + continue + key_path, value = override.split("=", 1) + if key_path == "provider.kind": + return True + if key_path == "provider": + parsed_value = _parse_override_value(value) + if isinstance(parsed_value, dict) and "kind" in parsed_value: + return True + return False + + def _apply_dotlist_overrides(base_dict: dict[str, Any], overrides: list[str]) -> dict[str, Any]: """Apply dotlist overrides to a dictionary, handling list indices. @@ -391,11 +406,14 @@ def _resolve_harness_config(self, model_name: str) -> HarnessConfig: harness_config: HarnessConfig + preserve_provider_kind = False + if self.harness_preset: try: from olmo_eval.harness import get_harness_preset harness_config = get_harness_preset(self.harness_preset) + preserve_provider_kind = True except ValueError as e: console.print(f"[red]Error:[/red] {e}") raise SystemExit(1) from None @@ -412,6 +430,8 @@ def _resolve_harness_config(self, model_name: str) -> HarnessConfig: else: config_dict = yaml.safe_load(f) + provider_dict = config_dict.get("provider", {}) + preserve_provider_kind = isinstance(provider_dict, dict) and "kind" in provider_dict harness_config = HarnessConfig.from_dict(config_dict) console.print(f"[dim]Using harness config: {self.harness_config_path}[/dim]") except FileNotFoundError: @@ -431,6 +451,9 @@ def _resolve_harness_config(self, model_name: str) -> HarnessConfig: # Apply CLI overrides to harness config if self.cli_harness_overrides: + preserve_provider_kind = preserve_provider_kind or _overrides_provider_kind( + self.cli_harness_overrides + ) harness_dict = harness_config.to_dict() harness_dict = _apply_dotlist_overrides(harness_dict, self.cli_harness_overrides) harness_config = HarnessConfig.from_dict(harness_dict) @@ -438,7 +461,10 @@ def _resolve_harness_config(self, model_name: str) -> HarnessConfig: from olmo_eval.common.configs import get_provider_config provider_config = get_provider_config(model_name) - harness_config = harness_config.merge_provider(provider_config) + harness_config = harness_config.merge_provider( + provider_config, + preserve_provider_kind=preserve_provider_kind, + ) if self.force_download_model: harness_config = harness_config.with_provider_overrides(force_download=True) diff --git a/src/olmo_eval/harness/config.py b/src/olmo_eval/harness/config.py index 0f6d50785..e731adbc5 100644 --- a/src/olmo_eval/harness/config.py +++ b/src/olmo_eval/harness/config.py @@ -236,11 +236,18 @@ def with_provider(self, provider: ProviderConfig) -> HarnessConfig: """Create a new config with a different provider configuration.""" return replace(self, provider=provider) - def merge_provider(self, provider: ProviderConfig) -> HarnessConfig: + def merge_provider( + self, + provider: ProviderConfig, + *, + preserve_provider_kind: bool = False, + ) -> HarnessConfig: """Merge model info from provider while preserving harness provider settings. - The harness's provider kind takes precedence if explicitly set (non-default), - while model-specific fields come from the new provider. + Harness provider settings take precedence when they differ from the base + provider defaults, while model-specific fields come from the new provider. + Set preserve_provider_kind when the caller knows the harness provider kind + was explicitly configured, even if it equals the base default. """ defaults = ProviderConfig() overrides = { @@ -248,6 +255,8 @@ def merge_provider(self, provider: ProviderConfig) -> HarnessConfig: for f in fields(self.provider) if getattr(self.provider, f.name) != getattr(defaults, f.name) } + if preserve_provider_kind: + overrides["kind"] = self.provider.kind # kwargs should merge, not replace if self.provider.kwargs: overrides["kwargs"] = {**provider.kwargs, **self.provider.kwargs} diff --git a/tests/cli/beaker/test_launch.py b/tests/cli/beaker/test_launch.py index 6984b8ceb..d9ca5a1b5 100644 --- a/tests/cli/beaker/test_launch.py +++ b/tests/cli/beaker/test_launch.py @@ -194,6 +194,61 @@ def test_provider_package_skips_vllm_extra_in_job_config(self): assert job_config.provider_packages is not None assert "https://github.com/user/vllm@custom" in job_config.provider_packages + def test_default_harness_vllm_server_install_matches_runtime_provider(self): + """Default harness should install and run vLLM model presets as vllm_server.""" + from unittest.mock import patch + + from olmo_eval.cli.beaker.config_loader import LaunchConfig + from olmo_eval.cli.beaker.experiment_plan import ExperimentPlan + from olmo_eval.cli.beaker.job_assembler import JobConfigAssembler + from olmo_eval.cli.run.config import RunConfigBuilder + from olmo_eval.common.types import ProviderKind + + launch_config = LaunchConfig( + name="test", + model_specs=["olmo-3-1025-7b"], + task_specs=["humaneval"], + cluster="h100", + workspace="ai2/test", + budget="ai2/test", + harness="default", + ) + exp = ExperimentPlan( + name="test", + model_spec="olmo-3-1025-7b", + priority="normal", + tasks=["humaneval"], + original_task_specs=["humaneval"], + total_expanded_tasks=1, + num_gpus=1, + ) + assembler = JobConfigAssembler( + config=launch_config, + effective_image="test-image", + effective_groups=[], + beaker_username="test-user", + common_secrets=[], + store_secrets=[], + task_secrets=[], + inject_aws_credentials=False, + inject_gcs_credentials=False, + ) + + with patch("olmo_eval.cli.beaker.job_assembler.cluster_has_weka", return_value=False): + job_config = assembler.assemble(exp) + + run_config = RunConfigBuilder( + model=exp.model_spec, + task=tuple(exp.tasks), + output_dir="/tmp/results", + harness_preset=launch_config.harness, + ).build() + + assert job_config.vllm_isolated_venv is True + assert "vllm" in job_config.extras + assert "clients" in job_config.extras + assert run_config.provider_config.kind == ProviderKind.VLLM_SERVER + def test_olmo_core_provider_package_replaces_olmo_core_extra(self): """OLMo-core package overrides should replace the bundled olmo_core extra.""" from unittest.mock import patch diff --git a/tests/cli/run/__init__.py b/tests/cli/run/__init__.py new file mode 100644 index 000000000..c620fb27d --- /dev/null +++ b/tests/cli/run/__init__.py @@ -0,0 +1 @@ +"""Tests for run CLI configuration.""" diff --git a/tests/cli/run/test_config.py b/tests/cli/run/test_config.py new file mode 100644 index 000000000..6a0ddff3e --- /dev/null +++ b/tests/cli/run/test_config.py @@ -0,0 +1,35 @@ +"""Tests for run command configuration assembly.""" + +from olmo_eval.cli.run.config import RunConfigBuilder +from olmo_eval.common.types import ProviderKind + + +class TestRunConfigBuilder: + """Tests for RunConfigBuilder provider resolution.""" + + def test_named_harness_preset_provider_kind_overrides_model_preset(self): + """The default harness should run vLLM model presets through vllm_server.""" + builder = RunConfigBuilder( + model="olmo-3-1025-7b", + task=("humaneval",), + output_dir="/tmp/results", + harness_preset="default", + ) + + config = builder.build() + + assert config.provider_config.kind == ProviderKind.VLLM_SERVER + assert config.provider_config.model == "allenai/Olmo-3-1025-7B" + + def test_model_preset_provider_kind_is_used_without_harness_preset(self): + """Without a harness preset, model presets should keep their provider kind.""" + builder = RunConfigBuilder( + model="olmo-3-1025-7b", + task=("humaneval",), + output_dir="/tmp/results", + ) + + config = builder.build() + + assert config.provider_config.kind == ProviderKind.VLLM + assert config.provider_config.model == "allenai/Olmo-3-1025-7B" diff --git a/tests/core/harness/test_harness.py b/tests/core/harness/test_harness.py index b05cba94b..082225aed 100644 --- a/tests/core/harness/test_harness.py +++ b/tests/core/harness/test_harness.py @@ -163,6 +163,42 @@ def test_harness_no_inject_if_no_prompt(self, mock_provider_config): assert injected == messages +class TestHarnessConfig: + """Tests for HarnessConfig helpers.""" + + def test_merge_provider_preserves_explicit_default_provider_kind(self): + """Explicit harness provider kind should win even when it matches the default.""" + harness_config = HarnessConfig( + name="default", + provider=ProviderConfig(kind=ProviderKind.VLLM_SERVER), + ) + model_provider = ProviderConfig( + kind=ProviderKind.VLLM, + model="allenai/Olmo-3-1025-7B", + ) + + merged = harness_config.merge_provider( + model_provider, + preserve_provider_kind=True, + ) + + assert merged.provider.kind == ProviderKind.VLLM_SERVER + assert merged.provider.model == "allenai/Olmo-3-1025-7B" + + def test_merge_provider_uses_model_kind_without_explicit_harness_kind(self): + """The default empty harness should not override model preset provider kind.""" + harness_config = HarnessConfig(name="default") + model_provider = ProviderConfig( + kind=ProviderKind.VLLM, + model="allenai/Olmo-3-1025-7B", + ) + + merged = harness_config.merge_provider(model_provider) + + assert merged.provider.kind == ProviderKind.VLLM + assert merged.provider.model == "allenai/Olmo-3-1025-7B" + + class TestCreateHarness: """Tests for create_harness factory function."""