From 4b8206748be9671fd36ef9cdf82d2736229cdfdd Mon Sep 17 00:00:00 2001 From: Eugene Volen Date: Mon, 12 Oct 2026 14:50:36 +0200 Subject: [PATCH] vllm: point import failures at rocm diagnose When vLLM aborts at startup because the engine cannot import (for example libtorch_cuda.so missing), the failure message now names the failing line and the command that diagnoses it, as it already does for out of memory. The command quotes the real log line only when it is safe to quote and diagnose recognises it. Otherwise it falls back to a canonical symptom that diagnose is known to match. The guard that decides this runs the check registry for both Linux and WSL hosts, so the engine cannot name a command that finds nothing on either. The out of memory guard uses the same per-family check. The import check is now registered for Linux and WSL, like the vLLM out of memory check, and its recipe applies on both. The published catalog, the doctor skill reference and the WSL docs list the new platform. The hint fires on a line naming libtorch_cuda.so that also contains OSError: or ImportError:, taking the last such line, or on a traceback frame of the extension loader. A warning that only names the loader module, or that only quotes the exception, stays quiet. The hint only reports. The matching remediation is print-only. Other failures still print only the log tail, and an out of memory failure keeps its memory hint, printed first. Signed-off-by: Eugene Volen --- crates/rocm-core/src/diagnose.rs | 171 ++++++++-- crates/rocm-core/src/fix.rs | 7 +- crates/rocm-core/src/lib.rs | 6 +- docs/vllm.md | 39 +++ docs/wsl.md | 9 +- doctor-catalog.json | 4 + engines/vllm/src/process.rs | 320 +++++++++++++++++- skills/rocm-doctor/reference.md | 6 +- tests/e2e-cucumber/features/diagnose.feature | 23 +- .../e2e-cucumber/tests/e2e/diagnose_steps.rs | 36 +- 10 files changed, 572 insertions(+), 49 deletions(-) diff --git a/crates/rocm-core/src/diagnose.rs b/crates/rocm-core/src/diagnose.rs index 887ff80c1..f78bd3ce2 100644 --- a/crates/rocm-core/src/diagnose.rs +++ b/crates/rocm-core/src/diagnose.rs @@ -1744,11 +1744,7 @@ fn vllm_anchored_lines(symptom: &str) -> String { /// a cause rather than reading as "the tool checked and found nothing". #[must_use] pub fn vllm_oom_symptom_is_diagnosable(symptom: &str) -> bool { - let e = Examination { - os_family: "linux".to_owned(), - ..Examination::default() - }; - check_16_vllm_oom(&e, symptom).score >= MIN_SCORE_FOR_MATCH + scores_on_every_engine_host(symptom, "fix-16-vllm-oom", run_all_checks) } /// vLLM ran the GPU out of memory. Keyword-only: an [`Examination`] carries no @@ -1942,6 +1938,60 @@ fn check_18_comgr_conflict(e: &Examination, symptom: &str) -> Diagnosis { finalize(ID, TITLE, score, evidence, fix) } +/// A canonical engine-startup import-failure `--symptom` string guaranteed to +/// clear [`MIN_SCORE_FOR_MATCH`]. +/// +/// `libtorch_cuda.so` is the one token in that traceback that cannot mean +/// anything else (a ROCm build of torch ships `libtorch_hip.so` and never that +/// file), and it is worth exactly the threshold on its own on purpose; see +/// [`KEYWORDS_TORCH_DLPACK_CUDA_VARIANT`]. The vLLM engine falls back to this when +/// the user's actual failing line would not itself be diagnosable, so the +/// `rocm diagnose` command it prints always reports a cause. +pub const VLLM_IMPORT_CANONICAL_SYMPTOM: &str = + "OSError: libtorch_cuda.so: cannot open shared object file"; + +/// Whether `symptom` clears [`MIN_SCORE_FOR_MATCH`] for the engine-startup +/// import failure. +/// +/// The vLLM engine calls this to decide whether the user's actual failing line +/// is worth routing into a `--symptom` example, or whether to fall back to +/// [`VLLM_IMPORT_CANONICAL_SYMPTOM`] so the printed command is guaranteed to +/// report a cause rather than reading as "the tool checked and found nothing". +/// +/// The hint fires on every host the vLLM engine runs on, which is Linux and +/// WSL2. The symptom therefore has to clear the threshold on each of those +/// families, run through the same registry `rocm diagnose` uses, so a check +/// that is registered for one family and not the other cannot make the engine +/// name a command that finds nothing. +#[must_use] +pub fn vllm_import_symptom_is_diagnosable(symptom: &str) -> bool { + scores_on_every_engine_host(symptom, "fix-17-torch-dlpack", run_all_checks) +} + +/// Whether the entry `id` clears [`MIN_SCORE_FOR_MATCH`] for `symptom` on every +/// host family the vLLM engine runs on (Linux and WSL2). `run` is the registry +/// run for one host. It is a parameter so a test can hand in a registry where +/// the entry is missing on one family. +fn scores_on_every_engine_host( + symptom: &str, + id: &str, + run: impl Fn(&Examination, &str) -> Vec, +) -> bool { + let linux = Examination { + os_family: "linux".to_owned(), + ..Examination::default() + }; + let wsl = Examination { + is_wsl: true, + ..linux.clone() + }; + [linux, wsl].iter().all(|host| { + run(host, symptom) + .iter() + .any(|d| d.id == id && d.score >= MIN_SCORE_FOR_MATCH) + }) +} + /// The vLLM engine-startup import failure (EAI-8012). /// /// Keyword-only, and not by preference. The fact that decides this failure is @@ -1953,15 +2003,15 @@ fn check_18_comgr_conflict(e: &Examination, symptom: &str) -> Diagnosis { /// why the parameter is unused: the user has to supply the error text through /// `rocm diagnose --symptom ""`. /// -/// That is the only route to this entry from the symptom, and it is a narrow -/// one: nothing on the serve or `rocm services` path points at `rocm diagnose` -/// at all — a service that died at startup renders a `logs:` and a `restart:` -/// hint and no more — so reaching this requires already knowing to paste the -/// error into `diagnose`. The recipe is also reachable by name (`rocm fix` lists -/// it, `rocm fix fix-17-torch-dlpack` prints it), but that needs the id rather -/// than the symptom. Closing that gap means adding a diagnose hint to the -/// service-failure output, which changes a shared surface for every failed -/// service regardless of cause and belongs in its own change. +/// That is the only route to this entry from the symptom. The vLLM engine's own +/// startup-failure message now points at it (`import_failure_hint` in +/// `engines/vllm`, which prints `rocm diagnose --symptom ''`), +/// but only for that engine. A service that died at startup for any other reason +/// still renders a `logs:` and a `restart:` hint and no more, because adding a +/// diagnose hint there changes a shared surface for every failed service +/// regardless of cause and belongs in its own change. The recipe is also +/// reachable by name (`rocm fix` lists it, `rocm fix fix-17-torch-dlpack` prints +/// it), but that needs the id rather than the symptom. fn check_17_torch_dlpack_cuda_variant(_e: &Examination, symptom: &str) -> Diagnosis { let (score, evidence) = keyword_score(symptom, KEYWORDS_TORCH_DLPACK_CUDA_VARIANT); if score <= 0 { @@ -2707,11 +2757,13 @@ const CHECKERS: &[Checker] = &[ (check_14_adrenalin_too_old, &["windows"]), (check_15_msvc_redist, &["windows"]), (check_16_vllm_oom, &["linux", "wsl"]), - // Linux-only: `libtorch_cuda.so` is an ELF name, and the vLLM engine is - // gated off native Windows. 17 rather than 16 because the vLLM - // out-of-memory entry above already holds 16; the number is a stable - // handle, so the two do not get to share one. - (check_17_torch_dlpack_cuda_variant, &["linux"]), + // Linux and WSL2: `libtorch_cuda.so` is an ELF name, and the vLLM engine is + // gated off native Windows but runs on WSL2. The engine's startup-failure + // hint fires on both, so the entry it points at must be reachable on both. + // 17 rather than 16 because the vLLM out-of-memory entry above already + // holds 16; the number is a stable handle, so the two do not get to share + // one. + (check_17_torch_dlpack_cuda_variant, &["linux", "wsl"]), (check_wsl_1_gpu_not_exposed, WSL_ONLY), (check_wsl_2_dxcore_missing, WSL_ONLY), (check_wsl_3_rocdxg_missing, WSL_ONLY), @@ -2828,9 +2880,9 @@ pub fn diagnose(e: &Examination, symptom: &str) -> DiagnoseReport { // in the catalog now, with its own entries; what keeps the bare-metal checks // off it is `platform_family`, not a special case at this level. // - // `fix-16-vllm-oom` opts into `wsl` in CHECKERS for that reason: it is a - // keyword-only check whose failure mode is just as real under WSL2, so it - // still answers there alongside the WSL entries. + // `fix-16-vllm-oom` and `fix-17-torch-dlpack` opt into `wsl` in CHECKERS for + // that reason: they are keyword-only checks whose failure modes are just as + // real under WSL2, so they still answer there alongside the WSL entries. // // `out_of_scope` still exists, for the platforms that genuinely have no // entries. That case used to fall through to an empty catalog and report "no @@ -6427,6 +6479,81 @@ mod tests { ); } + #[test] + fn canonical_vllm_import_symptom_is_diagnosable() { + // The engine's fallback symptom must always clear the threshold, so the + // `rocm diagnose` command it prints never reports nothing. + assert!(vllm_import_symptom_is_diagnosable( + VLLM_IMPORT_CANONICAL_SYMPTOM + )); + // A frame that names only the shim's module, and not the missing library, + // is not enough on its own, and the engine has to know that. + assert!(!vllm_import_symptom_is_diagnosable( + "File \"tvm_ffi/_optional_torch_c_dlpack.py\", line 42, in " + )); + assert!(!vllm_import_symptom_is_diagnosable("an ordinary failure")); + } + + #[test] + fn vllm_import_keyword_diagnosis_is_available_on_wsl() { + // The engine's import hint fires on WSL2 too, so `rocm diagnose + // --symptom` has to answer there. Check 17 used to be registered for + // Linux only, and this report had no match. + let report = diagnose(&wsl_base(), VLLM_IMPORT_CANONICAL_SYMPTOM); + assert!(report.out_of_scope.is_none()); + assert!(report.has_match()); + assert_eq!(report.matched[0].id, "fix-17-torch-dlpack"); + } + + #[test] + fn the_engine_host_guard_requires_the_entry_to_score_on_every_host_family() { + // A registry stand-in: `listed_on` says which hosts list the entry. + fn registry( + id: &'static str, + listed_on_wsl: bool, + listed_on_linux: bool, + ) -> impl Fn(&Examination, &str) -> Vec { + move |host, _symptom| { + let listed = if host.is_wsl { + listed_on_wsl + } else { + listed_on_linux + }; + if !listed { + return Vec::new(); + } + vec![Diagnosis { + id: id.to_owned(), + score: 100, + ..Diagnosis::default() + }] + } + } + let guard = |run| scores_on_every_engine_host("x", "fix-17-torch-dlpack", run); + assert!(guard(registry("fix-17-torch-dlpack", true, true))); + assert!( + !guard(registry("fix-17-torch-dlpack", false, true)), + "an entry missing on WSL must not count" + ); + assert!( + !guard(registry("fix-17-torch-dlpack", true, false)), + "an entry missing on Linux must not count" + ); + assert!( + !guard(registry("fix-16-vllm-oom", true, true)), + "a different entry scoring is not this entry scoring" + ); + // And the real registry lists both entries on both families. + assert!(vllm_import_symptom_is_diagnosable( + VLLM_IMPORT_CANONICAL_SYMPTOM + )); + assert!(vllm_oom_symptom_is_diagnosable(VLLM_OOM_CANONICAL_SYMPTOM)); + // An ISA error scores on another check, not on this one. + assert!(!vllm_import_symptom_is_diagnosable( + "hipErrorNoBinaryForGpu: Unable to find code object for all current devices" + )); + } + #[test] fn canonical_vllm_oom_symptom_is_diagnosable() { // The engine's fallback symptom must always clear the threshold, so the diff --git a/crates/rocm-core/src/fix.rs b/crates/rocm-core/src/fix.rs index 668e552ac..632c8bf40 100644 --- a/crates/rocm-core/src/fix.rs +++ b/crates/rocm-core/src/fix.rs @@ -660,7 +660,12 @@ const RECIPES: &[FixRecipe] = &[ "A service that failed at startup is hidden from a plain `rocm services list`; pass --all to recover its id.", ], doc_url: None, - applies_on: PRINT_ON_LINUX, + // Matches `check_17_torch_dlpack_cuda_variant`'s `["linux", "wsl"]` + // registration. vLLM is supported on WSL2 and its engine runs the same + // ELF import there, so the diagnosis the engine hint points at has to + // be reachable there too. Pinned by + // `every_checker_platform_is_covered_by_its_recipe`. + applies_on: PRINT_ON_LINUX_AND_WSL, runner: None, }, FixRecipe { diff --git a/crates/rocm-core/src/lib.rs b/crates/rocm-core/src/lib.rs index e0a1322ac..8853c358f 100644 --- a/crates/rocm-core/src/lib.rs +++ b/crates/rocm-core/src/lib.rs @@ -66,8 +66,10 @@ pub use crypto::{ verify_rsa_pkcs1_sha256_signature, }; pub use diagnose::{ - DiagnoseReport, Diagnosis, Fix, Route, VLLM_OOM_CANONICAL_SYMPTOM, diagnose as run_diagnose, - render_report_text as render_diagnose_text, vllm_oom_symptom_is_diagnosable, + DiagnoseReport, Diagnosis, Fix, Route, VLLM_IMPORT_CANONICAL_SYMPTOM, + VLLM_OOM_CANONICAL_SYMPTOM, diagnose as run_diagnose, + render_report_text as render_diagnose_text, vllm_import_symptom_is_diagnosable, + vllm_oom_symptom_is_diagnosable, }; pub use disk_space::{ SpaceCheck, available_space_for_path, check_space_for_path, ensure_space_for, diff --git a/docs/vllm.md b/docs/vllm.md index c94c40ba7..c9552a83e 100644 --- a/docs/vllm.md +++ b/docs/vllm.md @@ -267,6 +267,45 @@ rocm serve --engine vllm --gpu 1 --gpu-memory-utilization 0.5 rocm diagnose --symptom 'vllm: torch.OutOfMemoryError: HIP out of memory' ``` +## Engine import failures + +A startup failure can also come from the engine failing to import, with no +memory problem at all. The failure message then names the failing line and +points at `rocm diagnose --symptom ''`. + +The hint fires only when the log tail has one of two kinds of line. The first +is a line that contains `libtorch_cuda.so` and also contains `OSError:` or +`ImportError:`, compared without regard to case. It does not have to start with +that text, because the real line begins with a process prefix such as +`(EngineCore pid=412)`. A warning that quotes the exception in its repr form, +such as `ImportError('libtorch_cuda.so ...')`, does not count. A warning written +with a colon, such as `... with ImportError: libtorch_cuda.so ...`, does count, +and it can raise the hint when the real failure is something else. The +second is a traceback frame, meaning a line that contains `file "` and +`_optional_torch_c_dlpack`, compared without regard to case. The library line +wins when both are present, and when several lines name the library the last +one is quoted. A log line that only mentions the `_optional_torch_c_dlpack` +module, such as a `UserWarning`, does not fire the hint. Any other startup +failure prints only the log tail, except an out-of-memory failure, which prints +the memory hint above. If a failure matches both, both paragraphs are printed, +the memory hint first. + +The printed command quotes the failing line when it can be rendered as one +intact single-quoted argument and `rocm diagnose` recognises it. Otherwise it +quotes the canonical import symptom, +`OSError: libtorch_cuda.so: cannot open shared object file`. A line with an +apostrophe falls back this way. So does a frame that does not score on its own, +such as `... in `, which names only the shim module and scores below the +match threshold. A loader frame that ends in `in load_torch_c_dlpack_extension` +also names the extension, so it scores on its own and is quoted as it is. When a +frame alone does not score, the command quotes a canonical symptom that the +user's log did not contain. The command then always reports a cause, and the +user's own line stays visible in the message above it. + +The command only reports. On Linux and WSL it matches the conditional +engine-reinstall remediation (`fix-17-torch-dlpack`), which `rocm fix` prints +and does not run. + ## Tool calling The TUI chat tab attaches tool definitions to every chat request. vLLM rejects diff --git a/docs/wsl.md b/docs/wsl.md index 554151316..38b76b286 100644 --- a/docs/wsl.md +++ b/docs/wsl.md @@ -185,11 +185,12 @@ The WSL entries, in the order a broken stack usually reveals them: | `fix-wsl-5-distro-too-old` | The distro release is below the floor in the [prerequisites](#prerequisites) above | | `fix-wsl-6-host-driver-too-old` | The distro-side plumbing is complete but the Windows host driver is missing or too old | -Seven entries that are not WSL-specific also run on WSL: `fix-1-arch`, +Eight entries that are not WSL-specific also run on WSL: `fix-1-arch`, `fix-2-unset-override`, `fix-6-path`, `fix-8-wheel-rocm`, `fix-16-vllm-oom`, -`fix-18-comgr-conflict`, and `fix-19-shm-too-small`. `fix-16-vllm-oom` is driven -by the error text you pass with `--symptom` and reads nothing from the -environment or the ROCm install. `fix-18-comgr-conflict` and +`fix-17-torch-dlpack`, `fix-18-comgr-conflict`, and `fix-19-shm-too-small`. +`fix-16-vllm-oom` and `fix-17-torch-dlpack` are driven by the error text you +pass with `--symptom` and read nothing from the environment or the ROCm +install. `fix-18-comgr-conflict` and `fix-19-shm-too-small` apply when you run `rocm diagnose` inside the distro. A `--distro` run skips both (see [Diagnose a distro from Windows](#diagnose-a-distro-from-windows) later in this diff --git a/doctor-catalog.json b/doctor-catalog.json index dbe36976a..504ac05cd 100644 --- a/doctor-catalog.json +++ b/doctor-catalog.json @@ -264,6 +264,10 @@ { "os": "linux", "class": "print-only" + }, + { + "os": "wsl", + "class": "print-only" } ], "needs_sudo": false, diff --git a/engines/vllm/src/process.rs b/engines/vllm/src/process.rs index 86281468e..2b86bac4e 100644 --- a/engines/vllm/src/process.rs +++ b/engines/vllm/src/process.rs @@ -614,9 +614,9 @@ fn prepend_path_entries(entries: &[PathBuf], current: Option) -> Resul /// stripping control bytes out of the one echoed line further down while /// printing the raw tail a few lines above would have left the escapes a clear /// path to the terminal inside the same message. Stripping is per line so the -/// tail keeps its shape; `oom_utilization_hint` still reads the *raw* tail, -/// because what it selects from it is quoted into a command and has its own, -/// stricter admissibility test. +/// tail keeps its shape; `oom_utilization_hint` and `import_failure_hint` still +/// read the *raw* tail, because what they select from it is quoted into a +/// command and has its own, stricter admissibility test. fn startup_log_context(log_path: Option<&Path>) -> String { let summary = log_path .and_then(|p| summarize_startup_log_tail(p, STARTUP_FAILURE_LOG_TAIL_LINES).ok()) @@ -624,7 +624,11 @@ fn startup_log_context(log_path: Option<&Path>) -> String { if summary.is_empty() { return String::new(); } - let hint = oom_utilization_hint(&summary); + let hint = format!( + "{}{}", + oom_utilization_hint(&summary), + import_failure_hint(&summary) + ); let summary = summary .lines() .map(strip_terminal_control_sequences) @@ -721,6 +725,79 @@ fn log_tail_shows_oom(log_tail: &str) -> bool { lower.contains("out of memory") || lower.contains("outofmemory") } +/// Points at `rocm diagnose` when the startup-log tail shows the engine import +/// failure (`OSError: libtorch_cuda.so: cannot open shared object file`), and +/// prints nothing otherwise. It is the sibling of [`oom_utilization_hint`]: +/// without it, reaching the catalog entry for this failure meant already knowing +/// to paste the error into `rocm diagnose`. +/// +/// The message says only what was detected. The catalog entry carries the +/// conditions under which its remediation applies, so this does not advise a +/// reinstall itself. +/// +/// The quoted symptom follows the same rule as the out-of-memory hint: the +/// user's own line when the checker scores it and it renders as one intact +/// single-quoted argument, the canonical symptom otherwise, so the printed +/// command always reports a cause. +fn import_failure_hint(log_tail: &str) -> String { + let Some(raw_line) = import_failure_line(log_tail) else { + return String::new(); + }; + // The line is subprocess output, so it is echoed only after the terminal + // control bytes the engine's colourised logger emits are removed. + let symptom_line = strip_terminal_control_sequences(raw_line); + let symptom = if quotable_in_single_quotes(raw_line) + && rocm_core::vllm_import_symptom_is_diagnosable(raw_line) + { + raw_line + } else { + rocm_core::VLLM_IMPORT_CANONICAL_SYMPTOM + }; + format!( + "\n\nDetected an engine import failure ({symptom_line}).\n\ + For conditional remediation, run `rocm diagnose --symptom '{symptom}'`." + ) +} + +/// The line of the tail that names the import failure, if there is one. +/// +/// The line that names the missing library is the proximate failure, so it wins +/// and the last one is taken. It must also contain `OSError:` or +/// `ImportError:`. That is a substring match and not a prefix match, because +/// the real line begins with a process prefix such as `(EngineCore pid=412) `. +/// A non-fatal warning that quotes the exception in its repr form, such as +/// `Failed to import from vllm._C with ImportError('libtorch_cuda.so ...')`, +/// names the library too, but it is not the failure, and it is excluded. A +/// warning written with a colon is not excluded. The traceback frame of the +/// extension's loader (`File "...", line N` naming `_optional_torch_c_dlpack`) +/// is the fallback. +/// `torch_c_dlpack_ext` alone is deliberately not a signature: it also appears +/// in healthy package listings. A bare mention of the shim module is not one +/// either, because a `warnings.warn` from that module prints +/// `.../_optional_torch_c_dlpack.py:NN: UserWarning: ...` on a healthy start. +/// A hint on an unrelated failure is worse than none. +fn import_failure_line(log_tail: &str) -> Option<&str> { + let lines = || { + log_tail + .lines() + .rev() + .map(str::trim) + .filter(|line| !line.is_empty()) + }; + lines() + .find(|line| { + let lower = line.to_ascii_lowercase(); + lower.contains("libtorch_cuda.so") + && (lower.contains("oserror:") || lower.contains("importerror:")) + }) + .or_else(|| { + lines().find(|line| { + let lower = line.to_ascii_lowercase(); + lower.contains("file \"") && lower.contains("_optional_torch_c_dlpack") + }) + }) +} + /// Polls the vLLM endpoint until it reports the model is loaded, or times out. /// Uses a monotonic clock (`Instant`) so wall-clock adjustments cannot corrupt the /// timeout, and surfaces an early process exit immediately instead of waiting out @@ -1301,6 +1378,241 @@ mod tests { ); } + // ── The engine import failure: a pointer at `rocm diagnose` ────────────── + + /// A startup-log tail as the engine writes it when the extension loads its + /// CUDA variant on a ROCm build of torch. + const IMPORT_TRACEBACK: &str = "\ +(EngineCore pid=412) Traceback (most recent call last): +(EngineCore pid=412) File \"/opt/venv/lib/python3.12/site-packages/tvm_ffi/_optional_torch_c_dlpack.py\", line 98, in load_torch_c_dlpack_extension +(EngineCore pid=412) lib = ctypes.CDLL(str(lib_path)) +(EngineCore pid=412) OSError: libtorch_cuda.so: cannot open shared object file: No such file or directory"; + + #[test] + fn import_failure_hint_names_the_failing_line_and_the_diagnose_command() { + let hint = import_failure_hint(IMPORT_TRACEBACK); + let line = "(EngineCore pid=412) OSError: libtorch_cuda.so: cannot open shared object file: No such file or directory"; + assert_eq!( + hint, + format!( + "\n\nDetected an engine import failure ({line}).\n\ + For conditional remediation, run `rocm diagnose --symptom '{line}'`." + ), + "the line that names the missing library is the one to quote, verbatim" + ); + assert!( + rocm_core::vllm_import_symptom_is_diagnosable(quoted_symptom_argument(&hint)), + "the command the hint prints must report a cause: {hint}" + ); + } + + #[test] + fn import_failure_hint_is_part_of_the_startup_failure_message() { + let dir = tempfile::tempdir().expect("tempdir"); + let log = dir.path().join("vllm.log"); + std::fs::write(&log, IMPORT_TRACEBACK).expect("write log"); + let context = startup_log_context(Some(&log)); + assert!( + context.contains("Last 80 lines of startup log:"), + "{context}" + ); + assert!( + context.contains( + "rocm diagnose --symptom '(EngineCore pid=412) OSError: libtorch_cuda.so" + ), + "the user must be pointed at the diagnosis from the failure message: {context}" + ); + assert!( + !context.contains("--gpu-memory-utilization"), + "an import failure must not carry the memory hint: {context}" + ); + } + + #[test] + fn every_emitted_import_symptom_is_diagnosable_and_names_the_branch_that_produced_it() { + // The same loop-closing check as the out-of-memory sweep, and for the + // same reason: the engine prints `rocm diagnose --symptom ''`, + // so whatever it emits has to score. Each case pins the exact symptom, so + // the routing branch and the fallback branch are separately falsifiable + // (a mutant that always returns the canonical symptom would otherwise + // satisfy every diagnosability assertion). + let cases = [ + // The library line scores on its own and is quoted verbatim. (Not the + // canonical string itself, which would make the two branches + // indistinguishable here.) + ( + "ImportError: libtorch_cuda.so: cannot open shared object file", + "ImportError: libtorch_cuda.so: cannot open shared object file", + ), + ( + "(EngineCore pid=1) OSError: libtorch_cuda.so: cannot open shared object file: No such file or directory", + "(EngineCore pid=1) OSError: libtorch_cuda.so: cannot open shared object file: No such file or directory", + ), + // A frame that names only the shim's module is worth 35, below the + // threshold, so it falls back. (The same frame with the loader + // function's name adds a second signal and scores on its own.) + ( + "File \"tvm_ffi/_optional_torch_c_dlpack.py\", line 98, in ", + rocm_core::VLLM_IMPORT_CANONICAL_SYMPTOM, + ), + // An apostrophe cannot be quoted safely, so it falls back too. + ( + "OSError: libtorch_cuda.so: can't open shared object file", + rocm_core::VLLM_IMPORT_CANONICAL_SYMPTOM, + ), + ]; + for (line, expected) in cases { + let hint = import_failure_hint(line); + let symptom = quoted_symptom_argument(&hint); + assert_eq!( + symptom, expected, + "wrong branch for {line:?}: quote the user's own line when the checker scores \ + it and it is safe to quote, and fall back to the canonical symptom otherwise" + ); + assert!( + rocm_core::vllm_import_symptom_is_diagnosable(symptom), + "emitted symptom must be diagnosable, got {symptom:?} for line {line:?}" + ); + } + } + + #[test] + fn control_bytes_from_the_log_are_stripped_from_the_echoed_import_line() { + let log_tail = + "\u{1b}[31mOSError: libtorch_cuda.so: cannot open shared object file\u{1b}[0m\u{7}"; + let hint = import_failure_hint(log_tail); + assert!( + !hint.chars().any(|c| c.is_control() && c != '\n'), + "no control byte may survive into the printed hint: {hint:?}" + ); + assert_eq!( + hint, + format!( + "\n\nDetected an engine import failure (OSError: libtorch_cuda.so: cannot open shared object file).\n\ + For conditional remediation, run `rocm diagnose --symptom '{}'`.", + rocm_core::VLLM_IMPORT_CANONICAL_SYMPTOM + ), + "the colourised line is not quotable, so the command falls back while the sentence \ + keeps the user's own line, stripped" + ); + } + + #[test] + fn import_failure_hint_stays_quiet_for_unrelated_failures() { + for unrelated in [ + "ValueError: model architecture 'FooForCausalLM' is not supported", + "torch.OutOfMemoryError: HIP out of memory", + // An `OSError:` or `ImportError:` line that does not name the + // library is not this failure. Without the library-name condition + // in the primary match, every such line would raise the hint. + "(EngineCore pid=412) OSError: [Errno 98] Address already in use", + "ImportError: cannot import name 'FooForCausalLM' from 'vllm.model_executor'", + // The extension named in a healthy package listing is not a failure. + "INFO torch-c-dlpack-ext 0.2.1 installed; torch_c_dlpack_ext loaded", + // A `warnings.warn` from the shim module on a healthy start names + // the module on a line of its own. It is not a traceback frame. + "/opt/venv/lib/python3.12/site-packages/tvm_ffi/_optional_torch_c_dlpack.py:98: UserWarning: falling back to the JIT path", + "(EngineCore pid=412) /opt/venv/lib/python3.12/site-packages/tvm_ffi/_optional_torch_c_dlpack.py:98: UserWarning: falling back to the JIT path", + "", + ] { + assert!( + import_failure_hint(unrelated).is_empty(), + "must stay quiet for {unrelated:?}" + ); + } + } + + #[test] + fn import_failure_hint_stays_quiet_for_an_unrelated_traceback() { + // A traceback frame that does not name the shim module is not an import + // failure. Without the `_optional_torch_c_dlpack` condition in the + // fallback, any `File "` frame in the tail would match. + let tail = "\ +(EngineCore pid=412) Traceback (most recent call last): +(EngineCore pid=412) File \"/opt/venv/lib/python3.12/site-packages/vllm/engine/llm_engine.py\", line 12, in +(EngineCore pid=412) validate(config) +(EngineCore pid=412) ValueError: model architecture 'FooForCausalLM' is not supported"; + assert!(import_failure_hint(tail).is_empty()); + assert!(startup_log_context_for(tail).contains("ValueError")); + assert!(!startup_log_context_for(tail).contains("rocm diagnose")); + } + + #[test] + fn a_non_fatal_warning_that_names_the_library_is_not_an_import_failure() { + // Pinned: the primary match needs `OSError:` or `ImportError:` in the + // line, so a warning that quotes the exception in its repr form as + // `ImportError('...')` does not raise the hint when the real failure is + // something else. A warning written with a colon would, which is a + // known limit that `docs/vllm.md` states. + let tail = "\ +WARNING Failed to import from vllm._C with ImportError('libtorch_cuda.so: cannot open shared object file') +(EngineCore pid=1) ValueError: model architecture 'FooForCausalLM' is not supported"; + assert!(import_failure_hint(tail).is_empty()); + } + + /// Runs `startup_log_context` over a log file holding `tail`. + fn startup_log_context_for(tail: &str) -> String { + let dir = tempfile::tempdir().expect("tempdir"); + let log = dir.path().join("vllm.log"); + std::fs::write(&log, tail).expect("write log"); + startup_log_context(Some(&log)) + } + + #[test] + fn startup_log_context_carries_the_memory_hint_for_an_out_of_memory_tail() { + let context = startup_log_context_for("torch.OutOfMemoryError: HIP out of memory"); + assert!( + context.contains("--gpu-memory-utilization"), + "an out-of-memory failure must carry the memory hint: {context}" + ); + assert!( + !context.contains("Detected an engine import failure"), + "{context}" + ); + } + + #[test] + fn startup_log_context_carries_both_hints_in_order_when_both_fire() { + let tail = format!("torch.OutOfMemoryError: HIP out of memory\n{IMPORT_TRACEBACK}"); + let context = startup_log_context_for(&tail); + let memory = context.find("--gpu-memory-utilization"); + let import = context.find("Detected an engine import failure"); + assert!( + memory.is_some() && import.is_some(), + "both paragraphs must be present: {context}" + ); + assert!( + memory < import, + "the memory paragraph comes first, then the import paragraph: {context}" + ); + } + + #[test] + fn import_failure_hint_quotes_the_last_line_that_names_the_library() { + // Two lines name the library, as when a failure is retried or chained. + // The later one is the proximate failure, so it is the one quoted. + let tail = "\ +(EngineCore pid=1) OSError: libtorch_cuda.so: cannot open shared object file: first +(EngineCore pid=1) During handling of the above exception, another exception occurred: +(EngineCore pid=1) OSError: libtorch_cuda.so: cannot open shared object file: second"; + let hint = import_failure_hint(tail); + assert!(hint.contains(": second"), "{hint}"); + assert!(!hint.contains(": first"), "{hint}"); + } + + #[test] + fn a_loader_traceback_frame_is_the_fallback_when_no_line_names_the_library() { + let frame = "File \"/opt/venv/lib/python3.12/site-packages/tvm_ffi/_optional_torch_c_dlpack.py\", line 98, in "; + let hint = import_failure_hint(frame); + assert!(hint.contains("Detected an engine import failure"), "{hint}"); + // The frame alone is below the threshold, so the command quotes the + // canonical symptom rather than a line that would find nothing. + assert_eq!( + quoted_symptom_argument(&hint), + rocm_core::VLLM_IMPORT_CANONICAL_SYMPTOM + ); + } + #[test] fn vllm_serve_args_omit_gpu_memory_utilization_without_override() { let args = vllm_serve_args("facebook/opt-125m", "127.0.0.1", 8000, false, None); diff --git a/skills/rocm-doctor/reference.md b/skills/rocm-doctor/reference.md index ac22f795e..f8130fc09 100644 --- a/skills/rocm-doctor/reference.md +++ b/skills/rocm-doctor/reference.md @@ -126,7 +126,7 @@ reporting confident nonsense. | `fix-14-adrenalin-too-old` | windows | Adrenalin / kernel-mode driver too old for the HIP SDK | `hipInfo` can't enumerate, "driver too old", HSA "no agents found" | print-only | | `fix-15-msvc-redist` | windows | MSVC runtime missing (programs that link it, such as the HIP SDK DLLs and the llama.cpp server, can't load) | `vcruntime140.dll` / `vcruntime140_1.dll` missing | print-only | | `fix-16-vllm-oom` | linux/wsl | vLLM reserved ~90% of the GPU's TOTAL VRAM for its KV cache and ran out at startup | vLLM engine start fails with an out-of-memory error; the GPU is shared/busy, or the model does not fit | print-only | -| `fix-17-torch-dlpack` | linux | `torch-c-dlpack-ext` loads its CUDA prebuilt on a ROCm torch, aborting vLLM's engine start at import time | vLLM engine start fails on import; error names `torch_c_dlpack_ext` or tvm_ffi's `_optional_torch_c_dlpack` | print-only | +| `fix-17-torch-dlpack` | linux/wsl | `torch-c-dlpack-ext` loads its CUDA prebuilt on a ROCm torch, aborting vLLM's engine start at import time | vLLM engine start fails on import; error names `libtorch_cuda.so` (enough on its own), or both `torch_c_dlpack_ext` and tvm_ffi's `_optional_torch_c_dlpack` | print-only | | `fix-18-comgr-conflict` | linux/wsl | The code object manager library (`libamd_comgr`) that would load belongs to a different installation than the HIP runtime that would load, so device code compilation fails with an error naming neither | compilation error naming neither library; `rocm examine --json`'s `comgr_selected`/`hip_selected` resolve to two different `install_root`s | print-only | | `fix-19-shm-too-small` | linux/wsl | `/dev/shm` too small for a serving workload, which needs gigabytes where a container and WSL2 both default to 64 MB | reported under 1 GiB; a data-loader worker killed by a bus error, or a failed write to a temporary file, with nothing naming shared memory | print-only | | `info-20-gpu-no-rocm-support` | linux/windows | Every AMD GPU on the host is a part ROCm has never shipped a runtime build for (today: gfx90c, the Vega iGPU in Renoir / Lucienne / Cezanne / Barcelo). Vulkan-based engines still work | every detected AMD GPU identifies as an unsupported target, and none is unidentified; scores a flat 60 | diagnose-only | @@ -148,10 +148,10 @@ the number `fix-20` is simply not used here: slot 20 belongs to prefix because it has no fix to offer. A gap is not an error, and a retired entry would leave a hole rather than renumber the ones after it. -Linux-only: fix-3, -4, -5, -7, -10, -11, -12, -17. Windows-only: fix-13, -14, -15, -21. +Linux-only: fix-3, -4, -5, -7, -10, -11, -12. Windows-only: fix-13, -14, -15, -21. Linux + Windows: info-20 (diagnose-only). WSL-only: fix-wsl-1 through fix-wsl-7. Linux + Windows: fix-9. -Linux + WSL: fix-16, -18, -19. Linux + Windows + WSL: fix-1, -2, -6, -8. +Linux + WSL: fix-16, -17, -18, -19. Linux + Windows + WSL: fix-1, -2, -6, -8. ## Framework routing diff --git a/tests/e2e-cucumber/features/diagnose.feature b/tests/e2e-cucumber/features/diagnose.feature index b5eb86f11..429c76b2e 100644 --- a/tests/e2e-cucumber/features/diagnose.feature +++ b/tests/e2e-cucumber/features/diagnose.feature @@ -169,14 +169,18 @@ Feature: Diagnosing failures and listing fixes # a test of the runner's health. Clearing the threshold comes from the keyword # alone and holds on every host. # - # @requires-os:linux because the checker is registered linux-only, and - # @requires-bare-metal because WSL2 does not run the catalog at all — the two - # are not interchangeable, WSL2 reports an os_family of linux. - @id:diagnose-recognises-the-engine-import-failure @requires-bare-metal @requires-os:linux + # @requires-os:linux because the vLLM engine does not run on native Windows. + # The checker is registered for linux and wsl, and it reads only the symptom + # text, so the scenario holds on the WSL lane too. It is not tagged + # @requires-bare-metal, which would skip it exactly where the WSL entry was + # added. Following the diagnosis to the fix it named proves the recipe applies + # on the host the report was produced on. + @id:diagnose-recognises-the-engine-import-failure @requires-os:linux Scenario: diagnose-13 - A vLLM engine-startup import failure is recognised from its error text Given a user who hit the vLLM engine-startup import failure When the user asks the CLI to diagnose that symptom in machine-readable form Then the CLI reports the engine-startup import failure as an established cause + And the CLI can act on the fix named for the engine-startup import failure # Every other recipe in the catalog is a flat sequence for one shell. This one # is not: `rocm engines shell vllm` opens an INTERACTIVE subshell, the two @@ -187,11 +191,12 @@ Feature: Diagnosing failures and listing fixes # terminal stdin buffering to land each line in the right shell. What the user # can observe is the printed plan, so that is what is asserted. # - # Deliberately not OS-gated even though the fix is linux-only: the plan is - # printed before the fix's own platform gate is reached, so the text under test - # is identical on both lanes and the Windows lane exercises it too. The step - # therefore asserts the printed block and not the exit code, which does differ - # (0 where the fix applies, 3 where it does not). + # Deliberately not OS-gated even though the fix applies on Linux and WSL only: + # the plan is printed before the fix's own platform gate is reached, so the + # text under test is identical on every lane and the Windows lane exercises it + # too. The step therefore asserts the printed block and not the exit code, + # which does differ (0 on Linux and WSL, where the fix applies, and 3 on + # Windows, where it does not). @id:diagnose-fix-says-which-shell-each-step-runs-in Scenario: diagnose-14 - A fix whose steps span two shells says which shell each step runs in Given a user who has chosen the fix for the engine-startup import failure diff --git a/tests/e2e-cucumber/tests/e2e/diagnose_steps.rs b/tests/e2e-cucumber/tests/e2e/diagnose_steps.rs index 50a0c68c4..eee1bac41 100644 --- a/tests/e2e-cucumber/tests/e2e/diagnose_steps.rs +++ b/tests/e2e-cucumber/tests/e2e/diagnose_steps.rs @@ -719,15 +719,25 @@ async fn assert_json_identifies_match(world: &mut E2eWorld) { /// panicking with the raw output if it is absent. Shared by the two `Then` /// steps that assert on the vLLM OOM entry. fn find_vllm_oom<'a>(report: &'a serde_json::Value, output: &str) -> &'a serde_json::Value { + find_matched(report, "fix-16-vllm-oom", output) +} + +/// The entry of the diagnosis's `matched` list with the given id, panicking with +/// the raw output if it is absent. +fn find_matched<'a>( + report: &'a serde_json::Value, + id: &str, + output: &str, +) -> &'a serde_json::Value { report .get("matched") .and_then(serde_json::Value::as_array) .and_then(|matches| { matches.iter().find(|diagnosis| { - diagnosis.get("id").and_then(serde_json::Value::as_str) == Some("fix-16-vllm-oom") + diagnosis.get("id").and_then(serde_json::Value::as_str) == Some(id) }) }) - .unwrap_or_else(|| panic!("expected fix-16-vllm-oom in diagnosis:\n{output}")) + .unwrap_or_else(|| panic!("expected {id} in diagnosis:\n{output}")) } #[then("no vLLM startup OOM is reported")] @@ -791,12 +801,30 @@ async fn assert_named_fix_is_runnable_here(world: &mut E2eWorld) { .and_then(serde_json::Value::as_str) .unwrap_or_else(|| panic!("the matched cause must name a fix id:\n{output}")) .to_owned(); - let (fix_output, fix_stderr, rc) = crate::run_rocm(world, &["fix", &fix_id]); + assert_fix_is_printable_here(world, &fix_id); +} + +#[then("the CLI can act on the fix named for the engine-startup import failure")] +async fn assert_import_failure_fix_is_runnable_here(world: &mut E2eWorld) { + let (report, output) = parsed_diagnosis(world); + let fix_id = find_matched(&report, ENGINE_IMPORT_FIX_ID, &output) + .pointer("/fix/fix_id") + .and_then(serde_json::Value::as_str) + .unwrap_or_else(|| panic!("the matched cause must name a fix id:\n{output}")) + .to_owned(); + assert_fix_is_printable_here(world, &fix_id); +} + +/// Runs `rocm fix ` and checks that it prints its advisory plan rather +/// than refusing on this platform. Shared by the steps that follow a diagnosis +/// to the fix it named. +fn assert_fix_is_printable_here(world: &mut E2eWorld, fix_id: &str) { + let (fix_output, fix_stderr, rc) = crate::run_rocm(world, &["fix", fix_id]); assert_eq!( rc, 0, "{}", - e2e_cucumber::cli_failure_report(&["fix", &fix_id], rc, &fix_output, &fix_stderr) + e2e_cucumber::cli_failure_report(&["fix", fix_id], rc, &fix_output, &fix_stderr) ); assert!( fix_output.to_lowercase().contains("print-only"),