diff --git a/src/vllm/model_executor/models/minimax_h3_gguf.cpp b/src/vllm/model_executor/models/minimax_h3_gguf.cpp index 3befe82f7..2fcca407f 100644 --- a/src/vllm/model_executor/models/minimax_h3_gguf.cpp +++ b/src/vllm/model_executor/models/minimax_h3_gguf.cpp @@ -345,13 +345,21 @@ MiniMaxH3GgufDit LoadMiniMaxH3DitFromGgufBf16(const GgufFile& file) { int64_t numel = 1; for (int64_t d : spec.shape) numel *= d; VT_CHECK(numel > 0, "minimax_h3 gguf bf16: tensor has an empty logical shape"); - // The two buffers the forward consumes on the HOST as f32 — rope.inv_freq - // (it builds the cos/sin cache) and, for a pruned checkpoint, adaln_t_table - // (it is interpolated at the per-row timestep) — must NOT be rounded into - // bf16 bits here: both are read through Ptr(), which is an unchecked - // cast, so a bf16 buffer is silently reinterpreted as garbage floats. See - // https://github.com/mudler/vllm.cpp/issues/244. - if (spec.name == "rope.inv_freq" || spec.name == "adaln_t_table") { + // The fp32 ISLANDS stay f32 here, consulted from the SINGLE source rather + // than re-listed. Two distinct hazards live behind this one predicate: + // + // * `rope.inv_freq` and `adaln_t_table` are read on the HOST through + // `Ptr()`, an unchecked cast — a bf16 buffer is reinterpreted as + // garbage floats, not converted (#244). + // * the patch projections, the time embedder and the output heads are + // upstream's fp32 island (minimax_h3_transformer.py:85-101). Those are + // correctly TYPED when bound, so they are not garbage, but rounding them + // here silently makes this path lower precision than every other loader. + // + // Re-listing names is what let those two drift apart: this loader named two + // of the seven and no gate noticed, because the rule was documented as + // binding on "all four staging paths" and this host loader is a fifth. + if (MiniMaxH3IsFp32IslandTensor(spec.name)) { out.storage[spec.name] = DequantGgufRowToF32(info.ggml_type, info.data, numel); out.shapes[spec.name] = spec.shape; continue; diff --git a/tests/vllm/models/test_minimax_h3.cpp b/tests/vllm/models/test_minimax_h3.cpp index ea90993ae..92166a598 100644 --- a/tests/vllm/models/test_minimax_h3.cpp +++ b/tests/vllm/models/test_minimax_h3.cpp @@ -3897,6 +3897,36 @@ TEST_CASE("minimax_h3: a ComfyUI-format GGUF loads into a runnable DiT") { CHECK(loaded.weights.blocks[0].qkv_proj.shape[0] == 3 * inner); CHECK(loaded.weights.blocks[0].qkv_proj.shape[1] == want.hidden_size); + // ★ THE fp32-ISLAND RULE BINDS THE bf16 HOST LOADER TOO (#244). The rule is + // single-sourced in MiniMaxH3IsFp32IslandTensor and its contract says every + // staging path must agree; this HOST loader was a fifth path that re-listed + // two of the seven names by hand, so five islands silently became bf16 and no + // gate noticed. Two hazards ride on the same predicate: rope.inv_freq and + // adaln_t_table are read through an unchecked Ptr() (bf16 bits + // reinterpreted as garbage floats), while the patch projections, the time + // embedder and the output heads are correctly TYPED but must not be silently + // down-converted on one path only. + { + const vllm::MiniMaxH3GgufDit bf16_loaded = vllm::LoadMiniMaxH3DitFromGgufBf16(gguf); + // Assert on the loader's OWN storage split rather than the bound views: this + // is the decision under test, and it is the map a tensor lands in that + // decides whether the forward later reads f32 bits or bf16 bits. + size_t islands = 0, streamed = 0; + for (const auto& kv : bf16_loaded.shapes) { + const std::string& iname = kv.first; + const bool is_island = vllm::MiniMaxH3IsFp32IslandTensor(iname); + INFO("tensor " << iname << (is_island ? " (fp32 island)" : " (bf16 stream)")); + CHECK(bf16_loaded.storage.count(iname) == (is_island ? 1u : 0u)); + CHECK(bf16_loaded.bf16_storage.count(iname) == (is_island ? 0u : 1u)); + if (is_island) ++islands; else ++streamed; + } + // The fixture carries both patch projections, the time embedder, both output + // heads and rope.inv_freq, so the island set is non-trivial -- a predicate + // that matched nothing would pass the loop above vacuously. + CHECK(islands >= 6); + CHECK(streamed > islands); + } + // And the whole thing must actually RUN: a real forward off GGUF-loaded weights. const MiniMaxH3PackedSequence packed = BuildMiniMaxH3PackedSequence( 4, 2, 4, 4, 2, 2, /*include_keyframe_cond=*/false, {}, 0);