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"),