From 6450959ff8ed5f6dea808e5b94255eacc4f4d1e7 Mon Sep 17 00:00:00 2001 From: Eugene Volen Date: Tue, 15 Sep 2026 12:11:29 +0000 Subject: [PATCH] feat(diagnose): answer whether a model will run before downloading it `rocm diagnose --model ` composes a curated recipe with this host's GPU memory and the engine `serve` would select, and reports ready, degraded, blocked or undetermined. It fetches nothing, so the answer costs seconds instead of a failed download. Undetermined is kept distinct from blocked deliberately. A recipe catalog that could not be read, a GPU whose memory could not be measured, and a model the catalog does not carry all say nothing about whether the model fits; reporting any of them as an incompatibility produces a wrong answer that reads like a real one. An APU is judged on the memory its engine allocates from rather than the BIOS carve-out amd-smi reports, for the same reason. A blocked verdict names curated models that would run here instead, and each candidate is put through the same assessment against the same host, so a user is never pointed at a second model they also cannot run. Engine selection is passed in from `select_serve_engine` rather than re-derived, so the two commands cannot come to disagree about which engine would serve a model. The alternative-selection policy moves into rocm-core and is shared with `rocm model --verbose`, which keeps its own fit predicate and its current output. Review fixes: - The engine consistency guarantee had a gap: the doctor path derived the configured engine from its own `AppPaths::discover()`, separate from the one behind the GPU summary, so a config write racing a lookup could name a different engine than `rocm serve` would pick for the same model. `assess_model_on_this_host` now threads a single discovered `AppPaths` to both the GPU summary and the config lookup, and a regression test drives both call sites and compares. - WSL GPU detection always reported `has_amd_gpu: false`, which fed a false "no usable GPU" into every verdict on a WSL host. Fixed. - A GPU reporting zero VRAM total was accepted as a valid measurement instead of being treated as "could not measure", which could produce a confident-looking blocked verdict from a bad reading. Fixed. - Added end-to-end coverage for the two verdicts the existing scenarios did not reach: a model outside the curated catalog (undetermined, not blocked) and a model whose system RAM recommendation exceeds the host's (degraded, not blocked), the latter via a synthetic signed catalog since no built-in recipe can produce it against an arbitrary real host. - `/model`'s missing-VRAM-reading message no longer points at `/examine`, which is a static snapshot with no VRAM figure in it; it now names `amd-smi metric --json` directly, matching what `diagnose --model` already names for the same gap. - Documented `--model` in the README's exhaustive flag listing and added a Model Fit Preflight section to docs/testing.md. - The APU fallback fabricated `UnifiedSystemMemory` from total installed RAM, which is not the pool the engine allocates from -- the real GTT aperture is capped below installed RAM by BIOS/kernel policy, and this binary has no way to read it. Asserting installed RAM in its place could produce a false `Ready` for a model that does not actually fit. The classification is now a pure, unit-tested function separated from the `amd-smi` subprocess call, and the APU case reports `Unknown` instead of a fabricated figure. The degraded-vs-blocked e2e fixture also named `vllm` as its preferred engine, which the platform gate rules out on native Windows ahead of the RAM-softening logic the scenario exists to exercise, turning the expected `degraded` into `blocked` on that lane; swapped to `lemonade`, which no lane this suite runs excludes. - The APU fix above still left two gaps. First, reporting plain `Unknown` for an APU sent it down the same remediation as a genuine telemetry gap ("run `amd-smi metric --json`"), which is a dead end for an APU: amd-smi already ran and answered, it just named the BIOS carve-out instead of the pool the engine allocates from, and running it again produces the same wrong figure forever. `AcceleratorMemory` gains a `UnifiedMemoryUnreadable` variant and `UndeterminedReason` a matching case, so the APU path now says plainly that there is no dedicated VRAM and no command that makes the real pool readable today, with no fix offered -- and stays distinct from the genuine no-telemetry case, which keeps its real amd-smi remediation. Second, the now-dead `UnifiedSystemMemory` variant was still documented and still had a test pinning it; both are removed. Third, the platform-gate `unsupported_here` message had no test coverage on any single platform -- deleting the closure body left every test green -- so its logic is now a pure `unsupported_here_for(engine, ruled_out)` helper, unit-tested on both the ruled-out and allowed branches directly rather than depending on which OS the test happens to run on. - The `UnifiedMemoryUnreadable` reason above reached rocm-core, the classifier and the unit tests, but not the e2e scenario layer: the "told why" step still asserted the single reason string that existed before it, so an APU host now fails that assertion on output that is correct. The step's undetermined arm is now reason-specific instead of a single `assert_eq!`: the no-telemetry-at-all reason must say the memory could not be read and carry a fix, the APU reason must name the missing dedicated-VRAM pool and carry no fix, and any other reason string panics rather than passing by coincidence -- so a future regression that collapses the two back together fails this step instead of going unnoticed. Two stale comments this variant made false are corrected to name the APU case as a third machine class alongside the two they used to enumerate. - The same defect shape survived one branch over: the blocked arm of the same step still hard-asserted a single evidence string ("no GPU is visible to ROCm"), on the premise that the only way a host with no measurement reaches a blocked verdict is an absent GPU. The engine platform gate runs before the memory gate and returns blocked early with its own evidence, naming an engine with no adapter on this platform -- a vllm-only recipe on native Windows reaches this arm with that evidence instead, and the premise no longer held. The blocked arm is now reason-specific the same way the undetermined arm already is: the two causes are discriminated by `fix.summary`, written independently at the two call sites that set it, so a collapse in one cause's evidence text cannot borrow the other's identity and still pass; each cause is then asserted on its own terms -- the engine-platform refusal must name the ruled-out engine and offer the other-platform remedy, the GPU-visibility refusal must name the absent GPU and still carry its fix -- with a closed `other => panic!` fallback for a cause this step does not know about yet. The scenario's own comment, which still enumerated only two halves, now names all three unmeasured sub-causes. This is the change that explains the `E2E tests (Strix Halo, Windows)` regression on this PR: the failing job's own log carries the pre-fix panic text verbatim against a vllm/native-Windows payload naming the engine-platform cause, so it is not a flake. - A fresh review round after the rebase onto main caught a real correctness bug: `classify_accelerator_memory` summed VRAM across every GPU rather than comparing against the single ordinal `rocm serve` actually pins, so a homogeneous multi-GPU host (e.g. 8x192 GiB) could report READY for a model that OOMs on every individual card. It now narrows to the mask-visible rows (`rocm_core::usable_amd_gpu_indices`) and takes the largest one -- the best case `--gpu auto` could land on. Two new tests prove it: one against an 8x192 GiB host (would have reported ~1536 GiB summed; now reports 192 GiB) and one proving a masked-out card's capacity is ignored. - The rewritten `/model` missing-VRAM-reading remediation (`amd-smi metric --json`) was itself a dead end: every production caller of this rendering path hardcodes `aggregate_gpu_vram_gib = None`, so running amd-smi changes nothing this function ever sees. It now points at `rocm diagnose --model ` instead, the command this PR adds that does thread a live reading through. - `--model` with `--distro` was refused only when the probe happened to come back looking remote (`inspected_remotely`, derived from `examination.wsl`), not on whether `--distro` was actually passed -- a correct-by-coincidence check with no test. It is now keyed on the flag directly, checked before any probe runs, which also means the refusal no longer needs a reachable WSL distribution to exercise: added `diagnose-26` (feature + step defs), which runs on every lane. - `format_gib`'s fractional branch rounded to nearest, so a card that reports a hair under its nameplate size (e.g. 8176 MiB = 7.9844 GiB) could print "8.0 GiB" against an "8 GiB" recipe minimum on a BLOCKED verdict -- evidence that reads as an exact match while the verdict says otherwise. It now floors instead; `required`/`recommended` values are always exact integers so they are unaffected. - The verdict table's `UNDETERMINED` row was missing its fourth reason (`UnifiedMemoryUnreadable`, the APU case); the PR description now lists all four. - Corrected a stale off-by-one in two scenario doc comments (`diagnose-20`/`diagnose-21` meant `diagnose-21`/`diagnose-22`) and added a `docs/architecture.md` mention for the new `model_readiness.rs` module. - `rocm_doctor_skill.feature`'s field-sync check (new on main since this branch was cut) caught a second merged-tree-only gap: `skills/rocm-doctor/ reference.md` never names the new `model` field `diagnose --json` now emits, so an agent following the skill would not read it. Documented it there, alongside the new `--distro`/`--model` flags in the command synopsis. - The multi-GPU fix's own tests proved `classify_accelerator_memory` alone, not that picking the wrong figure would have changed a verdict a user sees. Added `a_host_whose_vram_sum_clears_the_minimum_but_no_single_card_does_is_blocked_not_ready`, which drives the real pipeline end to end: two 8 GiB cards (16 GiB summed, comfortably above qwen3.5-4b's 12 GiB minimum) against the real catalog, asserting the verdict is `Blocked`. Confirmed non-vacuous by temporarily restoring the sum and re-running it: the verdict reads `Ready` with the sum, `Blocked` with the fix. The remote doctor's test fixture builds a DiagnoseReport literal, so it names the new field too. That file arrived on main after this branch was cut, which is why the break appeared only in the merged tree: neither side is wrong alone, and no local build on either branch can see it. Signed-off-by: Eugene Volen --- README.md | 23 + apps/rocm/src/main.rs | 830 ++++++++++- apps/rocm/src/remote/doctor.rs | 5 + crates/rocm-core/src/diagnose.rs | 30 +- crates/rocm-core/src/lib.rs | 3 +- crates/rocm-core/src/model_readiness.rs | 1218 +++++++++++++++++ docs/architecture.md | 2 +- docs/testing.md | 27 + skills/rocm-doctor/reference.md | 11 +- tests/e2e-cucumber/features/diagnose.feature | 92 ++ .../e2e-cucumber/tests/e2e/diagnose_steps.rs | 673 +++++++++ 11 files changed, 2876 insertions(+), 38 deletions(-) create mode 100644 crates/rocm-core/src/model_readiness.rs diff --git a/README.md b/README.md index 8249dc1b5..55ccb70cb 100644 --- a/README.md +++ b/README.md @@ -227,6 +227,7 @@ form works depends on the engine your GPU selects. | `rocm` | Open the launcher menu (setup, serve, diagnose, chat, dashboard) | | `rocm examine` | Check GPU, ROCm install, engines, and managed folders | | `rocm diagnose` | Match this machine against known ROCm/PyTorch/llama.cpp failure modes | +| `rocm diagnose --model ` | Say whether a model will run here, before downloading it | | `rocm fix []` | Apply a fix reported by `rocm diagnose` | | `rocm install sdk` | Install TheRock ROCm wheels into a managed Python environment | | `rocm install driver` | Install the AMD kernel driver on Linux | @@ -261,6 +262,7 @@ the JSON report, not the human-readable one. ``` rocm diagnose [--symptom TEXT] [--top N] [--json] [--distro [NAME]] +rocm diagnose --model [--json] rocm fix [] [--yes] [--dry-run] [--device-index N] ``` @@ -452,6 +454,27 @@ rocm engines shell [--runtime-id KEY | --env-id ID] [--shell PATH] Supported engines: `lemonade`, `vllm`. +### Will a model run here? + +Ask before downloading anything: + +``` +rocm diagnose --model [--json] +``` + +Answers in seconds, from the curated recipe and this machine's GPU — it +fetches no weights and makes no network call. The verdict is `ready`, +`degraded`, `blocked`, or `undetermined`. A `ready` answer also names the +engine `rocm serve` would use; a `blocked` one names curated models that would +run here instead. + +`undetermined` is a real answer and not a failure: it is what you get when the +recipe catalog could not be read, when this machine's GPU memory could not be +measured, or when the model is not one of the curated recipes (`rocm model` +lists those). None of those say anything about whether the model fits, so none +of them are reported as though they did — `rocm serve` still accepts a model +outside the catalog, this just cannot tell you in advance how it will go. + ### Model serving Start a local OpenAI-compatible model server: diff --git a/apps/rocm/src/main.rs b/apps/rocm/src/main.rs index 1f0655acc..078d5c3bc 100644 --- a/apps/rocm/src/main.rs +++ b/apps/rocm/src/main.rs @@ -28,6 +28,9 @@ use crate::uninstall::uninstall; use anyhow::{Context, Result, bail}; use clap::{CommandFactory, FromArgMatches, Parser, Subcommand, ValueEnum}; +use rocm_core::model_readiness::{ + AcceleratorMemory, HostEngineChoice, HostFacts, ModelCatalogSource, ModelReadiness, +}; use rocm_core::{ AppPaths, AuditEventRecord, AutomationEventRecord, AutomationProposalRecord, AutomationRuntimeState, CodexBridgeEngine, CodexBridgeGpuSnapshot, CodexBridgeSnapshot, @@ -153,6 +156,19 @@ enum Command { /// name only when more than one is installed. #[arg(long, value_name = "NAME", num_args = 0..=1, default_missing_value = "")] distro: Option, + /// Also answer whether a model would run on this machine, before + /// downloading it. + /// + /// Takes a curated model name or alias (see `rocm model`). Nothing is + /// fetched: the answer comes from the recipe and this host's GPU. A + /// model the catalog does not carry is reported as undetermined rather + /// than blocked — `rocm serve` still accepts it, this just cannot say + /// in advance whether it fits. + /// + /// A flag rather than a positional: `diagnose` takes no positional + /// today, and one added now would be ambiguous against `--symptom`. + #[arg(long, value_name = "MODEL")] + model: Option, }, /// Apply a known fix by id (see `rocm diagnose`); run with no id to list fixes. /// @@ -2077,7 +2093,8 @@ fn dispatch(cli: Cli) -> Result<()> { top, json, distro, - }) => diagnose(symptom, top, json, distro), + model, + }) => diagnose(symptom, top, json, distro, model), // Keep this error chained rather than discarding it into a fresh // `anyhow!(...)` (e.g. via a `.map_err` that restringifies it) -- see // `FixExitCode`'s doc comment for why that would silently break its @@ -2748,7 +2765,34 @@ fn examine(json: bool, framework: rocm_core::FrameworkProbe) -> Result<()> { Ok(()) } -fn diagnose(symptom: Option, top: usize, json: bool, distro: Option) -> Result<()> { +fn diagnose( + symptom: Option, + top: usize, + json: bool, + distro: Option, + model: Option, +) -> Result<()> { + // The model verdict is about THIS machine, always. `--distro` retargets the + // environment examination at another one, but the GPU memory and engine + // selection behind a model verdict are read locally -- so answering for a + // model here would describe the wrong host under a heading naming another. + // Refuse rather than substitute, the same way `--distro` itself does when it + // cannot reach a machine. + // + // Keyed on whether `--distro` was passed, not on anything the probe below + // populates. An examination-derived signal (e.g. whether `examination.wsl` + // ended up set, or `locally_probed`) would make this refusal depend on + // probe internals instead of on the flag the user actually typed -- + // exactly the kind of coupling that lets a future change to the probe + // silently let a `--distro` run fall through and answer for the local host + // under a heading naming another machine. + if model.is_some() && distro.is_some() { + bail!( + "--model answers for the machine running this command, and --distro points the \ + examination at a different one. Run `rocm diagnose --model ` inside the \ + distribution instead." + ); + } // `rocm diagnose` is a query: it exits 0 whether it matched, found nothing, // or is out of scope. Callers read `has_match` / `out_of_scope` / // `route_when_no_match` from `--json` rather than branching on the exit code. @@ -2780,11 +2824,21 @@ fn diagnose(symptom: Option, top: usize, json: bool, distro: Option, top: usize, json: bool, distro: Option ModelReadiness { + let paths = AppPaths::discover().ok(); + assess_model_with_host_paths(model_ref, examination, paths.as_ref()) +} + +/// Testable core of [`assess_model_on_this_host`]. +/// +/// Takes the discovered `AppPaths` as an explicit parameter so the GPU-summary +/// lookup and the config lookup read the same directories, instead of each +/// racing its own `AppPaths::discover()` call. That used to be two separate +/// calls: one (implicitly `None`) feeding [`detect_host_gpu_summary`], another +/// feeding the config lookup a few lines later. Both resolve identically today +/// because `AppPaths::discover()` is deterministic within a process, but +/// `detect_host_gpu_summary`'s paths-driven TheRock-manifest gfx-target +/// fallback only ever sees a path when one is passed in -- silently skipping +/// it here is exactly the kind of drift that would let this command name a +/// different engine than `rocm serve` for the same model. +fn assess_model_with_host_paths( + model_ref: &str, + examination: &rocm_core::Examination, + paths: Option<&AppPaths>, +) -> ModelReadiness { + let host_gpu_summary = detect_host_gpu_summary(paths); + let host = HostFacts { + accelerator_memory: host_accelerator_memory(&host_gpu_summary, examination), + system_ram_gib: rocm_core::detect_system_ram_gib(), + }; + let configured_engine = paths + .and_then(|paths| RocmCliConfig::load(paths).ok()) + .and_then(|config| config.default_engine); + match load_model_recipe_registry() { + Ok(registry) => assess_model_for_host( + model_ref, + &ModelCatalogSource::Available(®istry), + &host, + configured_engine.as_deref(), + Some(&host_gpu_summary), + ), + Err(error) => assess_model_for_host( + model_ref, + &ModelCatalogSource::Unreachable { + detail: format!("{error:#}"), + }, + &host, + configured_engine.as_deref(), + Some(&host_gpu_summary), + ), + } +} + +/// The memory pool an engine on this host would allocate a model from. +/// +/// The APU case is why this is not simply a sum of `total_vram`. An APU has no +/// private VRAM; `amd-smi` reports the fixed BIOS carve-out (often ~4 GiB) while +/// the allocator serves the model out of GTT-backed system RAM. Comparing a +/// recipe minimum against the carve-out would refuse a 22 GiB model on a 128 GiB +/// Strix Halo, which serves it fine. [`vram_capacity_is_meaningful`] is the same +/// test `serve`'s low-VRAM warning uses, so the two cannot disagree about which +/// hosts the figure describes. +/// +/// The examination is what makes "there is no GPU" reachable at all. `amd-smi` +/// answers "how much memory", and its absence is ambiguous — no GPU, or no +/// `amd-smi`. The examination reads the devices themselves, so it can tell those +/// two apart, and they deserve different answers: one is a machine that needs a +/// GPU, the other a machine whose GPU could not be measured. +/// Whether the examination has positive or unresolved evidence of an AMD GPU, +/// used to tell "no GPU" apart from "GPU present but VRAM unmeasured" when +/// `amd-smi` produced nothing. +/// +/// `examination.has_amd_gpu` cannot answer this on WSL: the hardware probes +/// that set it are skipped there (see [`rocm_core::Examination::probe_with_interpreter`]), +/// so it reads `false` on every WSL host, healthy or not, and this command +/// would report a working machine as having no GPU at all -- `Blocked` +/// instead of the `Undetermined` a genuinely unmeasurable host deserves. +/// `rocm_sees_gpu`, not `has_amd_gpu`, is the WSL-native signal, per the same +/// reasoning `fix-wsl-6-host-driver-too-old` already uses in `diagnose.rs`. +/// Only `Some(false)` -- rocminfo positively enumerating no device -- counts +/// as "no GPU"; `None` means rocminfo was absent so the question went unasked, +/// which is not evidence of anything and must not read as a confident no. +fn examination_reports_amd_gpu(examination: &rocm_core::Examination) -> bool { + match examination.wsl.as_ref() { + Some(wsl) => wsl.rocm_sees_gpu != Some(false), + None => examination.has_amd_gpu, + } +} + +fn host_accelerator_memory( + host_gpu_summary: &rocm_core::HostGpuSummary, + examination: &rocm_core::Examination, +) -> AcceleratorMemory { + classify_accelerator_memory( + gpu_vram_usage().as_deref(), + host_gpu_summary.gfx_target.as_deref(), + examination_reports_amd_gpu(examination), + rocm_core::usable_amd_gpu_indices().as_deref(), + ) +} + +/// The pure classification behind [`host_accelerator_memory`], split out so it +/// can be exercised with fixed input instead of a live `amd-smi` subprocess. +/// Deciding whether a host is an APU and which figure represents its memory is +/// the actual judgement this command depends on; entangled with the +/// subprocess call it was untestable, and a checker that silently regressed +/// to always reporting `Dedicated` would leave every test green. +fn classify_accelerator_memory( + vram: Option<&[GpuVramUsage]>, + gfx_target: Option<&str>, + gpu_reported: bool, + usable_indices: Option<&[u32]>, +) -> AcceleratorMemory { + let Some(vram) = vram else { + if gpu_reported { + return AcceleratorMemory::Unknown; + } + return AcceleratorMemory::None; + }; + if vram_capacity_is_meaningful(gfx_target, vram.len()) { + // `rocm serve` pins exactly one ordinal -- there is no tensor-parallel + // path anywhere in this binary -- so summing every card's VRAM into one + // figure can report a model READY when it would OOM on every individual + // card. Narrow to the mask-visible rows (a `HIP_VISIBLE_DEVICES` mask + // hides devices the same way it does for `rocm serve` itself) and take + // the most capable one: that is the best case `--gpu auto` could + // actually land on, so it is the right upper bound for "would this + // fit", even though it cannot name which ordinal the verdict answers + // for. `usable_indices` of `None` means the mask could not be probed, + // so every reported row is treated as visible rather than refusing to + // answer. + let max_gib = vram + .iter() + .filter(|usage| usable_indices.is_none_or(|indices| indices.contains(&usage.index))) + .map(|usage| usage.total_mb as f64 / 1024.0) + .fold(0.0_f64, f64::max); + return AcceleratorMemory::Dedicated(max_gib); + } + // This is an APU: the carve-out `amd-smi` reports above is the wrong pool, + // per `vram_capacity_is_meaningful`'s own doc. The right pool is the GTT + // aperture, and nothing in this binary can read it -- `amd-smi` does not + // report it, and no code path here calls `rocm-smi`, the tool that does. + // Total system RAM is not a substitute: the GTT aperture is capped well + // below installed RAM by BIOS/kernel policy, and asserting installed RAM + // as the engine's allocation pool trades the carve-out's under-estimate + // for an over-estimate in exactly the direction that produces a false + // `Ready` for a model that does not actually fit -- the wrong answer this + // command exists to prevent. This is deliberately not `Unknown`: that + // variant means no telemetry at all, for which "run `amd-smi metric + // --json`" is a real next step. Here `amd-smi` already ran and answered -- + // it just named the wrong pool -- so the same remediation would send a + // Strix Halo user in a circle forever. Until the real aperture can be + // read, this reports `UnifiedMemoryUnreadable`, so the verdict reads + // `Undetermined` with no dead-end command attached, rather than a + // confident wrong one. + AcceleratorMemory::UnifiedMemoryUnreadable +} + +/// Assess a model against this host, with `serve`'s own engine decision. +/// +/// The engine is not re-derived here. `select_serve_engine` is the one place +/// that answers "which engine serves this model on this host", and passing it in +/// is what keeps `rocm diagnose --model` from confidently naming an engine +/// `rocm serve` would never pick. An engine ruled out by the platform gate is +/// reported as such rather than silently swapped for another: `serve` does not +/// fall back either, and a verdict that pretended otherwise would be wrong in +/// the user's favour. +fn assess_model_for_host( + model_ref: &str, + catalog: &ModelCatalogSource<'_>, + host: &HostFacts, + configured_default_engine: Option<&str>, + host_gpu_summary: Option<&rocm_core::HostGpuSummary>, +) -> ModelReadiness { + let engine_for = |recipe: &ModelRecipeRecord| { + let selection = select_serve_engine( + None, + configured_default_engine, + Some(recipe), + host_gpu_summary, + ); + HostEngineChoice { + unsupported_here: unsupported_here_for( + &selection.engine, + engine_ruled_out_by_platform(&selection.engine), + ), + engine: selection.engine, + source: selection.source.to_owned(), + } + }; + rocm_core::model_readiness::assess_model_readiness(model_ref, catalog, host, &engine_for) +} + +/// Whether the platform gate rules this engine out on this host. +/// +/// One place, so the `/model` adapter-availability note and the readiness +/// verdict cannot answer differently about the same host and engine. +const fn engine_ruled_out_by_platform(engine: &str) -> bool { + rocm_core::runtime_is_windows() && engine.eq_ignore_ascii_case("vllm") +} + +/// The `unsupported_here` message for an engine, given whether the platform +/// gate already ruled it out. +/// +/// Split out from the `engine_for` closure in [`assess_model_for_host`] so it +/// can be unit-tested on both branches directly, instead of only indirectly +/// through whichever platform the test happens to run on. `ruled_out` is a +/// plain `bool` rather than re-deriving it from `engine` here, so a test can +/// exercise the "ruled out" branch on Linux CI and the "allowed" branch on a +/// Windows runner without either one being unreachable. +fn unsupported_here_for(engine: &str, ruled_out: bool) -> Option { + ruled_out + .then(|| format!("{engine} has no adapter on native Windows; serve it from WSL or Linux")) +} + fn fix(fix_id: Option, yes: bool, dry_run: bool, device_index: Option) -> Result<()> { let Some(fix_id) = fix_id else { print!("{}", rocm_core::list_fix_recipes()); @@ -17081,9 +17357,19 @@ fn append_model_fit_lines( output, " reason: current telemetry has no aggregate GPU VRAM reading" ); + // Not `/examine`: an `Examination` is a static host snapshot and + // carries no VRAM figure at all, so sending the user there for a + // missing VRAM reading is a confident dead end. But naming + // `amd-smi metric --json` directly is just as much of one here: + // every production caller of this function hardcodes + // `aggregate_gpu_vram_gib = None`, so the reading this function + // sees never changes no matter what the user runs. `rocm + // diagnose --model` is the command that actually threads a live + // reading through (`host_accelerator_memory`), so it is pointed + // at instead of a command this one cannot act on. let _ = writeln!( output, - " action: run /examine or refresh GPU telemetry, then retry /model {}", + " action: run `rocm diagnose --model {}` for a live reading", recipe_display_ref(recipe) ); } @@ -17113,19 +17399,36 @@ fn append_manual_alternative_lines( } } +/// Curated models to suggest in place of one that does not fit, for `/model`. +/// +/// The *selection policy* — declared alternatives first, else same-task curated +/// recipes, capped — lives in `rocm_core::model_readiness::curated_alternatives` +/// and is shared with `rocm diagnose --model`, so the two commands cannot come +/// to answer "what should I run instead" differently. What stays local is the +/// predicate and the wording: `/model` asks only whether the VRAM minimum is +/// met, while `diagnose --model` asks the whole readiness question, and each is +/// right for its own report. #[allow(dead_code)] fn manual_alternative_recommendations( recipe: &ModelRecipeRecord, aggregate_gpu_vram_gib: Option, ) -> Vec { - let declared = recipe - .manual_alternatives - .iter() - .filter_map(|candidate_ref| { - resolve_builtin_model_recipe(candidate_ref).map(|candidate| (candidate_ref, candidate)) - }) - .filter(|(_, candidate)| recipe_is_manual_fit(candidate, aggregate_gpu_vram_gib)) - .map(|(candidate_ref, candidate)| { + let catalog = builtin_model_recipes(); + rocm_core::model_readiness::curated_alternatives(Some(recipe), &catalog, &|candidate| { + recipe_is_manual_fit(candidate, aggregate_gpu_vram_gib) + }) + .into_iter() + .map(|(candidate_ref, candidate)| { + // A declared alternative is shown with its requirement, a fallback with + // its name alone. The two branches are exclusive -- a fallback only runs + // when no declared candidate survived the predicate, and a candidate + // that failed it in one branch fails it in the other -- so testing the + // declared list here recovers exactly which branch produced this pick. + if recipe + .manual_alternatives + .iter() + .any(|declared| declared == candidate_ref) + { format!( "{} ({})", candidate_ref, @@ -17134,19 +17437,11 @@ fn manual_alternative_recommendations( |value| format!("{} min GPU", format_gib(f64::from(value))) ) ) - }) - .collect::>(); - if !declared.is_empty() { - return declared; - } - builtin_model_recipes() - .into_iter() - .filter(|candidate| candidate.canonical_model_id != recipe.canonical_model_id) - .filter(|candidate| candidate.task == recipe.task) - .filter(|candidate| recipe_is_manual_fit(candidate, aggregate_gpu_vram_gib)) - .take(3) - .map(|candidate| recipe_display_ref(&candidate).to_owned()) - .collect() + } else { + candidate_ref.to_owned() + } + }) + .collect() } #[allow(dead_code)] @@ -17228,7 +17523,7 @@ fn append_model_engine_support_lines( #[allow(dead_code)] const fn model_registry_adapter_availability_note(engine: &str) -> Option<&'static str> { - if rocm_core::runtime_is_windows() && engine.eq_ignore_ascii_case("vllm") { + if engine_ruled_out_by_platform(engine) { Some( "runtime_status=unsupported_native_windows reason=native Windows skipped; use WSL/Linux vLLM ROCm; gpu_execution_required=true; run /engine for adapter details", ) @@ -17237,12 +17532,12 @@ const fn model_registry_adapter_availability_note(engine: &str) -> Option<&'stat } } +/// One definition, shared with `rocm diagnose --model`: a model named in a +/// suggestion has to be a string the user can paste back into `rocm serve`, and +/// two answers to "what do I call this recipe" is one too many. #[allow(dead_code)] fn recipe_display_ref(recipe: &ModelRecipeRecord) -> &str { - recipe - .aliases - .first() - .map_or(recipe.canonical_model_id.as_str(), String::as_str) + rocm_core::model_readiness::recipe_display_ref(recipe) } #[allow(dead_code)] @@ -21362,7 +21657,7 @@ fn validate_pinned_gpu_index( } /// A GPU's local VRAM occupancy as reported by `amd-smi metric --json`. -#[derive(Debug, Clone, Copy)] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] struct GpuVramUsage { index: u32, used_mb: u64, @@ -21520,6 +21815,16 @@ fn gpu_vram_usage() -> Option> { /// Parse `amd-smi metric --json` output into per-GPU VRAM usage. Accepts both /// the `{"gpu_data": [...]}` envelope and a bare top-level array, mirroring the /// schema variance handled by the dashboard amd-smi collector. +/// +/// A row reporting `total_vram: 0` is dropped rather than kept as a measurement +/// of an empty pool. `amd-smi` emits that shape when it enumerated the device +/// but could not read its memory controller (a driver hiccup, not a 0 GiB +/// GPU), and every other reader of this figure already treats zero as "not a +/// real reading": [`GpuVramUsage::free_fraction`] returns `None` on it rather +/// than reporting the GPU fully occupied. Keeping the row here would let +/// [`host_accelerator_memory`] construct `AcceleratorMemory::Dedicated(0.0)`, +/// which reads as "measured, and it is nothing" rather than "could not +/// measure" -- turning a diagnostic gap into a confident `Blocked` verdict. fn parse_gpu_vram_usage(value: &serde_json::Value) -> Vec { let entries = value .get("gpu_data") @@ -21542,6 +21847,9 @@ fn parse_gpu_vram_usage(value: &serde_json::Value) -> Vec { let total_mb = entry .pointer("/mem_usage/total_vram/value") .and_then(serde_json::Value::as_u64)?; + if total_mb == 0 { + return None; + } Some(GpuVramUsage { index, used_mb, @@ -36586,6 +36894,13 @@ ID_LIKE="suse opensuse" assert!(rendered.contains("recommended_system_ram: 16 GiB")); assert!(rendered.contains("system_ram_fit: unknown")); assert!(rendered.contains("gpu_fit: unknown")); + // Every production caller of this function hardcodes + // `aggregate_gpu_vram_gib = None` (see `Command::Model` and the `/model` + // assistant tool), so this branch is the only one `rocm model` ever + // actually reaches -- its remediation must name a command this same + // path can act on, not one whose reading this function never sees. + assert!(rendered.contains("action: run `rocm diagnose --model")); + assert!(!rendered.contains("amd-smi metric --json")); assert!(rendered.contains("engine_support:")); assert!(rendered.contains("engine_action: use /engine install ")); assert!(rendered.contains("source: built-in recipe registry")); @@ -38055,4 +38370,457 @@ ID_LIKE="suse opensuse" ); } } + + /// Hosts whose engine choice differs, so the assertion below is exercised on + /// more than one branch of `select_serve_engine`. + fn engine_selection_hosts() -> Vec<(&'static str, rocm_core::HostGpuSummary)> { + vec![ + ( + "Instinct MI300X", + rocm_core::HostGpuSummary { + name: Some("AMD Instinct MI300X".to_owned()), + gfx_target: Some("gfx942".to_owned()), + therock_family: Some("gfx94X-dcgpu".to_owned()), + }, + ), + ( + "Strix Halo", + rocm_core::HostGpuSummary { + name: Some("AMD Radeon 8060S".to_owned()), + gfx_target: Some("gfx1151".to_owned()), + therock_family: Some("gfx1151".to_owned()), + }, + ), + ("no detected GPU", rocm_core::HostGpuSummary::default()), + ] + } + + /// I3 — `rocm diagnose --model` never names an engine `rocm serve` would not + /// select. + /// + /// Two layers agreeing about one decision. The defect this catches is not a + /// wrong `select_serve_engine`, it is the readiness path re-deriving the + /// choice from `recipe.preferred_engines` and drifting: that reads correctly + /// and is wrong on exactly the hosts where serve overrides the recipe. So + /// both real call sites are driven with the same inputs and compared, rather + /// than either being called with literal arguments. + #[test] + fn the_engine_doctor_names_is_the_engine_serve_would_select() { + let registry = rocm_core::builtin_model_recipe_registry(); + let host = HostFacts { + accelerator_memory: AcceleratorMemory::Dedicated(192.0), + system_ram_gib: Some(1024.0), + }; + + // Non-vacuity: at least one pair must be a case where serve OVERRIDES the + // recipe's own first preference, because that is the only case a + // re-derivation would get wrong. Native Windows has no such case -- + // `preferred_serve_engine_for_host_gpu_summary` never prefers vLLM there + // -- so the check is stated where it exists and the equality assertion + // below still runs everywhere. + if !rocm_core::runtime_is_windows() { + let overridden = engine_selection_hosts().into_iter().any(|(_, summary)| { + registry.recipes.iter().any(|recipe| { + let selected = select_serve_engine(None, None, Some(recipe), Some(&summary)); + recipe + .preferred_engines + .first() + .is_some_and(|preferred| !preferred.eq_ignore_ascii_case(&selected.engine)) + }) + }); + assert!( + overridden, + "no host/recipe pair here exercises serve overriding the recipe's preferred \ + engine, so the comparison below cannot catch the readiness path re-deriving \ + the choice itself" + ); + } + + for (label, summary) in engine_selection_hosts() { + for recipe in ®istry.recipes { + let expected = select_serve_engine(None, None, Some(recipe), Some(&summary)); + let report = assess_model_for_host( + &recipe.canonical_model_id, + &ModelCatalogSource::Available(®istry), + &host, + None, + Some(&summary), + ); + assert_eq!( + report.engine.as_deref(), + Some(expected.engine.as_str()), + "on {label}, `rocm diagnose --model {}` names {:?} but `rocm serve` would \ + select `{}`", + recipe.canonical_model_id, + report.engine, + expected.engine + ); + } + } + } + + /// When the platform gate rules an engine out, the message names it and + /// points at WSL/Linux -- the actual escape hatch, not a guess. + #[test] + fn unsupported_here_for_names_the_engine_when_ruled_out() { + let message = + unsupported_here_for("vllm", true).expect("a ruled-out engine gets a message"); + assert!( + message.contains("vllm"), + "the message has to name the engine it is talking about: {message:?}" + ); + assert!( + message.to_lowercase().contains("wsl") || message.to_lowercase().contains("linux"), + "the message has to point at the actual escape hatch: {message:?}" + ); + } + + /// When the platform gate allows an engine, there is nothing to report -- + /// asserted directly against the pure helper, not by relying on whichever + /// OS this test happens to run on. + #[test] + fn unsupported_here_for_is_none_when_allowed() { + assert_eq!(unsupported_here_for("vllm", false), None); + assert_eq!(unsupported_here_for("llamacpp", false), None); + } + + /// Regression test for the engine-consistency bug: `assess_model_on_this_host` + /// used to call `AppPaths::discover()` twice -- once (implicitly `None`) for + /// the GPU summary, once for the config lookup a few lines later -- so the + /// two could read different directories. Writing a `default_engine` to one + /// known `AppPaths` and driving both the config lookup and the GPU-summary + /// lookup off that same value pins it down: if a future change reintroduces + /// a second, independent `AppPaths::discover()` for either half, the + /// configured engine this test writes to disk stops reaching the verdict. + #[test] + fn the_configured_default_engine_reaches_the_readiness_verdict_from_the_same_paths_as_the_gpu_summary() + { + let dir = std::env::temp_dir().join(format!( + "rocm-diagnose-model-engine-consistency-{}", + std::process::id() + )); + let _ = std::fs::remove_dir_all(&dir); + let paths = AppPaths { + config_dir: dir.clone(), + data_dir: dir.clone(), + cache_dir: dir.clone(), + }; + let config = RocmCliConfig { + default_engine: Some("vllm".to_owned()), + ..Default::default() + }; + config + .save(&paths) + .expect("write a config.json under the temp AppPaths"); + + let registry = rocm_core::builtin_model_recipe_registry(); + let recipe = registry + .recipes + .first() + .expect("the builtin registry ships at least one recipe"); + let examination = rocm_core::Examination::default(); + + // `configured_default` outranks the host GPU summary and the recipe's own + // preference in `select_serve_engine`, so this stays deterministic + // regardless of what this test happens to run on. + let summary = detect_host_gpu_summary(Some(&paths)); + let expected = select_serve_engine(None, Some("vllm"), Some(recipe), Some(&summary)); + + let readiness = + assess_model_with_host_paths(&recipe.canonical_model_id, &examination, Some(&paths)); + + let _ = std::fs::remove_dir_all(&dir); + + assert_eq!( + readiness.engine.as_deref(), + Some(expected.engine.as_str()), + "the config written under this AppPaths said default_engine = \"vllm\", but the \ + readiness verdict named {:?} -- the config lookup and the GPU-summary lookup must \ + read the same discovered AppPaths", + readiness.engine + ); + } + + /// A row reporting `total_vram: 0` is not a measurement of an empty GPU; it + /// is `amd-smi` enumerating a device it could not read the memory + /// controller for. Keeping the row would let `host_accelerator_memory` + /// report `AcceleratorMemory::Dedicated(0.0)`, which the readiness verdict + /// then treats as "measured, and it is nothing" (`Blocked`) rather than + /// "could not measure" (`Undetermined`). + #[test] + fn parse_gpu_vram_usage_drops_a_zero_total_row() { + let value = json!({ + "gpu_data": [ + { + "gpu": 0, + "mem_usage": { + "used_vram": {"value": 0}, + "total_vram": {"value": 0}, + }, + }, + { + "gpu": 1, + "mem_usage": { + "used_vram": {"value": 512}, + "total_vram": {"value": 16384}, + }, + }, + ], + }); + + let rows = parse_gpu_vram_usage(&value); + + assert_eq!( + rows, + vec![GpuVramUsage { + index: 1, + used_mb: 512, + total_mb: 16384, + }], + "a zero-total row must be dropped, not kept as a 0 GiB measurement: {rows:?}" + ); + } + + /// All rows reporting a zero total collapses to no telemetry at all, so + /// `gpu_vram_usage()`'s own `is_empty` check turns it into `None` -- + /// "could not measure" -- rather than a `Vec` of one unusable row. + #[test] + fn parse_gpu_vram_usage_returns_nothing_when_every_row_is_zero_total() { + let value = json!({ + "gpu_data": [{ + "gpu": 0, + "mem_usage": { + "used_vram": {"value": 0}, + "total_vram": {"value": 0}, + }, + }], + }); + + assert!(parse_gpu_vram_usage(&value).is_empty()); + } + + /// A single dedicated GPU reports its measured total, unmodified. + #[test] + fn classify_accelerator_memory_reports_dedicated_vram_for_a_discrete_gpu() { + let vram = [GpuVramUsage { + index: 0, + used_mb: 1024, + total_mb: 16384, + }]; + let result = classify_accelerator_memory(Some(&vram), Some("gfx942"), true, None); + assert_eq!(result, AcceleratorMemory::Dedicated(16.0)); + } + + /// The defect this guards against: an APU's `amd-smi` carve-out figure is + /// not the pool the engine actually allocates from (the real pool is the + /// GTT aperture, which nothing in this binary can read), so asserting + /// installed system RAM as a stand-in used to fabricate a number that + /// could read `Ready` for a model that does not actually fit. The correct, + /// honest answer is `UnifiedMemoryUnreadable`, not a number of any kind, + /// and not the plain `Unknown` used for genuine no-telemetry hosts either + /// -- confirming this never again quietly regresses to reporting a + /// fabricated figure (or worse, the APU's own tiny carve-out as + /// `Dedicated`), and never again collapses back into the generic + /// `Unknown`, whose `amd-smi` remediation would be a dead end here. + #[test] + fn classify_accelerator_memory_reports_unifiedmemoryunreadable_not_total_ram_for_an_apu() { + let apu_carveout = [GpuVramUsage { + index: 0, + used_mb: 2048, + total_mb: 4096, + }]; + let result = classify_accelerator_memory(Some(&apu_carveout), Some("gfx1151"), true, None); + assert_eq!( + result, + AcceleratorMemory::UnifiedMemoryUnreadable, + "an APU's memory pool must read UnifiedMemoryUnreadable, not a fabricated figure, \ + and not the generic Unknown either: {result:?}" + ); + } + + /// No VRAM telemetry at all, but the examination positively saw an AMD + /// GPU: unmeasurable, not absent. + #[test] + fn classify_accelerator_memory_reports_unknown_when_gpu_present_but_unmeasured() { + let result = classify_accelerator_memory(None, None, true, None); + assert_eq!(result, AcceleratorMemory::Unknown); + } + + /// No VRAM telemetry and no evidence of an AMD GPU: there is nothing to + /// allocate a model from. + #[test] + fn classify_accelerator_memory_reports_none_when_no_gpu_is_present() { + let result = classify_accelerator_memory(None, None, false, None); + assert_eq!(result, AcceleratorMemory::None); + } + + /// An APU paired with a discrete card (more than one GPU reported) keeps + /// the discrete card's warning meaningful, per `vram_capacity_is_meaningful` + /// -- the multi-GPU case cannot attribute the gfx target to one ordinal, so + /// it is never treated as the unified-memory case even when the target + /// looks like an APU family. + #[test] + fn classify_accelerator_memory_treats_multi_gpu_as_dedicated_even_with_apu_gfx_target() { + let vram = [ + GpuVramUsage { + index: 0, + used_mb: 1024, + total_mb: 4096, + }, + GpuVramUsage { + index: 1, + used_mb: 2048, + total_mb: 8192, + }, + ]; + let result = classify_accelerator_memory(Some(&vram), Some("gfx1151"), true, None); + assert_eq!(result, AcceleratorMemory::Dedicated(8.0)); + } + + /// The defect a reviewer caught: `rocm serve` pins exactly one GPU + /// ordinal, never the sum of every card, so summing VRAM across GPUs can + /// report a model READY when it would OOM on every individual card. On an + /// 8x192 GiB host a sum of ~1536 GiB would clear even the largest curated + /// recipe's minimum; the true per-card figure (192 GiB) is what the + /// verdict must compare against. Were this still summing, the assertion + /// below would see `Dedicated(1536.0)`, not `Dedicated(192.0)`. + #[test] + fn classify_accelerator_memory_takes_the_largest_card_not_the_sum() { + let vram: Vec = (0..8) + .map(|index| GpuVramUsage { + index, + used_mb: 0, + total_mb: 192 * 1024, + }) + .collect(); + let result = classify_accelerator_memory(Some(&vram), Some("gfx942"), true, None); + assert_eq!( + result, + AcceleratorMemory::Dedicated(192.0), + "a homogeneous 8-GPU host must report one card's capacity, not the sum of all \ + eight, or a model that OOMs on every card reads READY: {result:?}" + ); + } + + /// A `HIP_VISIBLE_DEVICES`-style mask hides a GPU from `rocm serve` the + /// same way it hides one from auto-selection (see + /// `select_auto_gpu_index`'s own mask handling) -- but `amd-smi` enumerates + /// at the driver level and knows nothing about that mask, so its rows + /// still include the hidden card. Narrowing by `usable_indices` is what + /// keeps a masked-out GPU's capacity from inflating the verdict for a + /// model that could never actually land on it. + #[test] + fn classify_accelerator_memory_ignores_a_gpu_masked_out_by_the_visibility_filter() { + let vram = [ + GpuVramUsage { + index: 0, + used_mb: 0, + total_mb: 8 * 1024, + }, + GpuVramUsage { + index: 1, + used_mb: 0, + total_mb: 192 * 1024, + }, + ]; + // Only index 0 is visible: the 192 GiB card at index 1 is masked out. + let result = classify_accelerator_memory(Some(&vram), Some("gfx942"), true, Some(&[0])); + assert_eq!( + result, + AcceleratorMemory::Dedicated(8.0), + "a masked-out GPU's VRAM must not count toward the verdict: {result:?}" + ); + } + + /// The end-to-end proof, one layer above `classify_accelerator_memory`'s own + /// unit tests: not just that the function picks the right number, but that + /// picking the wrong one would have changed the verdict a user actually + /// sees. Two 8 GiB cards sum to 16 GiB, comfortably above `qwen3.5-4b`'s 12 + /// GiB minimum -- a verdict built on the sum would read READY, and `rocm + /// serve` pins exactly one ordinal, so that model would OOM on every card + /// this host has. No single card clears 12 GiB, so the correct verdict is + /// BLOCKED. Were `classify_accelerator_memory` still summing, this would + /// observe `Ready`, not `Blocked`. + #[test] + fn a_host_whose_vram_sum_clears_the_minimum_but_no_single_card_does_is_blocked_not_ready() { + let vram = [ + GpuVramUsage { + index: 0, + used_mb: 0, + total_mb: 8 * 1024, + }, + GpuVramUsage { + index: 1, + used_mb: 0, + total_mb: 8 * 1024, + }, + ]; + let accelerator_memory = + classify_accelerator_memory(Some(&vram), Some("gfx942"), true, None); + // Premise check: if this is ever 16.0 (the sum) instead of 8.0 (the + // largest single card), the scenario below no longer tests what it + // claims to. + assert_eq!(accelerator_memory, AcceleratorMemory::Dedicated(8.0)); + + let host = HostFacts { + accelerator_memory, + system_ram_gib: Some(1024.0), + }; + let registry = rocm_core::builtin_model_recipe_registry(); + let report = assess_model_for_host( + "qwen3.5-4b", + &ModelCatalogSource::Available(®istry), + &host, + None, + None, + ); + assert_eq!( + report.verdict, + rocm_core::model_readiness::ModelVerdict::Blocked, + "qwen3.5-4b needs 12 GiB; two 8 GiB cards sum to 16 GiB (which would clear it) but \ + no single card does (8 GiB each) -- the verdict must be BLOCKED, not READY, or \ + `rocm serve` would pin one 8 GiB card to a model that does not fit on it: {report:#?}" + ); + } + + /// `has_amd_gpu` is always `false` on WSL (the probes that set it are + /// skipped there), so `host_accelerator_memory` must read `rocm_sees_gpu` + /// instead on a WSL host -- and only a confirmed `Some(false)` counts as + /// "no GPU"; `None` means rocminfo was absent, which is not evidence of + /// anything, per the same reasoning `fix-wsl-6-host-driver-too-old` in + /// `diagnose.rs` already uses. + #[test] + fn examination_reports_amd_gpu_reads_rocm_sees_gpu_on_wsl() { + let wsl_examination = |rocm_sees_gpu: Option| rocm_core::Examination { + is_wsl: true, + wsl: Some(rocm_core::examine::WslFacts { + rocm_sees_gpu, + ..Default::default() + }), + ..rocm_core::Examination::default() + }; + + assert!( + !examination_reports_amd_gpu(&wsl_examination(Some(false))), + "rocminfo positively enumerating no device must read as no GPU" + ); + assert!( + examination_reports_amd_gpu(&wsl_examination(Some(true))), + "rocminfo positively enumerating a device must read as a GPU present" + ); + assert!( + examination_reports_amd_gpu(&wsl_examination(None)), + "rocminfo absent (the question went unasked) must not read as a confident no" + ); + } + + /// Off WSL, the bare-metal probes are what set `has_amd_gpu`, so this must + /// keep using it rather than a WSL-only signal that was never populated. + #[test] + fn examination_reports_amd_gpu_reads_has_amd_gpu_off_wsl() { + let mut examination = rocm_core::Examination::default(); + assert!(!examination_reports_amd_gpu(&examination)); + examination.has_amd_gpu = true; + assert!(examination_reports_amd_gpu(&examination)); + } } diff --git a/apps/rocm/src/remote/doctor.rs b/apps/rocm/src/remote/doctor.rs index 06ca50bad..77001d4c0 100644 --- a/apps/rocm/src/remote/doctor.rs +++ b/apps/rocm/src/remote/doctor.rs @@ -338,6 +338,11 @@ mod tests { url: "https://example.invalid/issues".to_owned(), }, out_of_scope: None, + // These fixtures exercise the remote renderer, which prints the + // environment half of a report. A model verdict rides on top of + // that half rather than replacing it, so there is nothing for these + // to say about one. + model: None, } } diff --git a/crates/rocm-core/src/diagnose.rs b/crates/rocm-core/src/diagnose.rs index e4652479c..34ecfe030 100644 --- a/crates/rocm-core/src/diagnose.rs +++ b/crates/rocm-core/src/diagnose.rs @@ -87,6 +87,16 @@ pub struct DiagnoseReport { /// avoid emitting bare-metal-Linux diagnoses that don't apply. #[serde(default)] pub out_of_scope: Option, + /// The verdict on a model, when `--model` named one. + /// + /// Attached by the caller after the fact rather than produced by + /// [`diagnose`]: answering it needs the host's GPU memory and the engine + /// `serve` would select, neither of which an [`Examination`] carries. It + /// rides on this report rather than replacing it because the environment + /// answer stays true and useful either way — a blocked model on a host whose + /// driver is also misconfigured is two findings, not one. + #[serde(default)] + pub model: Option, } /// Whether any diagnosis cleared [`MIN_SCORE_FOR_MATCH`]. @@ -2115,6 +2125,20 @@ fn catalog_covers(e: &Examination) -> bool { .any(|(_, applicable)| applicable.contains(&family)) } +/// Where to report something this CLI could not answer. +/// +/// The one place a `Route` is built, so every command that has to say "I don't +/// recognise this" sends the user to the same tracker for the same target — +/// `rocm diagnose --model` has the same problem for a model the catalog does not +/// carry as `rocm diagnose` has for a symptom it does not recognise. +#[must_use] +pub fn upstream_route(target: &str) -> Route { + Route { + target: target.to_owned(), + url: upstream_tracker(target).to_owned(), + } +} + /// Where to send a user when nothing in the catalog matched. /// /// Keyed off the *host-detected* framework, which `Examination::probe` only @@ -2132,10 +2156,7 @@ fn route_when_no_match(e: &Examination) -> Route { "llama-cpp" => "llama-cpp", _ => "rocm-core", }; - Route { - target: target.to_owned(), - url: upstream_tracker(target).to_owned(), - } + upstream_route(target) } /// Diagnose an examination against the closed catalog. @@ -2162,6 +2183,7 @@ pub fn diagnose(e: &Examination, symptom: &str) -> DiagnoseReport { high_confidence_threshold: HIGH_CONFIDENCE, route_when_no_match: route_when_no_match(e), out_of_scope, + model: None, } } diff --git a/crates/rocm-core/src/lib.rs b/crates/rocm-core/src/lib.rs index c9d096f50..86b1a7cd7 100644 --- a/crates/rocm-core/src/lib.rs +++ b/crates/rocm-core/src/lib.rs @@ -31,6 +31,7 @@ pub mod diagnose; pub mod disk_space; pub mod examine; pub mod fix; +pub mod model_readiness; pub mod openmpi; pub mod proc_lifecycle; pub mod runtime; @@ -38,7 +39,7 @@ pub mod runtime; mod test_env; pub mod uv; pub use diagnose::{ - DiagnoseReport, Diagnosis, Fix, diagnose as run_diagnose, + DiagnoseReport, Diagnosis, Fix, Route, diagnose as run_diagnose, render_report_text as render_diagnose_text, }; pub use disk_space::{ diff --git a/crates/rocm-core/src/model_readiness.rs b/crates/rocm-core/src/model_readiness.rs new file mode 100644 index 000000000..ceeead4e7 --- /dev/null +++ b/crates/rocm-core/src/model_readiness.rs @@ -0,0 +1,1218 @@ +// Copyright © Advanced Micro Devices, Inc., or its affiliates. +// +// SPDX-License-Identifier: MIT + +//! Will this model run on this machine? +//! +//! `examine` answers what is on the host and `diagnose` answers what went wrong +//! after a failure. This answers the question that comes before both, from data +//! the CLI already has: a curated [`ModelRecipeRecord`] (dtype, quantization, +//! minimum GPU memory, device policy, preferred engines, declared alternatives) +//! composed with what the host can offer. **It fetches nothing** — no weights, +//! no metadata probe, no network call of any kind — which is the whole point: +//! today a user learns a model will not run by starting a download and waiting +//! for it to fail. +//! +//! Three distinctions carry the design, and collapsing any of them produces a +//! confident wrong answer: +//! +//! - A catalog that could not be **read** is not a model that does not **fit**. +//! [`ModelCatalogSource`] keeps them apart and [`ModelVerdict::Undetermined`] +//! is where the first one lands. +//! - Memory the CLI **could not measure** is not memory the host **does not +//! have**. See [`AcceleratorMemory`]. +//! - No telemetry at all is not the same gap as telemetry that names the +//! *wrong pool*: an APU's `amd-smi` reading is real, it just is not the +//! figure its engine allocates from, and the CLI cannot read that figure +//! today. Conflating the two hands an APU user a remediation that can never +//! work. See [`AcceleratorMemory::UnifiedMemoryUnreadable`]. +//! +//! Curated recipes only. Answering for an arbitrary hub model needs a metadata +//! probe and a table mapping quantization schemes to available kernels, neither +//! of which has a source of truth here; such a model is reported +//! [`UndeterminedReason::ModelNotCurated`], never blocked. + +use crate::ModelRecipeRecord; +use crate::ModelRecipeRegistry; +use crate::diagnose::{Fix, Route, upstream_route}; +use serde::{Deserialize, Serialize}; + +/// How many fallback alternatives to offer when a recipe declares none that fit. +/// +/// Matches what `rocm model --verbose` has always offered; the cap exists so a +/// refusal stays readable, not because more could not be found. +const MAX_FALLBACK_ALTERNATIVES: usize = 3; + +/// Whether the model will run here. +/// +/// Four states, deliberately not three. Folding `Undetermined` into `Blocked` +/// would report "this machine cannot serve that model" when the truth is "the +/// CLI could not find out", which reads exactly like a real answer and sends the +/// user looking for hardware they may not need. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum ModelVerdict { + /// It will run, on the named engine. + Ready, + /// It will run, but below what the recipe recommends. + Degraded, + /// It will not run, and the evidence says why. + Blocked, + /// The CLI could not find out. See [`ModelReadiness::undetermined_reason`]. + Undetermined, +} + +/// Why no verdict about the model could be reached. +/// +/// Serialized so a caller can tell a local misconfiguration (the first two) from +/// a gap in host telemetry (the third) without parsing prose. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum UndeterminedReason { + /// The recipe catalog itself could not be read or verified. + CatalogUnreachable, + /// The catalog was read and carries no recipe for this model. + ModelNotCurated, + /// The host's accelerator memory could not be measured. + AcceleratorMemoryUnknown, + /// The host is an APU: telemetry exists, but it names the BIOS carve-out + /// rather than the pool its engine allocates from, and the CLI cannot yet + /// read that pool. Distinct from [`Self::AcceleratorMemoryUnknown`] + /// because the remediation is different -- there is no command that makes + /// this readable today, so none is offered. + UnifiedMemoryUnreadable, +} + +/// The memory pool an engine would allocate this model from. +#[derive(Debug, Clone, Copy, PartialEq)] +pub enum AcceleratorMemory { + /// Dedicated VRAM, in GiB. What a discrete card reports is what it has. + Dedicated(f64), + /// No GPU is visible to ROCm on this host. + None, + /// There is a GPU, but its memory could not be read at all -- no + /// telemetry, positive or otherwise. + /// + /// Distinct from [`AcceleratorMemory::None`] on purpose: one is a fact about + /// the machine and the other is a gap in what the CLI can see, and they do + /// not deserve the same verdict. + Unknown, + /// An APU: `amd-smi` telemetry exists, but it reports the fixed BIOS + /// carve-out (often ~4 GiB) as `total_vram`, while the allocator serves the + /// model out of GTT-backed system memory. That figure is not readable by + /// anything in this binary today, so there is no number to compare a + /// recipe minimum against, and no command to run that would produce one. + /// + /// Distinct from [`AcceleratorMemory::Unknown`]: that variant means no + /// telemetry at all, so "run `amd-smi metric --json`" is a real next step. + /// Here `amd-smi` already ran and answered -- it just named the wrong + /// pool -- so the same remediation would send a Strix Halo user in a + /// circle forever. + UnifiedMemoryUnreadable, +} + +impl AcceleratorMemory { + /// The figure to compare a recipe minimum against, when there is one. + const fn measured_gib(self) -> Option { + match self { + Self::Dedicated(gib) => Some(gib), + Self::None | Self::Unknown | Self::UnifiedMemoryUnreadable => None, + } + } +} + +/// The engine `serve` would select for a recipe on this host. +/// +/// Supplied by the caller rather than derived here. Engine selection is one +/// decision with one implementation (`select_serve_engine` in the `rocm` +/// binary); re-deriving it from `preferred_engines` reads correctly and is wrong +/// on exactly the hosts where serve overrides the recipe's preference. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct HostEngineChoice { + pub engine: String, + /// Why that engine, in serve's own words. + pub source: String, + /// Set when the platform gate rules this engine out here (for example vLLM + /// on native Windows). `serve` does not silently pick another one, so + /// neither does this. + pub unsupported_here: Option, +} + +/// What the host can offer, as far as this question needs. +#[derive(Debug, Clone, PartialEq)] +pub struct HostFacts { + pub accelerator_memory: AcceleratorMemory, + /// Host system RAM in GiB, or `None` when it could not be read. Advisory: + /// it can only soften a verdict to [`ModelVerdict::Degraded`], never decide + /// one, so not knowing it does not make the answer undeterminable. + pub system_ram_gib: Option, +} + +/// The recipe catalog, or the reason there isn't one. +pub enum ModelCatalogSource<'a> { + Available(&'a ModelRecipeRegistry), + /// The index could not be read, parsed, or signature-verified. + Unreachable { + detail: String, + }, +} + +/// A curated model offered in place of one that will not run here. +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +pub struct ModelAlternative { + /// The reference to pass to `rocm serve`. + pub model_ref: String, + pub required_gpu_memory_gib: Option, + pub engine: String, +} + +/// The answer to "will this model run here". +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +pub struct ModelReadiness { + /// What the user asked about, verbatim. + pub model_ref: String, + /// Set once a recipe was matched; `None` means no recipe was ever read, so + /// nothing below describes the model itself. + pub canonical_model_id: Option, + pub verdict: ModelVerdict, + /// What the verdict was reached from, one fact per line. + pub evidence: Vec, + pub engine: Option, + pub engine_source: Option, + pub required_gpu_memory_gib: Option, + pub available_gpu_memory_gib: Option, + pub alternatives: Vec, + /// What to do about it, in the same shape `rocm diagnose` uses for a cause. + /// + /// Carries no `fix_id`: there is no catalog entry behind it, so `rocm fix` + /// would reject one, and rendering an `apply with:` line here would name a + /// command that cannot run. + pub fix: Option, + /// Where to report it, when the answer is one this CLI cannot give. + pub route: Option, + pub undetermined_reason: Option, + /// The recipe's own warnings, passed through unchanged. + /// + /// Informational — every curated recipe carries at least one — so they do + /// not move the verdict. Degradation is decided from measurements. + pub warnings: Vec, +} + +impl ModelReadiness { + /// An answer reached before any recipe was read. + /// + /// Every field describing the comparison is left empty, including the host's + /// own measured memory: they are read as one set, and a figure sitting + /// beside a null requirement invites a reader to supply the missing half + /// themselves. `AcceleratorMemoryUnknown` does not come through here — a + /// recipe *was* read in that case, and what it needs is worth saying even + /// when the host side of the comparison is missing. + fn undetermined(model_ref: &str, reason: UndeterminedReason, evidence: Vec) -> Self { + Self { + model_ref: model_ref.to_owned(), + canonical_model_id: None, + verdict: ModelVerdict::Undetermined, + evidence, + engine: None, + engine_source: None, + required_gpu_memory_gib: None, + available_gpu_memory_gib: None, + alternatives: Vec::new(), + fix: None, + route: None, + undetermined_reason: Some(reason), + warnings: Vec::new(), + } + } +} + +/// Pick curated models to offer in place of one that will not run. +/// +/// The recipe's declared `manual_alternatives` first, in the order it lists +/// them; only when none of those survive `fits` does it fall back to other +/// curated recipes for the same task, capped at +/// [`MAX_FALLBACK_ALTERNATIVES`]. The cap applies to the fallback alone, because +/// a recipe author listing four alternatives meant all four. +/// +/// `fits` is the caller's, because callers mean different things by it: `rocm +/// model --verbose` asks only whether the GPU-memory minimum is met, while +/// `rocm diagnose --model` asks the whole readiness question. Sharing the +/// *selection policy* while keeping the predicates apart is deliberate — what +/// must not exist twice is the ordering and the fallback rule. +/// +/// Returns each candidate paired with the reference to name it by: the string +/// the recipe declared, or the candidate's own display alias for fallbacks. +#[must_use] +pub fn curated_alternatives<'a>( + recipe: Option<&'a ModelRecipeRecord>, + catalog: &'a [ModelRecipeRecord], + fits: &dyn Fn(&ModelRecipeRecord) -> bool, +) -> Vec<(&'a str, &'a ModelRecipeRecord)> { + let declared = recipe + .map(|recipe| { + recipe + .manual_alternatives + .iter() + .filter_map(|candidate_ref| { + catalog + .iter() + .find(|candidate| candidate.matches_ref(candidate_ref)) + .map(|candidate| (candidate_ref.as_str(), candidate)) + }) + .filter(|(_, candidate)| fits(candidate)) + .collect::>() + }) + .unwrap_or_default(); + if !declared.is_empty() { + return declared; + } + catalog + .iter() + .filter(|candidate| { + recipe.is_none_or(|recipe| candidate.canonical_model_id != recipe.canonical_model_id) + }) + .filter(|candidate| recipe.is_none_or(|recipe| candidate.task == recipe.task)) + .filter(|candidate| fits(candidate)) + .take(MAX_FALLBACK_ALTERNATIVES) + .map(|candidate| (recipe_display_ref(candidate), candidate)) + .collect() +} + +/// The shortest reference a user can type for a recipe. +#[must_use] +pub fn recipe_display_ref(recipe: &ModelRecipeRecord) -> &str { + recipe + .aliases + .first() + .map_or(recipe.canonical_model_id.as_str(), String::as_str) +} + +/// Whether this recipe needs a GPU at all. +fn requires_gpu(recipe: &ModelRecipeRecord) -> bool { + recipe.device_policy != "cpu_only" +} + +/// Format a GiB figure for a verdict's evidence text. +/// +/// The fractional branch floors to one decimal rather than rounding to +/// nearest. A measured figure (`available`/`actual`) sits just under its +/// nameplate size -- an 8 GiB card reports 8176 MiB, which is 7.9844 GiB -- and +/// rounding to nearest would print "8.0 GiB" against an "8 GiB" recipe minimum +/// on a BLOCKED verdict: evidence that reads as "the host has exactly what the +/// recipe needs" while the verdict says otherwise. `required`/`recommended` +/// values come from `u32` catalog fields, so they are always exact integers +/// and never reach this branch; flooring only ever shades a measured figure +/// down, never a minimum. +fn format_gib(value: f64) -> String { + if value.fract().abs() < f64::EPSILON { + format!("{value:.0} GiB") + } else { + format!("{:.1} GiB", (value * 10.0).floor() / 10.0) + } +} + +/// Answer "will this model run here", from the catalog and the host alone. +/// +/// Reads no file the caller did not already hand over and opens no socket. The +/// `engine_for` closure is how the caller's engine decision reaches this without +/// being copied into it. +#[must_use] +pub fn assess_model_readiness( + model_ref: &str, + catalog: &ModelCatalogSource<'_>, + host: &HostFacts, + engine_for: &dyn Fn(&ModelRecipeRecord) -> HostEngineChoice, +) -> ModelReadiness { + assess(model_ref, catalog, host, engine_for, true) +} + +/// The body of [`assess_model_readiness`], with alternatives made optional. +/// +/// `offer_alternatives` is what terminates the recursion, and it is not an +/// optimisation. Deciding whether an alternative is worth offering means asking +/// the same question about it, and the catalog's declared alternatives point +/// both ways — `qwen` offers `qwen-tiny`, `qwen-tiny` offers `qwen`. On a host +/// where both are blocked, an assessment that recursed freely would follow that +/// cycle until the stack ran out. A candidate is therefore assessed as a model +/// and never as a source of further candidates: the recursion is exactly one +/// level deep by construction. +fn assess( + model_ref: &str, + catalog: &ModelCatalogSource<'_>, + host: &HostFacts, + engine_for: &dyn Fn(&ModelRecipeRecord) -> HostEngineChoice, + offer_alternatives: bool, +) -> ModelReadiness { + let registry = match catalog { + // The first of the three collapses this module exists to prevent. A + // signed index that is missing, unreadable or fails verification says + // nothing whatsoever about the model, so nothing about the model is + // reported -- not even the minimum it requires, which was never read. + ModelCatalogSource::Unreachable { detail } => { + let mut readiness = ModelReadiness::undetermined( + model_ref, + UndeterminedReason::CatalogUnreachable, + vec![ + format!("the model recipe catalog could not be read: {detail}"), + format!( + "this is a fact about the catalog, not about `{model_ref}`; the recipe \ + was never read, so nothing here says whether the model fits" + ), + ], + ); + readiness.fix = Some(Fix { + summary: "restore the recipe catalog, then ask again".to_owned(), + verify: "rocm model".to_owned(), + notes: vec![ + "ROCM_CLI_MODEL_RECIPE_INDEX_PATH selects a signed index; unset it to fall \ + back to the catalog built into this binary" + .to_owned(), + "a configured index also needs \ + ROCM_CLI_MODEL_RECIPE_INDEX_PUBLIC_KEY_PATH, and its signature sidecar \ + beside it" + .to_owned(), + ], + ..Fix::default() + }); + return readiness; + } + ModelCatalogSource::Available(registry) => *registry, + }; + + let Some(recipe) = registry + .recipes + .iter() + .find(|recipe| recipe.matches_ref(model_ref)) + else { + // Not "this model will not run" -- the catalog simply has no recipe for + // it, and answering for an arbitrary hub model is out of scope. What can + // honestly be offered is what the catalog does carry that runs here. + let mut readiness = ModelReadiness::undetermined( + model_ref, + UndeterminedReason::ModelNotCurated, + vec![ + format!("`{model_ref}` is not in the curated recipe catalog"), + "`rocm diagnose --model` answers for curated recipes only: judging an arbitrary \ + model needs metadata this CLI does not fetch" + .to_owned(), + ], + ); + if offer_alternatives { + readiness.alternatives = alternatives_for(None, registry, host, engine_for); + } + readiness.route = Some(upstream_route("rocm-core")); + readiness.fix = Some(Fix { + summary: "ask about a curated recipe, or serve this model directly and read the \ + engine's own refusal" + .to_owned(), + commands: vec!["rocm model".to_owned()], + verify: format!("rocm serve {model_ref}"), + notes: vec![ + "`rocm serve` accepts models outside this catalog; it just cannot say in advance \ + whether they will fit" + .to_owned(), + ], + ..Fix::default() + }); + return readiness; + }; + + let choice = engine_for(recipe); + let required = recipe.min_gpu_mem_gb.map(f64::from); + let available = host.accelerator_memory.measured_gib(); + let mut evidence = Vec::new(); + evidence.push(format!( + "recipe {} ({}, {})", + recipe.canonical_model_id, + recipe.dtype, + recipe + .quantization + .as_deref() + .unwrap_or("no quantization declared") + )); + evidence.push(format!( + "{} would serve it ({})", + choice.engine, choice.source + )); + let mut readiness = ModelReadiness { + model_ref: model_ref.to_owned(), + canonical_model_id: Some(recipe.canonical_model_id.clone()), + verdict: ModelVerdict::Ready, + evidence, + engine: Some(choice.engine.clone()), + engine_source: Some(choice.source.clone()), + required_gpu_memory_gib: required, + available_gpu_memory_gib: available, + alternatives: Vec::new(), + fix: None, + route: None, + undetermined_reason: None, + warnings: recipe.warnings.clone(), + }; + + // Ordered by what pre-empts what. The engine gate comes first because a + // model that fits in memory still will not run on an engine this platform + // has no adapter for, and reporting the memory verdict there would be true + // and useless. + if let Some(reason) = &choice.unsupported_here { + readiness.verdict = ModelVerdict::Blocked; + readiness.evidence.push(reason.clone()); + if offer_alternatives { + readiness.alternatives = alternatives_for(Some(recipe), registry, host, engine_for); + } + readiness.fix = Some(blocked_fix( + "serve this model from a platform that has the engine", + &readiness.alternatives, + model_ref, + vec![reason.clone()], + )); + return readiness; + } + + if requires_gpu(recipe) && matches!(host.accelerator_memory, AcceleratorMemory::None) { + readiness.verdict = ModelVerdict::Blocked; + readiness.evidence.push(format!( + "no GPU is visible to ROCm, and this recipe's device policy is `{}`; there is no CPU \ + fallback", + recipe.device_policy + )); + readiness.fix = Some(Fix { + summary: "make a GPU visible to ROCm, then ask again".to_owned(), + commands: vec!["rocm diagnose".to_owned()], + verify: format!("rocm diagnose --model {model_ref}"), + notes: vec![ + "`rocm diagnose` matches this machine against the known reasons a GPU does not \ + show up" + .to_owned(), + ], + ..Fix::default() + }); + return readiness; + } + + match (required, available) { + // The second collapse: memory that could not be measured is not memory + // the host does not have. Blocking here would name the model as the + // problem when the gap is in the host telemetry. + (Some(required), None) + if matches!( + host.accelerator_memory, + AcceleratorMemory::UnifiedMemoryUnreadable + ) => + { + // The APU case: telemetry is not missing, it names the wrong pool, + // and there is no command that makes the right one readable today. + // Offering "run amd-smi again" here would be false -- amd-smi + // already ran, answered, and will answer identically forever. + readiness.verdict = ModelVerdict::Undetermined; + readiness.undetermined_reason = Some(UndeterminedReason::UnifiedMemoryUnreadable); + readiness.evidence.push(format!( + "this recipe needs {}, but this host has no dedicated VRAM and the CLI cannot \ + yet read the memory pool its engine allocates from; there is nothing to run \ + today that would change this answer", + format_gib(required) + )); + readiness.fix = None; + } + (Some(required), None) => { + readiness.verdict = ModelVerdict::Undetermined; + readiness.undetermined_reason = Some(UndeterminedReason::AcceleratorMemoryUnknown); + readiness.evidence.push(format!( + "this recipe needs {}, but the GPU memory on this host could not be read; that \ + is a gap in what the CLI can see, not a verdict about the model", + format_gib(required) + )); + readiness.fix = Some(Fix { + summary: "let the CLI read this host's GPU memory, then ask again".to_owned(), + commands: vec!["amd-smi metric --json".to_owned()], + verify: format!("rocm diagnose --model {model_ref}"), + notes: vec![ + "the reading comes from `amd-smi`; if it is missing or failing, the ROCm \ + install is what needs attention first" + .to_owned(), + ], + ..Fix::default() + }); + } + (Some(required), Some(available)) if available < required => { + readiness.verdict = ModelVerdict::Blocked; + readiness.evidence.push(format!( + "this recipe needs {}, and this host offers {}", + format_gib(required), + format_gib(available) + )); + if offer_alternatives { + readiness.alternatives = alternatives_for(Some(recipe), registry, host, engine_for); + } + readiness.fix = Some(blocked_fix( + "serve a curated model that fits this machine", + &readiness.alternatives, + model_ref, + Vec::new(), + )); + } + (Some(required), Some(available)) => { + readiness.evidence.push(format!( + "this recipe needs {}, and this host offers {}", + format_gib(required), + format_gib(available) + )); + } + (None, _) => { + readiness.evidence.push(format!( + "this recipe declares no GPU memory minimum (device policy `{}`)", + recipe.device_policy + )); + } + } + + if readiness.verdict == ModelVerdict::Ready + && let (Some(recommended), Some(actual)) = ( + recipe.recommended_system_ram_gb.map(f64::from), + host.system_ram_gib, + ) + && actual < recommended + { + readiness.verdict = ModelVerdict::Degraded; + readiness.evidence.push(format!( + "this recipe recommends {} of system RAM and this host has {}; it will run, with \ + slower loading and less headroom", + format_gib(recommended), + format_gib(actual) + )); + readiness.fix = Some(Fix { + summary: "expect a slower load, or free system RAM before serving".to_owned(), + verify: format!("rocm diagnose --model {model_ref}"), + ..Fix::default() + }); + } + + readiness +} + +/// A remediation for a model that will not run, built from what would. +fn blocked_fix( + summary: &str, + alternatives: &[ModelAlternative], + model_ref: &str, + mut notes: Vec, +) -> Fix { + if alternatives.is_empty() { + notes.push( + "no curated recipe for this task runs on this host either; `rocm model` lists the \ + whole catalog with its requirements" + .to_owned(), + ); + return Fix { + summary: summary.to_owned(), + commands: vec!["rocm model".to_owned()], + verify: format!("rocm diagnose --model {model_ref}"), + notes, + ..Fix::default() + }; + } + Fix { + summary: summary.to_owned(), + commands: alternatives + .iter() + .map(|alternative| format!("rocm serve {}", alternative.model_ref)) + .collect(), + // Whichever one the user picks, this is how they confirm it before + // committing to a download -- the command this whole feature exists to + // put in front of that decision. + verify: format!("rocm diagnose --model {}", alternatives[0].model_ref), + notes, + ..Fix::default() + } +} + +/// Curated models that would actually run here. +/// +/// The predicate is the whole readiness question, asked against the same host: +/// an alternative is only worth naming if asking about it would come back ready. +/// See [`assess`] for why the inner call is the one that does not recurse. +fn alternatives_for( + recipe: Option<&ModelRecipeRecord>, + registry: &ModelRecipeRegistry, + host: &HostFacts, + engine_for: &dyn Fn(&ModelRecipeRecord) -> HostEngineChoice, +) -> Vec { + let verdict_for = |candidate: &ModelRecipeRecord| { + assess( + &candidate.canonical_model_id, + &ModelCatalogSource::Available(registry), + host, + engine_for, + false, + ) + .verdict + }; + // Ready first. Degraded is a legitimate suggestion -- it runs -- but on a + // host where something runs well, saying so is better than offering a + // compromise. Blocked and Undetermined candidates are never offered at all: + // pointing a user at a second model they also cannot run is the failure this + // whole branch exists to avoid. + let ready = curated_alternatives(recipe, ®istry.recipes, &|candidate| { + verdict_for(candidate) == ModelVerdict::Ready + }); + let chosen = if ready.is_empty() { + curated_alternatives(recipe, ®istry.recipes, &|candidate| { + matches!( + verdict_for(candidate), + ModelVerdict::Ready | ModelVerdict::Degraded + ) + }) + } else { + ready + }; + chosen + .into_iter() + .map(|(candidate_ref, candidate)| ModelAlternative { + model_ref: candidate_ref.to_owned(), + required_gpu_memory_gib: candidate.min_gpu_mem_gb.map(f64::from), + engine: engine_for(candidate).engine, + }) + .collect() +} + +/// The word this verdict is reported under. +const fn verdict_label(verdict: ModelVerdict) -> &'static str { + match verdict { + ModelVerdict::Ready => "READY", + ModelVerdict::Degraded => "DEGRADED", + ModelVerdict::Blocked => "BLOCKED", + ModelVerdict::Undetermined => "UNDETERMINED", + } +} + +/// Render the human-facing model verdict. +/// +/// Deliberately laid out like `diagnose::render_report_text`: the same ` - ` +/// for evidence, ` $ ` for a runnable command and ` verify after fix: ` +/// for the check afterwards. Those prefixes are how anything post-processing the +/// report tells prose from commands, and a second report shape would quietly +/// exempt this one from it. +#[must_use] +pub fn render_model_readiness_text(readiness: &ModelReadiness) -> String { + use std::fmt::Write as _; + let mut out = String::new(); + let _ = writeln!( + out, + "rocm diagnose --model {}: {}", + readiness.model_ref, + verdict_label(readiness.verdict) + ); + if let Some(canonical) = &readiness.canonical_model_id { + let _ = writeln!(out, " recipe: {canonical}"); + } + for line in &readiness.evidence { + let _ = writeln!(out, " - {line}"); + } + if !readiness.alternatives.is_empty() { + let _ = writeln!(out, " what would run here instead:"); + for alternative in &readiness.alternatives { + let requirement = alternative.required_gpu_memory_gib.map_or_else( + || "no GPU minimum".to_owned(), + |value| format!("{} minimum", format_gib(value)), + ); + let _ = writeln!( + out, + " {} ({requirement}, {})", + alternative.model_ref, alternative.engine + ); + } + } + if let Some(fix) = &readiness.fix { + let _ = writeln!(out, " plan: {}", fix.summary); + for command in &fix.commands { + let _ = writeln!(out, " $ {command}"); + } + for note in &fix.notes { + let _ = writeln!(out, " note: {note}"); + } + if !fix.verify.is_empty() { + let _ = writeln!(out, " verify after fix: {}", fix.verify); + } + } + if let Some(route) = &readiness.route { + let _ = writeln!(out, " report it: {:>12}: {}", route.target, route.url); + } + // The recipe's own warnings come last and carry no verdict weight. Putting + // them above the plan would read as reasons for it. + for warning in &readiness.warnings { + let _ = writeln!(out, " recipe note: {warning}"); + } + out +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::builtin_model_recipe_registry; + + /// A recipe with a minimum far above anything a test host offers, so the + /// "would have been blocked" premise of the unreadable-catalog test cannot + /// quietly stop holding. + const LARGE_MODEL_REF: &str = "qwen3-32b-fp8"; + + /// The smallest curated recipe: 2 GiB minimum, 4 GiB recommended RAM. + const SMALL_MODEL_REF: &str = "qwen-smoke"; + + /// Stands in for the host's engine decision. The invariant under test here is + /// about verdicts, not about which engine was picked — that is I3, and it is + /// asserted where the real decision lives. + fn engine_choice(recipe: &ModelRecipeRecord) -> HostEngineChoice { + HostEngineChoice { + engine: recipe + .preferred_engines + .first() + .cloned() + .unwrap_or_else(|| "lemonade".to_owned()), + source: "test host".to_owned(), + unsupported_here: None, + } + } + + fn host_with(gpu_gib: f64, ram_gib: f64) -> HostFacts { + HostFacts { + accelerator_memory: AcceleratorMemory::Dedicated(gpu_gib), + system_ram_gib: Some(ram_gib), + } + } + + /// I1 — a catalog the CLI could not read never produces a compatibility + /// verdict. + /// + /// The host here is one that *would* be told the model does not fit: the + /// paired assertion below proves it, and without that pairing the + /// undetermined assertion would pass against a host incapable of producing + /// the wrong answer in the first place. The failure this guards against is + /// not "no verdict at all", it is an unreadable source being reported with + /// the same word a real incompatibility gets. + #[test] + fn an_unreadable_catalog_is_never_reported_as_an_incompatible_model() { + let registry = builtin_model_recipe_registry(); + let starved = host_with(0.0, 64.0); + + let readable = assess_model_readiness( + LARGE_MODEL_REF, + &ModelCatalogSource::Available(®istry), + &starved, + &engine_choice, + ); + assert_eq!( + readable.verdict, + ModelVerdict::Blocked, + "premise failed: this host must be one that a readable catalog reports as blocked, \ + otherwise the undetermined assertion below proves nothing. Got {:?} with evidence {:?}", + readable.verdict, + readable.evidence + ); + + let detail = "index /nonexistent/model-index.json could not be read"; + let unreachable = assess_model_readiness( + LARGE_MODEL_REF, + &ModelCatalogSource::Unreachable { + detail: detail.to_owned(), + }, + &starved, + &engine_choice, + ); + assert_eq!( + unreachable.verdict, + ModelVerdict::Undetermined, + "an unreadable catalog was reported as {:?}, which is a verdict about the model; \ + the CLI does not know anything about the model here. Evidence: {:?}", + unreachable.verdict, + unreachable.evidence + ); + assert_eq!( + unreachable.undetermined_reason, + Some(UndeterminedReason::CatalogUnreachable), + "the reason given was {:?}, not the unreachable source", + unreachable.undetermined_reason + ); + assert!( + unreachable + .evidence + .iter() + .any(|line| line.contains(detail)), + "the source failure `{detail}` appears nowhere in the evidence: {:?}", + unreachable.evidence + ); + assert_eq!( + unreachable.required_gpu_memory_gib, None, + "a requirement of {:?} was reported for a model whose recipe was never read", + unreachable.required_gpu_memory_gib + ); + } + + /// I2 — an alternative offered is never one that is itself not runnable here. + /// + /// Every alternative is fed back through the same assessment with the same + /// host, rather than through the filter helper with literal arguments: the + /// failure mode is the filter being handed the wrong host, not the filter + /// being wrong. + #[test] + fn every_alternative_offered_would_itself_run_on_this_host() { + let registry = builtin_model_recipe_registry(); + // Non-vacuity. The loop below asserts something about each alternative + // offered, so an assessment that offers none passes it while proving + // nothing -- which is what the first run of this test actually did + // against the stub. This host is one where the catalog's largest recipe + // cannot run and smaller ones can, so a correct assessment has to offer + // something here. + let crowded = host_with(24.0, 512.0); + let largest = assess_model_readiness( + "glm5", + &ModelCatalogSource::Available(®istry), + &crowded, + &engine_choice, + ); + assert!( + !largest.alternatives.is_empty(), + "a 905 GiB recipe on a 24 GiB host offered nothing that would work instead, so the \ + per-alternative assertions below check nothing. Verdict was {:?}: {:?}", + largest.verdict, + largest.evidence + ); + + for gpu_gib in [4.0_f64, 8.0, 24.0, 48.0] { + let host = host_with(gpu_gib, 512.0); + for recipe in ®istry.recipes { + let report = assess_model_readiness( + &recipe.canonical_model_id, + &ModelCatalogSource::Available(®istry), + &host, + &engine_choice, + ); + for alternative in &report.alternatives { + let offered = assess_model_readiness( + &alternative.model_ref, + &ModelCatalogSource::Available(®istry), + &host, + &engine_choice, + ); + assert!( + !matches!( + offered.verdict, + ModelVerdict::Blocked | ModelVerdict::Undetermined + ), + "on a {gpu_gib} GiB host, `{}` was offered as what would work instead of \ + `{}`, but asking about `{}` reports {:?}: {:?}", + alternative.model_ref, + recipe.canonical_model_id, + alternative.model_ref, + offered.verdict, + offered.evidence + ); + // Anchored against the host fact rather than against another + // assessment. The two assertions either side of this one ask + // `assess_model_readiness` whether what it offered is + // runnable, so they share a definition of "runnable" with the + // code under test: an implementation that compared the wrong + // memory figure everywhere would be wrong consistently, and + // they would both stay green. This one names GPU memory + // itself, so it fails when the comparison drifts onto another + // field. The hosts above pair a small GPU with 512 GiB of + // system RAM precisely so the two figures cannot be confused + // for one another. + if let Some(needs) = alternative.required_gpu_memory_gib { + assert!( + needs <= gpu_gib, + "on a {gpu_gib} GiB GPU, `{}` was offered instead of `{}`, but it \ + declares a need for {needs} GiB of GPU memory", + alternative.model_ref, + recipe.canonical_model_id, + ); + } + // The host above has ample system RAM, so nothing offered on + // it has an excuse to be merely degraded. Asserted separately + // from the invariant because the invariant has to keep + // holding on a RAM-starved host, where degraded is honest. + assert_eq!( + offered.verdict, + ModelVerdict::Ready, + "on a {gpu_gib} GiB host with ample system RAM, `{}` was offered instead \ + of `{}` but is only {:?}: {:?}", + alternative.model_ref, + recipe.canonical_model_id, + offered.verdict, + offered.evidence + ); + } + } + } + } + + /// Memory that could not be measured is not memory the host does not have, + /// and neither is the same as there being no GPU. Three inputs a two-state + /// `Option` would have collapsed into two answers. + #[test] + fn unmeasured_memory_absent_gpu_and_ample_memory_are_three_different_answers() { + let registry = builtin_model_recipe_registry(); + let catalog = ModelCatalogSource::Available(®istry); + let ask = |memory| { + assess_model_readiness( + SMALL_MODEL_REF, + &catalog, + &HostFacts { + accelerator_memory: memory, + system_ram_gib: Some(64.0), + }, + &engine_choice, + ) + }; + + let unknown = ask(AcceleratorMemory::Unknown); + assert_eq!(unknown.verdict, ModelVerdict::Undetermined); + assert_eq!( + unknown.undetermined_reason, + Some(UndeterminedReason::AcceleratorMemoryUnknown), + "a GPU whose memory could not be read must not be reported as one that is too \ + small: {:?}", + unknown.evidence + ); + + let absent = ask(AcceleratorMemory::None); + assert_eq!( + absent.verdict, + ModelVerdict::Blocked, + "a gpu_required recipe on a host with no GPU does not run, and there is no CPU \ + fallback: {:?}", + absent.evidence + ); + + assert_eq!( + ask(AcceleratorMemory::Dedicated(64.0)).verdict, + ModelVerdict::Ready + ); + } + + /// An APU's `amd-smi` telemetry names the wrong pool (the BIOS carve-out, + /// not what the engine allocates from), and there is no command that makes + /// the right pool readable today. This is `Undetermined`, not `Blocked` -- + /// the model might well fit -- and it must not be conflated with the + /// no-telemetry-at-all case, whose remediation (`amd-smi metric --json`) + /// would send an APU user in a circle forever. + #[test] + fn an_apu_with_no_readable_pool_is_undetermined_with_no_dead_end_command() { + let registry = builtin_model_recipe_registry(); + let catalog = ModelCatalogSource::Available(®istry); + let strix_halo = HostFacts { + accelerator_memory: AcceleratorMemory::UnifiedMemoryUnreadable, + system_ram_gib: Some(128.0), + }; + let readiness = assess_model_readiness("qwen3.6", &catalog, &strix_halo, &engine_choice); + assert_eq!( + readiness.verdict, + ModelVerdict::Undetermined, + "no readable figure means no verdict, not a confident one: {:?}", + readiness.evidence + ); + assert_eq!( + readiness.undetermined_reason, + Some(UndeterminedReason::UnifiedMemoryUnreadable), + "the reason has to name the APU-specific gap, not the generic unmeasured one: {:?}", + readiness.undetermined_reason + ); + assert!( + readiness + .evidence + .iter() + .any(|line| line.contains("no dedicated VRAM")), + "the report has to say why, in terms an APU user can act on: {:?}", + readiness.evidence + ); + assert!( + readiness.fix.is_none(), + "there is no command that makes this readable today; offering one would be a dead \ + end: {:?}", + readiness.fix + ); + } + + /// The sibling gap -- no telemetry at all -- keeps its own remediation. + /// Pinned alongside the APU case above so the two branches of the same + /// match arm cannot collapse back into each other unnoticed. + #[test] + fn no_telemetry_at_all_still_points_at_amd_smi() { + let registry = builtin_model_recipe_registry(); + let readiness = assess_model_readiness( + SMALL_MODEL_REF, + &ModelCatalogSource::Available(®istry), + &HostFacts { + accelerator_memory: AcceleratorMemory::Unknown, + system_ram_gib: Some(64.0), + }, + &engine_choice, + ); + assert_eq!(readiness.verdict, ModelVerdict::Undetermined); + assert_eq!( + readiness.undetermined_reason, + Some(UndeterminedReason::AcceleratorMemoryUnknown) + ); + let fix = readiness + .fix + .as_ref() + .expect("a genuine telemetry gap has a real next step: run amd-smi"); + assert!( + fix.commands + .iter() + .any(|command| command.contains("amd-smi")), + "the remediation for a true telemetry gap has to name the tool that would fill it: \ + {:?}", + fix.commands + ); + } + + /// Below the recipe's recommended system RAM it still runs, and saying it + /// does not would be wrong in the direction that costs the user the model. + #[test] + fn a_host_below_the_recommended_system_ram_is_degraded_not_blocked() { + let registry = builtin_model_recipe_registry(); + let readiness = assess_model_readiness( + SMALL_MODEL_REF, + &ModelCatalogSource::Available(®istry), + &HostFacts { + accelerator_memory: AcceleratorMemory::Dedicated(64.0), + system_ram_gib: Some(2.0), + }, + &engine_choice, + ); + assert_eq!( + readiness.verdict, + ModelVerdict::Degraded, + "GPU memory is ample and only the RAM recommendation is missed: {:?}", + readiness.evidence + ); + } + + /// An engine the platform rules out pre-empts the memory comparison. A model + /// that fits perfectly still will not start on an engine with no adapter + /// here, and reporting the memory verdict instead would be true and useless. + #[test] + fn an_engine_the_platform_rules_out_blocks_a_model_that_would_otherwise_fit() { + let registry = builtin_model_recipe_registry(); + let ruled_out = |recipe: &ModelRecipeRecord| HostEngineChoice { + unsupported_here: Some("vllm has no adapter on native Windows".to_owned()), + ..engine_choice(recipe) + }; + let readiness = assess_model_readiness( + SMALL_MODEL_REF, + &ModelCatalogSource::Available(®istry), + &HostFacts { + accelerator_memory: AcceleratorMemory::Dedicated(512.0), + system_ram_gib: Some(512.0), + }, + &ruled_out, + ); + assert_eq!( + readiness.verdict, + ModelVerdict::Blocked, + "512 GiB is ample for a 2 GiB recipe, so only the engine gate can be blocking it: \ + {:?}", + readiness.evidence + ); + assert!( + readiness + .evidence + .iter() + .any(|line| line.contains("no adapter on native Windows")), + "the refusal has to name the engine gate, not leave it reading as a memory problem: \ + {:?}", + readiness.evidence + ); + } + + /// A model the catalog does not carry is a gap in the catalog, not a fact + /// about the machine. It is also the one case where the CLI genuinely has + /// nothing to say, so it routes onward the way `rocm diagnose` does. + #[test] + fn a_model_outside_the_catalog_is_undetermined_and_routed_onward() { + let registry = builtin_model_recipe_registry(); + let readiness = assess_model_readiness( + "some-lab/never-curated-7b", + &ModelCatalogSource::Available(®istry), + &host_with(64.0, 64.0), + &engine_choice, + ); + assert_eq!(readiness.verdict, ModelVerdict::Undetermined); + assert_eq!( + readiness.undetermined_reason, + Some(UndeterminedReason::ModelNotCurated) + ); + assert!( + readiness.canonical_model_id.is_none() && readiness.required_gpu_memory_gib.is_none(), + "no recipe was read, so nothing may be reported about the model itself: {readiness:?}" + ); + let route = readiness + .route + .expect("an unanswerable question needs a route onward"); + assert!( + route.url.starts_with("http"), + "the route has to be somewhere the user can actually go: {route:?}" + ); + } + + /// The renderer is driven for real rather than having its shape assumed. + /// + /// The prefixes are a coupling `diagnose`'s renderer already has and anything + /// post-processing a report depends on: ` $ ` marks a runnable command + /// and ` - ` marks prose. A second report shape that looked similar but + /// differed would silently exempt this half of the output from it. + #[test] + fn the_rendered_answer_marks_commands_the_way_the_diagnosis_does() { + let registry = builtin_model_recipe_registry(); + let readiness = assess_model_readiness( + LARGE_MODEL_REF, + &ModelCatalogSource::Available(®istry), + &host_with(4.0, 64.0), + &engine_choice, + ); + assert_eq!(readiness.verdict, ModelVerdict::Blocked); + let rendered = render_model_readiness_text(&readiness); + assert!( + rendered.starts_with(&format!("rocm diagnose --model {LARGE_MODEL_REF}: BLOCKED")), + "the verdict has to lead: {rendered}" + ); + let commands = rendered + .lines() + .filter(|line| line.starts_with(" $ ")) + .count(); + assert_eq!( + commands, + readiness.fix.as_ref().map_or(0, |fix| fix.commands.len()), + "every command in the plan has to reach the report behind the same prefix a \ + diagnosis uses:\n{rendered}" + ); + for alternative in &readiness.alternatives { + assert!( + rendered.contains(&alternative.model_ref), + "`{}` was chosen as an alternative but never rendered:\n{rendered}", + alternative.model_ref + ); + } + } + + /// The defect a reviewer caught: a card's real VRAM total sits just under + /// its nameplate size (an 8 GiB card reports 8176 MiB, i.e. 7.9844 GiB), so + /// rounding to nearest prints "8.0 GiB" against an "8 GiB" recipe minimum -- + /// evidence that reads as "the host has exactly what it needs" on a verdict + /// that says BLOCKED. Flooring instead keeps the displayed figure honestly + /// below the minimum it failed to meet. Were this still rounding to + /// nearest, the assertion below would see "8.0 GiB", not "7.9 GiB". + #[test] + fn format_gib_floors_a_measured_figure_rather_than_rounding_up_to_the_minimum() { + let real_eight_gib_card_mib = 8176.0 / 1024.0; + assert_eq!(format_gib(real_eight_gib_card_mib), "7.9 GiB"); + } + + /// A whole-number figure (every `required`/`recommended` minimum, and any + /// measured figure that happens to land exactly on an integer) is + /// unaffected by the floor: it still renders with no decimal at all. + #[test] + fn format_gib_renders_a_whole_number_with_no_decimal() { + assert_eq!(format_gib(8.0), "8 GiB"); + } +} diff --git a/docs/architecture.md b/docs/architecture.md index b07b2f52c..bf201f4de 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -35,7 +35,7 @@ Subsystem modules already following full domain extraction (each owns its own ty ### `crates/rocm-core` — core library -Already-extracted subsystem modules include `diagnose.rs`, `examine.rs`, and several siblings following the same pattern. `lib.rs` itself is **not yet modularized** — see EAI-7768, planned last in the modularization effort: highest fan-in (every app and engine crate depends on it), but lowest novelty since the existing sibling modules already prove the pattern works. +Already-extracted subsystem modules include `diagnose.rs`, `examine.rs`, `model_readiness.rs` (the `rocm diagnose --model`/`rocm model --verbose` fit assessment: curated-recipe lookup against a host's measured GPU/RAM, shared between the two commands so they cannot disagree about whether a model fits), and several siblings following the same pattern. `lib.rs` itself is **not yet modularized** — see EAI-7768, planned last in the modularization effort: highest fan-in (every app and engine crate depends on it), but lowest novelty since the existing sibling modules already prove the pattern works. ### `crates/rocm-dash-core`, `rocm-dash-collectors`, `rocm-dash-daemon`, `rocm-dash-tui` — dashboard/telemetry diff --git a/docs/testing.md b/docs/testing.md index 545378c81..b4f9de30d 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -1241,3 +1241,30 @@ The install is covered by unit tests over the generated plan (`cargo test -p rocm --bin rocm wsl_rocdxg`). Running it end to end needs a WSL2 host with `/dev/dxg` and dxcore present, since the plan refuses before installing otherwise. + +## Model Fit Preflight + +`rocm diagnose --model ` answers whether a curated model will run on this +host before anything downloads. It reports one of four verdicts: `ready`, +`degraded` (runs, but under the recipe's recommended system RAM), +`blocked` (will not run here, with alternatives that would), or +`undetermined` (the CLI could not judge it — an unreachable catalog, an +unmeasured GPU, or a ref outside the curated set). + +```bash +rocm diagnose --model qwen-smoke --json # smallest curated recipe +rocm diagnose --model glm5 --json # largest curated recipe +``` + +On a host with a measured GPU, the smallest recipe should report `ready` (or +`degraded`, if this host's system RAM is below its recommendation) and name +the engine `rocm serve` would pick; the largest should report `blocked` with +at least one alternative that fits. On a host with no GPU visible to ROCm, +both report `blocked` with no fitting alternative. Either way, the command +must exit 0 — it is a query, not a check that only passes on a compatible +host — and must not populate the model-weight cache. + +The e2e suite (`cargo xtask e2e -- -n diagnose-2`) exercises all four verdicts, +including the two that need a synthetic signed catalog to trigger +deterministically (`ModelNotCurated`, `Degraded`) since no built-in recipe can +produce them on an arbitrary real host. diff --git a/skills/rocm-doctor/reference.md b/skills/rocm-doctor/reference.md index be7179abb..50670663b 100644 --- a/skills/rocm-doctor/reference.md +++ b/skills/rocm-doctor/reference.md @@ -16,7 +16,7 @@ tooling. - It's a general system inspector, so it **always exits 0**. The verdict is the `status` field: `ok` · `no-amd-gpu` · `wsl` · `unsupported-os` · `degraded`. -### `rocm diagnose [--symptom ""] [--top N] [--json]` +### `rocm diagnose [--symptom ""] [--top N] [--json] [--distro [NAME]] [--model REF]` Match the host + symptom against the closed catalog. **Always exits 0**; read the result from `--json`: @@ -35,6 +35,15 @@ the result from `--json`: covered, so this now fires only for a platform outside those three. It means nothing was checked — not a clean bill of health. - `route_when_no_match` — `{ target, url }` upstream tracker to use when `has_match` is false. +- `model` — set only when `--model ` was passed; otherwise not set. Answers + "will this curated model run here" from the recipe catalog and this host's + GPU/RAM, without downloading anything: `{ verdict, evidence[], engine, + required_gpu_memory_gib, available_gpu_memory_gib, alternatives[], + undetermined_reason, fix }`. `verdict` is one of `ready`, `degraded`, + `blocked`, `undetermined` — treat `undetermined` as "the CLI could not find + out", never as evidence the model is incompatible. `--model` is refused + together with `--distro`: the verdict is about the machine running the + command, not the one `--distro` names. ### `rocm fix [] [--yes] [--dry-run] [--device-index N]` diff --git a/tests/e2e-cucumber/features/diagnose.feature b/tests/e2e-cucumber/features/diagnose.feature index d33c22e74..cdc9bfed4 100644 --- a/tests/e2e-cucumber/features/diagnose.feature +++ b/tests/e2e-cucumber/features/diagnose.feature @@ -289,3 +289,95 @@ Feature: Diagnosing failures and listing fixes When the user previews that fix without applying it Then the preview states that the fix requires sudo and a re-login And the preview states that the CLI can run it automatically + # `--model` answers the question that comes before the other two: given this + # machine and that model, will it run. The point is that it answers in seconds + # and fetches nothing, so the user is not told by a download that failed. + # + # Host-agnostic in the same way diagnose-10 is, and for the same reason. The + # verdict depends on what this machine can measure of its own GPU, which + # differs per lane, so the scenario asks the CLI what it measured and then + # holds it to the matching half of the contract. Each half can fail, which is + # the bar an assertion has to clear: a lane that could not measure its GPU -- + # whether because there is no GPU at all, an engine this platform's gate + # rules out, or a GPU whose memory the CLI cannot read -- exercises the + # "told why, not that the model is incompatible" half, the GPU lanes the + # "measured and it does not fit" half. What holds everywhere is that a model + # no machine could serve is never called ready, and that asking costs no + # download. + @id:diagnose-model-too-large-is-refused-with-something-that-fits + Scenario: diagnose-21 - A model this machine cannot serve is refused before anything is downloaded + Given a user asking about a model no single machine could serve + When the user asks the CLI whether that model would run, in machine-readable form + Then the model is never reported as ready + And a machine that measured its GPU is told the model will not run, and what would + And a machine that could not measure its GPU is told why, rather than that the model is incompatible + And the human-readable answer names what would run instead + And no model weights were fetched + + # The other half of the verdict, and the one a user acts on: a model that does + # fit has to say which engine would serve it, because that is what `rocm serve` + # will pick and the user has no other way to know before starting it. + @id:diagnose-model-that-fits-is-ready-and-names-the-engine + Scenario: diagnose-22 - A model this machine can serve is reported ready, with the engine that would serve it + Given a user asking about the smallest curated model + When the user asks the CLI whether that model would run, in machine-readable form + Then a machine with enough measured GPU memory is told the model is ready + And the answer names the engine that would serve it + And a machine that could not measure its GPU is told why, rather than that the model is incompatible + + # The failure this guards is not an error, it is a WRONG ANSWER that reads like + # a real one. If a recipe catalog that cannot be read is scored as though it + # had been, the user is told their machine cannot run a model when the truth is + # that the CLI never found out what the model needs — and they go looking for + # hardware they may already have. Deterministic on every lane: the catalog + # source is pointed at a path that does not exist. + @id:diagnose-model-unreachable-catalog-is-not-an-incompatible-model + Scenario: diagnose-23 - A recipe catalog that cannot be read is not reported as an incompatible model + Given a machine that cannot reach the model recipe catalog + When the user asks the CLI whether that model would run, in machine-readable form + Then the CLI reports that it could not determine the answer + And the reason given is the unreachable catalog, not the model + And nothing is claimed about whether the model fits this machine + + # A model outside the curated catalog is not a model this CLI has judged + # incompatible -- it is one the CLI never had the metadata to judge at all. + # Folding the two together would tell a user "this will not run" about a + # model that might run fine, on the strength of nothing. Deterministic on + # every lane: the catalog is read successfully, it simply carries no recipe + # by this name. + @id:diagnose-model-not-curated-is-undetermined-not-blocked + Scenario: diagnose-24 - A model outside the curated catalog is undetermined, not blocked + Given a user asking about a model the curated catalog does not carry + When the user asks the CLI whether that model would run, in machine-readable form + Then the CLI reports that it could not determine the answer + And the reason given is that the model is not curated, not that it does not fit + And nothing is claimed about whether the model fits this machine + + # `degraded` exists so a model that runs, but below what the recipe + # recommends, is never folded into the same answer as one that will not run + # at all -- a user who is about to accept slower loading deserves a different + # word than one being turned away. The fixture recipe needs almost no GPU + # memory (so it clears the fit check on any lane that measured a GPU) but + # recommends more system RAM than any real test host has, so the RAM + # softening is the only thing left to trigger. A machine with no GPU still + # cannot serve it at all, so that half is asserted the same way diagnose-21 + # and diagnose-22 already do. + @id:diagnose-model-below-recommended-ram-is-degraded-not-blocked + Scenario: diagnose-25 - A model that runs below its recommended system RAM is degraded, not blocked + Given a user asking about a model that recommends far more system RAM than this host has + When the user asks the CLI whether that model would run, in machine-readable form + Then a machine with enough measured GPU memory to run it is told the model is degraded + And a machine that could not measure its GPU is told why, rather than that the model is incompatible + + # `--model` and `--distro` together used to be refused only when the probe + # happened to come back looking remote, so the refusal tracked a derived + # examination property rather than the flag itself. Keyed on the flag now: + # the refusal fires before any probe runs at all, so this holds even with no + # `wsl.exe` on PATH and no distribution installed -- every lane proves it, + # not only a WSL host. + @id:diagnose-model-with-distro-is-refused-not-answered-for-the-local-host + Scenario: diagnose-26 - Asking --model with --distro is refused before answering for the wrong machine + Given a user who asks --model together with --distro + When the user asks the CLI to diagnose with both flags + Then the CLI refuses and says --model answers for this machine, not the one --distro names + And no model verdict is reported diff --git a/tests/e2e-cucumber/tests/e2e/diagnose_steps.rs b/tests/e2e-cucumber/tests/e2e/diagnose_steps.rs index 2c59e6de1..5de2ca185 100644 --- a/tests/e2e-cucumber/tests/e2e/diagnose_steps.rs +++ b/tests/e2e-cucumber/tests/e2e/diagnose_steps.rs @@ -1133,3 +1133,676 @@ async fn assert_command_failure_reported_on_stderr(world: &mut E2eWorld) { "the command-failure explanation must not also be on stdout:\n{stdout}" ); } + +// ── `rocm diagnose --model` ──────────────────────────────────────── + +/// The largest curated recipe. Its 905 GiB minimum is above any single machine +/// the suite runs on, which is what makes the "will not run" half of +/// diagnose-21 hold on every lane rather than only on the small ones. +const OVERSIZED_MODEL_REF: &str = "glm5"; + +/// The smallest curated recipe (2 GiB minimum). Any machine that measured +/// dedicated GPU memory can serve it, so the "ready" half of diagnose-22 holds +/// on every discrete-GPU lane rather than only the Instinct one. An APU lane +/// does not qualify for that half even though `amd-smi` telemetry exists +/// there too: it names the BIOS carve-out, not the pool the engine allocates +/// from, so those hosts report no measured figure and land in the +/// "could not measure" half instead, alongside hosts with no GPU at all and +/// hosts with no telemetry whatsoever. +const SMALLEST_MODEL_REF: &str = "qwen-smoke"; + +/// A recipe index path that cannot exist, used to make the catalog source +/// genuinely unreachable rather than merely empty. Under the scenario's isolated +/// root so it is impossible for a runner to have planted one there. +fn unreachable_index_path(world: &E2eWorld) -> std::path::PathBuf { + world + .isolated_root + .as_ref() + .expect("no isolated root") + .path() + .join("no-such-recipe-index.json") +} + +/// A model-weight cache the scenario owns and that starts out absent, so +/// "nothing was fetched" is a question about a directory the runner cannot have +/// pre-populated. The shared `HF_HOME` the harness normally sets could not +/// answer it: it is deliberately shared across scenarios and already holds +/// weights. +fn scenario_weights_dir(world: &E2eWorld) -> std::path::PathBuf { + world + .isolated_root + .as_ref() + .expect("no isolated root") + .path() + .join("weights-must-stay-empty") +} + +/// Read the `model` section of a `rocm diagnose --model ... --json` report. +fn model_section(world: &E2eWorld) -> serde_json::Value { + let output = world.cli_output.as_ref().expect("no diagnose output"); + let report: serde_json::Value = + serde_json::from_str(output).expect("diagnose --json did not emit valid JSON"); + report + .get("model") + .cloned() + .filter(|value| !value.is_null()) + .unwrap_or_else(|| panic!("diagnose --model emitted no model section:\n{output}")) +} + +fn model_field<'a>(model: &'a serde_json::Value, field: &str) -> &'a serde_json::Value { + model + .get(field) + .unwrap_or_else(|| panic!("the model section has no `{field}`: {model:#}")) +} + +fn model_verdict(model: &serde_json::Value) -> &str { + model_field(model, "verdict") + .as_str() + .expect("verdict is not a string") +} + +/// Whether this host measured its own GPU memory. Everything downstream of it +/// differs per lane, so it is read back from the report rather than assumed. +fn measured_gpu_gib(model: &serde_json::Value) -> Option { + model_field(model, "available_gpu_memory_gib").as_f64() +} + +#[given("a user asking about a model no single machine could serve")] +async fn user_asks_about_an_oversized_model(world: &mut E2eWorld) { + world.model_name = Some(OVERSIZED_MODEL_REF.to_string()); + let weights = scenario_weights_dir(world); + world + .command_env + .push(("HF_HOME", weights.into_os_string())); +} + +#[given("a user asking about the smallest curated model")] +async fn user_asks_about_the_smallest_model(world: &mut E2eWorld) { + world.model_name = Some(SMALLEST_MODEL_REF.to_string()); +} + +#[given("a machine that cannot reach the model recipe catalog")] +async fn machine_cannot_reach_the_catalog(world: &mut E2eWorld) { + // The public key path is supplied too. Without it the CLI fails earlier, on + // the missing key rather than the missing index, and the scenario would be + // asserting about a different unreachable thing than the one it names. + let index = unreachable_index_path(world); + let key = index.with_extension("pem"); + world + .command_env + .push(("ROCM_CLI_MODEL_RECIPE_INDEX_PATH", index.into_os_string())); + world.command_env.push(( + "ROCM_CLI_MODEL_RECIPE_INDEX_PUBLIC_KEY_PATH", + key.into_os_string(), + )); + world.model_name = Some(SMALLEST_MODEL_REF.to_string()); +} + +#[when("the user asks the CLI whether that model would run, in machine-readable form")] +async fn user_asks_whether_a_model_would_run(world: &mut E2eWorld) { + let model = world.model_name.clone().expect("no model named"); + let (stdout, stderr, rc) = + crate::run_rocm_with_scenario_env(world, &["diagnose", "--model", &model, "--json"]); + world.cli_output = Some(stdout); + world.cli_stderr = Some(stderr); + world.cli_rc = Some(rc); +} + +#[then("the model is never reported as ready")] +async fn assert_model_is_never_ready(world: &mut E2eWorld) { + assert_eq!( + world.cli_rc, + Some(0), + "diagnose should exit 0 (it is a query)" + ); + let model = model_section(world); + assert_ne!( + model_verdict(&model), + "ready", + "the `{OVERSIZED_MODEL_REF}` recipe needs more memory than any single machine here has, \ + so no lane may call it ready: {model:#}" + ); +} + +#[then("a machine that measured its GPU is told the model will not run, and what would")] +async fn assert_measured_machine_is_blocked_with_alternatives(world: &mut E2eWorld) { + let model = model_section(world); + let Some(available) = measured_gpu_gib(&model) else { + return; + }; + assert_eq!( + model_verdict(&model), + "blocked", + "this machine measured {available} GiB against a 905 GiB recipe, so the answer is a \ + refusal and not anything softer: {model:#}" + ); + let alternatives = model_field(&model, "alternatives") + .as_array() + .expect("alternatives is not an array") + .clone(); + assert!( + !alternatives.is_empty(), + "a refusal with nothing offered instead leaves the user where they started: {model:#}" + ); + // The point of naming an alternative is that it runs HERE. One that does not + // fit either is worse than silence: it costs the user a second download to + // find out. + for alternative in &alternatives { + let required = alternative + .get("required_gpu_memory_gib") + .and_then(serde_json::Value::as_f64); + assert!( + required.is_none_or(|required| required <= available), + "`{}` was offered as what would run instead, but it needs {required:?} GiB and this \ + machine measured {available} GiB: {model:#}", + alternative + .get("model_ref") + .and_then(serde_json::Value::as_str) + .unwrap_or("") + ); + } +} + +#[then( + "a machine that could not measure its GPU is told why, rather than that the model is incompatible" +)] +async fn assert_unmeasured_machine_is_told_why(world: &mut E2eWorld) { + let model = model_section(world); + if measured_gpu_gib(&model).is_some() { + return; + } + // Four different machines land here and none of them deserves the same + // answer. One has no GPU at all, which is a fact and a refusal the CLI can + // stand behind. Another has an engine this platform's gate rules out + // before memory is even considered -- the gate runs first because a model + // that fits in memory still will not run on an engine with no adapter + // here, and reporting a memory verdict instead would be true and useless. + // A third has a GPU whose memory it could not read at all, which is a gap + // in what the CLI can see and must not be dressed up as a verdict about + // the model. The fourth is an APU: `amd-smi` telemetry exists, but it + // names the BIOS carve-out rather than the pool the engine allocates + // from, and there is no command that makes the right pool readable today + // -- so that gap earns its own reason, distinct from the no-telemetry-at- + // all case, and no dead-end remediation. None of the four may be reported + // as "this model is too big for you". + let evidence = model_field(&model, "evidence").to_string(); + match model_verdict(&model) { + "blocked" => { + // Two causes reach `blocked` with nothing measured: the engine + // platform gate, and no GPU being visible at all. They must not + // be told apart by sniffing the same evidence text a collapse + // would corrupt -- `fix.summary` is written by two independent + // call sites in `assess_model_for_host`, so a future bug that + // makes one cause's evidence drift into the other's still gets + // caught here instead of passing silently. + let fix_summary = model_field(&model, "fix") + .get("summary") + .and_then(serde_json::Value::as_str) + .unwrap_or_default(); + match fix_summary { + "serve this model from a platform that has the engine" => { + let engine = model_field(&model, "engine").as_str().unwrap_or_default(); + assert!( + evidence.contains(&format!("{engine} has no adapter")), + "the engine-platform refusal has to name the engine this host ruled \ + out, or the user cannot tell which one to avoid: {model:#}" + ); + assert!( + evidence.contains("WSL or Linux"), + "the engine-platform refusal has to offer the other-platform remedy, \ + or the user has nothing to act on: {model:#}" + ); + } + "make a GPU visible to ROCm, then ask again" => { + assert!( + evidence.contains("no GPU is visible to ROCm"), + "the GPU-visibility refusal has to name the absent GPU, or the user \ + reads this as a fact about the model: {model:#}" + ); + assert!( + model_field(&model, "fix") + .get("commands") + .and_then(serde_json::Value::as_array) + .is_some_and(|commands| commands + .iter() + .any(|command| command.as_str() == Some("rocm diagnose"))), + "a host with no GPU at all has a real next step (`rocm diagnose`); \ + losing it would strand the user: {model:#}" + ); + } + other => panic!( + "a machine that measured nothing reached a blocked verdict behind a fix \ + summary (`{other}`) this scenario does not know how to tell apart: \ + {model:#}" + ), + } + assert!( + model_field(&model, "available_gpu_memory_gib").is_null(), + "a machine that measured nothing must not report a figure it compared against: \ + {model:#}" + ); + } + "undetermined" => match model_field(&model, "undetermined_reason") + .as_str() + .unwrap_or_default() + { + "accelerator_memory_unknown" => { + assert!( + evidence.contains("could not be read"), + "the no-telemetry-at-all reason has to say the memory could not be read, or \ + the user reads it as a fact about the model: {model:#}" + ); + assert!( + !model_field(&model, "fix").is_null(), + "a host with no telemetry at all has a real next step (`amd-smi metric \ + --json`); collapsing this into the APU case would drop it: {model:#}" + ); + } + "unified_memory_unreadable" => { + assert!( + evidence.contains("no dedicated VRAM"), + "the APU reason has to name the missing dedicated-VRAM pool, or the user \ + reads it as a fact about the model: {model:#}" + ); + assert!( + model_field(&model, "fix").is_null(), + "there is no command that makes the APU's pool readable today; offering one \ + would be a dead end: {model:#}" + ); + } + other => panic!( + "a machine that could not measure its GPU reported an undetermined reason \ + `{other}` this scenario does not know about: {model:#}" + ), + }, + other => panic!( + "a machine that could not measure its GPU reported `{other}`, which claims more than \ + it knows: {model:#}" + ), + } +} + +#[then("the human-readable answer names what would run instead")] +async fn assert_human_answer_names_alternatives(world: &mut E2eWorld) { + let model = model_section(world); + if measured_gpu_gib(&model).is_none() { + return; + } + let expected = model_field(&model, "alternatives") + .as_array() + .and_then(|alternatives| alternatives.first().cloned()) + .and_then(|alternative| { + alternative + .get("model_ref") + .and_then(serde_json::Value::as_str) + .map(str::to_owned) + }) + .expect("the machine-readable answer offered no alternative to look for"); + // The same question again without `--json`: the structured answer being + // right is no use if the report the user actually reads does not carry it. + let (stdout, _, rc) = crate::run_rocm(world, &["diagnose", "--model", OVERSIZED_MODEL_REF]); + assert_eq!(rc, 0, "diagnose should exit 0 (it is a query)"); + assert!( + stdout.contains(&expected), + "the human-readable report never names `{expected}` as what would run instead:\n{stdout}" + ); +} + +#[then("no model weights were fetched")] +async fn assert_no_weights_were_fetched(world: &mut E2eWorld) { + // Two places a fetch would land: the model-weight cache the scenario pointed + // the CLI at, and rocm-cli's own artifact cache under the isolated data dir. + // Neither exists before the run, so their continued absence is not something + // the runner could have arranged in advance. + let weights = scenario_weights_dir(world); + assert!( + !weights.exists(), + "asking whether a model would run created {} -- the answer is supposed to cost no \ + download", + weights.display() + ); + let artifacts = world + .isolated_root + .as_ref() + .expect("no isolated root") + .path() + .join("data") + .join("models") + .join("artifacts"); + assert!( + !artifacts.exists(), + "asking whether a model would run populated the artifact cache at {}", + artifacts.display() + ); +} + +#[then("a machine with enough measured GPU memory is told the model is ready")] +async fn assert_fitting_model_is_ready(world: &mut E2eWorld) { + assert_eq!( + world.cli_rc, + Some(0), + "diagnose should exit 0 (it is a query)" + ); + let model = model_section(world); + let Some(available) = measured_gpu_gib(&model) else { + return; + }; + let required = model_field(&model, "required_gpu_memory_gib") + .as_f64() + .unwrap_or(0.0); + if available < required { + return; + } + // `degraded` is allowed alongside `ready`: a machine whose system RAM is + // below the recipe's recommendation still runs it, and calling that a + // failure would make the scenario a test of the runner's RAM. + assert!( + matches!(model_verdict(&model), "ready" | "degraded"), + "this machine measured {available} GiB against a {required} GiB recipe, so the model runs \ + here: {model:#}" + ); +} + +#[then("the answer names the engine that would serve it")] +async fn assert_answer_names_the_engine(world: &mut E2eWorld) { + let model = model_section(world); + // Named whatever the verdict, as long as a recipe was read: which engine + // would serve a model does not depend on whether it fits, and withholding it + // on a refusal is what would leave the user unable to check an alternative. + if model_field(&model, "canonical_model_id").is_null() { + return; + } + let engine = model_field(&model, "engine") + .as_str() + .unwrap_or_default() + .to_owned(); + assert!( + !engine.is_empty(), + "the answer names no engine, so the user cannot tell what `rocm serve` would start: \ + {model:#}" + ); +} + +#[then("the CLI reports that it could not determine the answer")] +async fn assert_catalog_failure_is_undetermined(world: &mut E2eWorld) { + assert_eq!( + world.cli_rc, + Some(0), + "diagnose should exit 0 (it is a query)" + ); + let model = model_section(world); + assert_eq!( + model_verdict(&model), + "undetermined", + "a catalog that could not be read says nothing about the model, so no verdict about the \ + model may be reported: {model:#}" + ); +} + +#[then("the reason given is the unreachable catalog, not the model")] +async fn assert_reason_is_the_catalog(world: &mut E2eWorld) { + let model = model_section(world); + assert_eq!( + model_field(&model, "undetermined_reason") + .as_str() + .unwrap_or_default(), + "catalog_unreachable", + "the reason must name the source, not the model: {model:#}" + ); + let index = unreachable_index_path(world); + let file_name = index + .file_name() + .expect("index path has no file name") + .to_string_lossy() + .into_owned(); + let evidence = model_field(&model, "evidence").to_string(); + assert!( + evidence.contains(&file_name), + "the evidence never names the source that could not be read ({}): {evidence}", + index.display() + ); +} + +#[then("nothing is claimed about whether the model fits this machine")] +async fn assert_nothing_claimed_about_fit(world: &mut E2eWorld) { + let model = model_section(world); + // The recipe was never read, so its requirement is not known. Reporting one + // anyway -- even a plausible one -- is how an unreachable source turns into + // a confident statement about the model. + for field in ["required_gpu_memory_gib", "canonical_model_id"] { + assert!( + model_field(&model, field).is_null(), + "`{field}` was reported for a model whose recipe was never read: {model:#}" + ); + } +} + +/// A ref shaped like a real model name but planted nowhere in the built-in +/// catalog. Deliberately not a nonsense string: the point of the scenario is +/// that a well-formed ref outside the curated set is still undetermined, not +/// that a malformed one is. +const UNCURATED_MODEL_REF: &str = "e2e-fixtures/not-a-curated-model"; + +#[given("a user asking about a model the curated catalog does not carry")] +async fn user_asks_about_an_uncurated_model(world: &mut E2eWorld) { + world.model_name = Some(UNCURATED_MODEL_REF.to_string()); +} + +#[then("the reason given is that the model is not curated, not that it does not fit")] +async fn assert_reason_is_not_curated(world: &mut E2eWorld) { + let model = model_section(world); + assert_eq!( + model_verdict(&model), + "undetermined", + "a model the catalog never carried is not a refusal, and must not be scored as one: \ + {model:#}" + ); + assert_eq!( + model_field(&model, "undetermined_reason") + .as_str() + .unwrap_or_default(), + "model_not_curated", + "the reason must name the missing metadata, or the user reads it as a fact about \ + whether the model fits: {model:#}" + ); +} + +fn workspace_root() -> std::path::PathBuf { + std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .parent() + .and_then(std::path::Path::parent) + .expect("e2e-cucumber must live under /tests") + .to_path_buf() +} + +fn xtask_command() -> std::process::Command { + if let Some(binary) = std::env::var_os("ROCM_XTASK_BINARY") { + std::process::Command::new(binary) + } else { + let mut command = std::process::Command::new( + std::env::var_os("CARGO").unwrap_or_else(|| std::ffi::OsString::from("cargo")), + ); + command.arg("xtask"); + command + } +} + +/// A ref that exists only in the synthetic catalog [`user_asks_about_a_ram_degraded_model`] +/// signs and points the CLI at. +const DEGRADED_MODEL_REF: &str = "e2e-fixtures/ram-degraded"; + +/// A recipe that needs almost no GPU memory -- so it clears the fit check on +/// any lane that measured a GPU at all -- but recommends more system RAM than +/// any real test host has, so the RAM softening is the only thing left that +/// can fire. Built fresh per scenario (mirrors the signed-catalog pattern in +/// `artifact_steps.rs`) because no built-in recipe can produce this +/// combination: every built-in recipe's RAM recommendation scales with its +/// GPU requirement, and `detect_system_ram_gib` has no env-var override to +/// fake the other side of the comparison. +#[given("a user asking about a model that recommends far more system RAM than this host has")] +async fn user_asks_about_a_ram_degraded_model(world: &mut E2eWorld) { + let root = world + .isolated_root + .as_ref() + .expect("no isolated root") + .path() + .to_path_buf(); + let index = root.join("degraded-recipes.json"); + let signature = root.join("degraded-recipes.json.sig"); + let public_key = root.join("degraded-recipe-public.pem"); + let private_key = root.join("degraded-recipe-private.pem"); + + let document = serde_json::json!({ + "schema_version": 1, + "source": "e2e-ram-degraded", + "recipes": [{ + "canonical_model_id": DEGRADED_MODEL_REF, + "aliases": [], + "task": "chat", + "source": "signed_recipe_index", + "revision": "main", + "loader": "transformers", + "trust_remote_code": false, + "dtype": "float16", + "device_policy": "gpu_required", + "min_gpu_mem_gb": 1, + "recommended_system_ram_gb": 999_999, + "artifacts": [], + "engine_recipes": [], + "manual_alternatives": [], + "featured": false, + "chat_template_mode": "auto", + // `lemonade`, not `vllm`: this scenario carries no platform tag and + // runs on every lane, including native Windows, where vLLM has no + // adapter and would be ruled out before the RAM softening this + // scenario exists to exercise ever runs -- turning the expected + // `degraded` into `blocked` on exactly the lane that also measures + // a GPU. `lemonade` is not ruled out on any lane this suite runs. + "preferred_engines": ["lemonade"], + "warnings": [] + }] + }); + std::fs::write( + &index, + serde_json::to_vec_pretty(&document).expect("failed to serialize recipe fixture"), + ) + .expect("failed to write recipe fixture"); + + let keygen = xtask_command() + .args(["keygen", "--private-out"]) + .arg(&private_key) + .arg("--public-out") + .arg(&public_key) + .current_dir(workspace_root()) + .status() + .expect("failed to run xtask keygen"); + assert!(keygen.success(), "xtask keygen failed"); + let sign = xtask_command() + .args(["sign", "--private-key"]) + .arg(&private_key) + .arg("--in") + .arg(&index) + .arg("--out") + .arg(&signature) + .current_dir(workspace_root()) + .status() + .expect("failed to run xtask sign"); + assert!(sign.success(), "xtask sign failed"); + + world + .command_env + .push(("ROCM_CLI_MODEL_RECIPE_INDEX_PATH", index.into_os_string())); + world.command_env.push(( + "ROCM_CLI_MODEL_RECIPE_INDEX_SIGNATURE_PATH", + signature.into_os_string(), + )); + world.command_env.push(( + "ROCM_CLI_MODEL_RECIPE_INDEX_PUBLIC_KEY_PATH", + public_key.into_os_string(), + )); + world.model_name = Some(DEGRADED_MODEL_REF.to_string()); +} + +#[then("a machine with enough measured GPU memory to run it is told the model is degraded")] +async fn assert_ram_short_machine_is_degraded(world: &mut E2eWorld) { + assert_eq!( + world.cli_rc, + Some(0), + "diagnose should exit 0 (it is a query)" + ); + let model = model_section(world); + let Some(_available) = measured_gpu_gib(&model) else { + // No GPU measurement means this lane cannot clear the fit check at + // all, so it lands on the "no GPU visible" or "memory unknown" halves + // that the reused unmeasured-machine step already covers -- not on + // degraded. + return; + }; + assert_eq!( + model_verdict(&model), + "degraded", + "this recipe needs 1 GiB of GPU memory (which any measuring lane clears) and recommends \ + 999999 GiB of system RAM (which no real host has), so the RAM softening is the only \ + path left, and blocked or ready are both wrong here: {model:#}" + ); +} + +// ── `--model` with `--distro` ────────────────────────────────────── + +#[given("a user who asks --model together with --distro")] +async fn user_asks_model_with_distro(world: &mut E2eWorld) { + world.model_name = Some(SMALLEST_MODEL_REF.to_string()); +} + +#[when("the user asks the CLI to diagnose with both flags")] +async fn user_diagnoses_with_model_and_distro(world: &mut E2eWorld) { + let model_ref = world.model_name.clone().expect("no model ref set"); + // No distro name given: the refusal must fire on the flag itself, before + // any probe that would need one to exist runs at all -- so this holds on + // a lane with no WSL and no `wsl.exe`, not only on a WSL host. + let (stdout, stderr, rc) = + crate::run_rocm(world, &["diagnose", "--model", &model_ref, "--distro"]); + world.cli_output = Some(format!("{stdout}\n{stderr}")); + world.cli_rc = Some(rc); +} + +#[then("the CLI refuses and says --model answers for this machine, not the one --distro names")] +async fn assert_model_with_distro_refused(world: &mut E2eWorld) { + let output = world.cli_output.clone().unwrap_or_default(); + let rc = world.cli_rc.expect("no exit code recorded"); + assert_ne!( + rc, 0, + "--model together with --distro must be refused, not answered:\n{output}" + ); + assert!( + output.contains("--model answers for the machine running this command") + && output.contains("--distro points the examination at a different one"), + "the refusal must say --model answers for this machine and --distro names another \ + one, not some other failure (e.g. wsl.exe missing, which would mean the refusal fired \ + too late -- after a probe attempt rather than on the flag itself):\n{output}" + ); + // The refusal must fire before any probe, so it must not also carry a + // probe failure (e.g. "wsl.exe was not found") -- that would mean the two + // flags together produced the right exit code for the wrong reason. + assert!( + !output.to_lowercase().contains("wsl.exe"), + "the refusal must preempt the distro probe entirely, not race it:\n{output}" + ); +} + +#[then("no model verdict is reported")] +async fn assert_no_model_verdict_on_refusal(world: &mut E2eWorld) { + let output = world.cli_output.clone().unwrap_or_default(); + let model_ref = world.model_name.clone().expect("no model ref set"); + // Not a bare `contains("rocm diagnose --model ")`: the refusal's own + // remediation text names that command (with a literal `` + // placeholder) as what to run instead, so that substring appears in a + // correct refusal too. A real verdict line always follows the command + // with the actual ref and a colon (`render_model_readiness_text`'s + // leading line); the placeholder never does. + let verdict_marker = format!("rocm diagnose --model {model_ref}:"); + assert!( + !output.contains(&verdict_marker), + "a refused request must not also report a model verdict for the wrong machine:\n{output}" + ); +}