diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a6baa5a42..60e29f89cb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,20 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), ### Fixed +- **A vLLM profile round had its profiler bounds dropped before launch.** The + argv preflight probe sees only `EXTRA_VLLM_ARGS`, while the launcher appends + `--profiler-config.profiler torch` and a trace directory of its own + afterwards. `ProfilerConfig` refuses the iteration bounds this layer injects + unless both are present in the same fragment, so the probe rejected an argv + that is valid once the launcher's flags are appended, and the round then ran + with no bound on the capture window. Both flags are now asserted alongside + the bounds so the probed fragment is self-consistent on its own. The trace + directory is a placeholder: the launcher's own value has to win vLLM's + last-wins dotted-flag merge, so the bypass backend now emits its profiler + flags after `EXTRA_VLLM_ARGS` the way Magpie's launcher already does, rather + than before it where the placeholder would have won and sent the trace + somewhere trace discovery never looks. An operator-set profiler flag is left + untouched. - **A campaign the host killed cost the next task in the same repository.** Every in-place task is handed the same `forge_experiments` directory, and the release archives it -- but a run that was killed never reaches the release. diff --git a/src/hyperloom/inference_optimizer/tests/test_bypass_backend.py b/src/hyperloom/inference_optimizer/tests/test_bypass_backend.py index edc10b8f91..e0e4aa37d2 100644 --- a/src/hyperloom/inference_optimizer/tests/test_bypass_backend.py +++ b/src/hyperloom/inference_optimizer/tests/test_bypass_backend.py @@ -178,6 +178,32 @@ def test_server_command_sglang(): assert cmd[-2:] == ["--foo", "1"] +def test_the_backend_trace_dir_wins_over_the_preflight_placeholder(): + """EXTRA_VLLM_ARGS carries a trace dir only so the argv preflight accepts the bounds beside it. + + ProfilerConfig refuses ``profiler=torch`` without a ``torch_profiler_dir``, so the probed + fragment has to name one; it is a placeholder, and the launcher's own value has to win vLLM's + last-wins merge or the trace lands where this backend's discovery never looks. + """ + cmd = bypass_engine.build_server_command( + framework="vllm", + model="/m", + tp=1, + port=8888, + max_model_len=None, + extra_args=[ + "--profiler-config.profiler", + "torch", + "--profiler-config.torch_profiler_dir", + "/round-dir", + ], + profile_dir="/ws/torch_trace", + ) + + dirs = [cmd[i + 1] for i, token in enumerate(cmd) if token == "--profiler-config.torch_profiler_dir"] + assert dirs[-1] == "/ws/torch_trace" + + def test_sglang_atom_server_command_honors_python_exe(): """sglang/atom launch under the provided interpreter (not a PATH python3).""" sglang = bypass_engine.build_server_command( diff --git a/src/hyperloom/inference_optimizer/tests/test_kernel_request_handlers_units.py b/src/hyperloom/inference_optimizer/tests/test_kernel_request_handlers_units.py index 0270025777..ed211435d5 100644 --- a/src/hyperloom/inference_optimizer/tests/test_kernel_request_handlers_units.py +++ b/src/hyperloom/inference_optimizer/tests/test_kernel_request_handlers_units.py @@ -3658,6 +3658,10 @@ async def fake_run(cmd: list[str], *, timeout_sec: int): def test_handler_passes_non_fp8_geak_to_next_hyperloom_prereq(self, tmp_path, monkeypatch): monkeypatch.setenv("GEMM_TUNING_BACKEND", "geak") + # The backend is chosen by KERNEL_OPT_BACKEND_ORDER, not by GEMM_TUNING_BACKEND, so + # leaving it to the ambient environment sends this down the forge branch instead -- + # which reports model_path_missing, a prerequisite this test is not about. + monkeypatch.delenv("KERNEL_OPT_BACKEND_ORDER", raising=False) monkeypatch.delenv("HYPERLOOM_KERNEL_AGENT_ROOT", raising=False) state = SharedState(precision="bf16", framework="sglang") state.save(tmp_path) diff --git a/src/hyperloom/inference_optimizer/tests/test_profile_and_kernel_handlers.py b/src/hyperloom/inference_optimizer/tests/test_profile_and_kernel_handlers.py index 7bfbba316d..82c34df0d0 100644 --- a/src/hyperloom/inference_optimizer/tests/test_profile_and_kernel_handlers.py +++ b/src/hyperloom/inference_optimizer/tests/test_profile_and_kernel_handlers.py @@ -446,6 +446,40 @@ def test_materialize_profile_window_vllm_skill_formula_default_R( extra = rendered["benchmark"]["envs"]["EXTRA_VLLM_ARGS"] assert "--profiler-config.delay_iterations 6080" in extra, extra assert "--profiler-config.max_iterations 128" in extra, extra + # ``profiler=torch`` and a trace dir have to be asserted here too, not left to + # Magpie's launcher script alone: that script appends its own flags *after* + # EXTRA_VLLM_ARGS at actual launch, but the argv preflight probe only sees + # EXTRA_VLLM_ARGS, and vLLM's ProfilerConfig validator rejects + # delay/max_iterations without both present in the checked fragment. + assert "--profiler-config.profiler torch" in extra, extra + assert "--profiler-config.torch_profiler_dir" in extra, extra + + +def test_materialize_profile_does_not_duplicate_an_explicit_profiler_flag( + tmp_path, + monkeypatch, +): + """An operator-set ``profiler``/``torch_profiler_dir`` must not be doubled.""" + import yaml + + _clear_workload_env(monkeypatch) + src = _profile_yaml( + tmp_path, + "vllm", + { + "CONC": 32, + "ISL": 256, + "OSL": 1024, + "EXTRA_VLLM_ARGS": ( + "--profiler-config.profiler torch --profiler-config.torch_profiler_dir /tmp/operator-dir" + ), + }, + ) + out = _materialize_config_with_envs(src, tmp_path) + extra = yaml.safe_load(out.read_text())["benchmark"]["envs"]["EXTRA_VLLM_ARGS"] + assert extra.count("--profiler-config.profiler") == 1, extra + assert extra.count("--profiler-config.torch_profiler_dir") == 1, extra + assert "/tmp/operator-dir" in extra, extra def test_materialize_profile_window_vllm_skill_formula_explicit_R( @@ -734,6 +768,8 @@ def test_materialize_profile_restore_accepts_a_bound_that_already_holds( tmp_path, extra_envs={ "EXTRA_VLLM_ARGS": ( + "--profiler-config.profiler torch " + "--profiler-config.torch_profiler_dir /tmp/already-set " "--profiler-config.delay_iterations 6080 " "--profiler-config.max_iterations 64 " "--profiler-config.ignore_frontend True " diff --git a/src/hyperloom/orchestrator/actions/executors/_workload_envs.py b/src/hyperloom/orchestrator/actions/executors/_workload_envs.py index 0e7ffb3347..6372efa18b 100644 --- a/src/hyperloom/orchestrator/actions/executors/_workload_envs.py +++ b/src/hyperloom/orchestrator/actions/executors/_workload_envs.py @@ -1494,6 +1494,27 @@ def materialize_config_with_envs( ("delay_iterations", f"--profiler-config.delay_iterations {delay_iters}"), ("max_iterations", f"--profiler-config.max_iterations {max_iters}"), ] + # ``profiler`` and ``torch_profiler_dir`` are normally set by + # Magpie's launcher script, not by this layer -- but that script + # appends its own flags *after* EXTRA_VLLM_ARGS in the real + # ``vllm serve`` invocation, so the argv preflight probe (which + # only sees EXTRA_VLLM_ARGS) checks capture_torch_profiler/ + # delay_iterations/max_iterations against a ProfilerConfig that + # never saw ``profiler=torch`` or a trace dir. vLLM's validator + # requires both whenever those bounds are present, so the probe + # fails an argv that will be valid once Magpie's flags are + # appended, and this layer's profiler bounds get treated as + # invalid and dropped instead of launched. Asserting placeholders + # here keeps the probed fragment self-consistent; the actual + # ``torch_profiler_dir`` Magpie computes from ``$WORKSPACE_DIR`` + # overrides this one at real launch time via vLLM's dotted-flag + # last-wins merge, so the value here only has to be a valid + # absolute path, not the directory the trace ends up under. An + # operator-set flag is left untouched either way. + if _profiler_flag_value(existing_vllm_args, "profiler") is None: + profiler_flags.append(("profiler", "--profiler-config.profiler torch")) + if _profiler_flag_value(existing_vllm_args, "torch_profiler_dir") is None: + profiler_flags.append(("torch_profiler_dir", f"--profiler-config.torch_profiler_dir {output_dir}")) if tracelens_patch_ok: profiler_flags.append(("capture_torch_profiler", "--profiler-config.capture_torch_profiler True")) profiler_flags.append(("detailed_trace_annotation", "--profiler-config.detailed_trace_annotation True")) diff --git a/src/hyperloom/orchestrator/actions/executors/bypass_engine.py b/src/hyperloom/orchestrator/actions/executors/bypass_engine.py index 75b7772ff1..c6f5ba560c 100644 --- a/src/hyperloom/orchestrator/actions/executors/bypass_engine.py +++ b/src/hyperloom/orchestrator/actions/executors/bypass_engine.py @@ -85,16 +85,26 @@ def build_server_command( ] if max_model_len: cmd += ["--max-model-len", str(max_model_len)] - if profile_dir: - # vLLM enables the torch profiler via --profiler-config (the legacy VLLM_TORCH_PROFILER_DIR env is - # ignored), without which /start_profile returns 404 and no trace is written. - cmd += [ + if not profile_dir: + return cmd + list(extra_args) + # vLLM enables the torch profiler via --profiler-config (the legacy VLLM_TORCH_PROFILER_DIR env is + # ignored), without which /start_profile returns 404 and no trace is written. + # + # Last, after extra_args, the way Magpie's launcher orders its own: EXTRA_VLLM_ARGS has to carry a + # torch_profiler_dir for the argv preflight to accept the profile bounds beside it -- ProfilerConfig + # refuses `profiler=torch` without one -- and that value is a placeholder standing in for whatever the + # launcher computes. Emitting it first would let the placeholder win vLLM's last-wins dotted-flag merge + # and send the trace somewhere this backend's discovery never looks. + return ( + cmd + + list(extra_args) + + [ "--profiler-config.profiler", "torch", "--profiler-config.torch_profiler_dir", profile_dir, ] - return cmd + list(extra_args) + ) if fw == "atom": cmd = [ interp,