diff --git a/.agents/issue-index.md b/.agents/issue-index.md index 9a66f7d73..bcfb2d8b0 100644 --- a/.agents/issue-index.md +++ b/.agents/issue-index.md @@ -525,5 +525,6 @@ rather than merged. `scripts/check-agent-record.py` gates both. | [#1542](https://github.com/mudler/vllm.cpp/issues/1542) | `MODEL-MUSIC-minimax-music3-mini-max-music3-for-conditional-generation` | **MiniMax-Music3's flow-matching DiT is 62.24 % of a run and has never been profiled below the stage boundary.** `.agents/specs/minimax-music3.md` §20 (merge `f0396049d`, `rc` job `c206ec87` on an idle `thor:gpu0`, checkpoint staged to local disk) prices the developer's 20 s / 30 steps configuration: `denoise.dit_device` **370.556 s / 62.24 % / 120 calls**, against `vocoder.decode_window` 122.169, `ar.lm_decode_step` 56.174 and `ar.depth_forward` 21.099. The DiT **did not move** — 370.556 s against §15.7's 370.746 s over an identical 120 calls — while everything around it got 3.45x to 18.9x faster, so it went from ~3 % of a run to 62 % without changing. §15.2's "the GPU is not the problem, and no further DiT work will move this number" is recorded as INVERTED by §20.5 and must not be resurrected: it was true of a 4 s / 4-step clip whose wall was 69.5 % CIFS load and 23.4 % host depth decoder. `VLLM_CPP_MUSIC3_PROFILE=1` reports ONE bucket for the whole forward (`minimax_music3_speech.cpp:321`), so everything known about those 370.556 s is arithmetic on the outside of a black box: ~1.544 s per forward, ~3.33 TFLOP of block-stack GEMM at seq ~690, hence **~2.2 TFLOP/s** and 44x above the 9.66 GB weight-read floor, with no measurement of which of the GEMMs, `vt::AttentionCross`, the per-call cuBLASLt descriptor and heuristic rebuild, or the host packing owns it. **The dtype is NOT the gap and is settled rather than assumed**: at the pinned diffusers oracle `c6da9936`, `scripts/convert_minimax_music3_to_diffusers.py:267` defaults `--dtype float32` and `:208` applies it to the transformer while `:214` forces ONLY the RVQ depth decoder to bf16, `denoise.py:83` casts the condition to `transformer.dtype`, and the released `transformer/` shard header reports `F32` for all 231 tensors — so a bf16 or TF32 DiT would be a divergence from the oracle rather than a repair of one. Scoped as: intra-forward profiler SPANS behind a second opt-in (`VLLM_CPP_MUSIC3_DIT_SPANS=1`, so the default path stays byte-for-byte what §20 timed and prior tables stay comparable), a cuBLASLt ceiling probe at the DiT's own shapes on `thor:gpu0`, then the lever the evidence names with correctness first at the UNCHANGED `kDitRelTol` / `kDitAbsFloor` / `kDitMeanAbsTol` bounds. Spec §21 of [`minimax-music3.md`](specs/minimax-music3.md). Follows [#1512](https://github.com/mudler/vllm.cpp/issues/1512) | perf | | [#1555](https://github.com/mudler/vllm.cpp/issues/1555) | `MODEL-MUSIC-minimax-music3-mini-max-music3-for-conditional-generation` | **`vt::AttentionCross`'s CUDA kernel does 4.0 % of the MiniMax-Music3 DiT's arithmetic in 43.9 % of its time.** Measured on an idle `thor:gpu0` (sm_110, driver 595.78, `uptime` 3.46 with 0 logins at job start), `rc` job `0f95377f-70dd-4bf8-93b5-8e44fd762713`, checkpoint staged to local disk with `SRC_BYTES == DST_BYTES == 28517617303` and `findmnt -T /tmp/ckpt` reading `overlay`, at tree `0e18f8afd` with the intra-DiT spans of [#1542](https://github.com/mudler/vllm.cpp/issues/1542). At the shipped geometry (`dit.seq_sum / 16 = 690`, window 689 latent frames, 36 blocks, inner 2048, ff 8192) `denoise.dit_device` is 25.104 s over 16 forwards and the sixteen spans sum to 25.100 s, a 99.98 % partition: **`dit.attn` 11.010 s = 43.9 % at 0.140 TFLOP per forward = 0.204 TFLOP/s**, against the four `vt::MatmulBT` GEMMs' 13.390 s = 53.3 % at 3.334 TFLOP = **3.98 TFLOP/s**, with norms, rope, SiLU, packing and readback 0.700 s = 2.8 %. **The attention kernel is 19.5x slower per flop than the GEMMs beside it, on the same tensors, in the same forward, on the same device.** At the developer's 20 s / 30 steps that is 0.6881 s x 240 forwards = **165.1 s of the 370.510 s `denoise.dit_device` bucket and 27.6 % of the whole 598.207 s run**. MECHANISM AS A HYPOTHESIS, not a measurement: `AttentionCrossFlashKernel` (`src/vt/cuda/cuda_attention_cross.cu`) gives one warp one query row and pays a **five-step `__shfl_xor_sync` butterfly per key** for 64 useful flops (`npl = 2` at `head_dim` 64), launching `grid(44,32)` of 512 threads with `bc = 64` and 32 KiB dynamic shared memory, so occupancy is bounded near 32 warps per streaming multiprocessor — about 8 per scheduler, under what hides a 40-60 cycle dependent chain; an instruction count over 22 080 warps x 690 keys predicts ~1.8 ms per layer against a measured 19.1 ms, and every CTA re-reads the whole of K and V for 17.9 GB per forward (~65 ms, ~10 %). **No `ncu` counter was read on either side and no occupancy was measured**, so the split is measured and its attribution is not. DIRECTION, NOT A DESIGN: amortize the reduction — a lane per key so the dot product is sequential in registers, or a query TILE per warp as FlashAttention-2 does. Both reorder the head-dim summation so neither is bit-identical, which the op's contract already allows (`cuda_attention_cross.cu` NUMERICS note), and admitting one requires BOTH consumers to hold their EXISTING tolerances — this DiT at `kDitRelTol` 1e-4 / `kDitAbsFloor` 5e-5 / `kDitMeanAbsTol` 5e-6 (§14.4) and LTX-2.5, which reaches the op six times per layer. **NOT a precision change**: the DiT's fp32 is upstream's resolved choice at diffusers `c6da9936` (`convert_minimax_music3_to_diffusers.py:267` defaults `--dtype float32`, `:208` applies it, `:214` forces only the RVQ depth decoder to bf16, `denoise.py:83` casts into it, and the released `transformer/` shard header is `F32` for all 231 tensors), and cuBLASLt's fp32 emulation is separately MEASURED UNAVAILABLE here (`CUBLASLT_MATMUL_DESC_EMULATION_STRATEGY` undefined in nvcc 13.0.88, `PROBE_BUILD_RC=2`). Found by [#1542](https://github.com/mudler/vllm.cpp/issues/1542); listed under `## Owed` in §21.7 of [`minimax-music3.md`](specs/minimax-music3.md) | perf | | [#1563](https://github.com/mudler/vllm.cpp/issues/1563) | `GATE-SQUASH-SEPARATOR` | **A markdown `---` horizontal rule anywhere in a pull request body silently voids the trailer block, and `check-commit-trailers.py` blames the trailers instead of the framing.** Found 2026-08-21 writing the body for PR [#1550](https://github.com/mudler/vllm.cpp/pull/1550) ([#1542](https://github.com/mudler/vllm.cpp/issues/1542)). `parsed_trailers()` shells out to git's trailer parser, and **git treats a line of exactly `---` as the start of the patch section**, so everything after the first one is not part of the message and a trailer block below it is invisible. Reproduced with no repository state: a body of `subject / prose / --- / more prose / FOLLOWING_AGENTS_PROTOCOL / the three trailers` reports `[trailers] Following-Agents-Protocol must appear exactly once` and `[attribution] AI-Assisted must appear exactly once`; `sed -i '/^---$/d'` on that same file reports `OK: commit trailer contract`, and the `---` is the only difference. **The MESSAGE is the defect, not only the behaviour**: `Following-Agents-Protocol` appears EXACTLY ONCE in the body while the checker says it must appear exactly once, so a reader counts occurrences, finds one, counts again and dumps bytes before thinking to test the parser's own framing. `_strict_errors` already computes `_paragraphs(body)[-1]` correctly as the three trailers verbatim, so the checker holds the information needed to say "the trailer paragraph is present but git could not parse it; a `---` line at line N ends the message". Worse, the neighbouring `FOLLOWING_AGENTS_PROTOCOL must appear exactly once as a separate paragraph before the trailer paragraph` check stays SILENT, so the two errors that fire both point away from the cause. **Beyond one confusing message**: the repository sets `squash_merge_commit_message = PR_BODY`, so the body IS the landed commit message, and a body carrying a `---` lands a commit whose trailers `git interpret-trailers` cannot see, on a branch that is never force-pushed. Same permanent-damage shape AGENTS.md records for the `---------` separator GitHub wrote under `COMMIT_MESSAGES`, arriving from the AUTHOR side rather than the forge side. `scripts/agent-pr-body.py --pr ` DOES catch it and caught it here before the merge; the exposure is a body never passed through that command, which AGENTS.md notes is not a gate and cannot be one because it reaches the network, while the CI guard reads the frozen `pull_request` payload and so does not re-read a body edited after the final push. NOT FIXED HERE: it changes a checker's semantics and its message, so under `## Changing the rules or a checker` it needs its own row, a red-before test and green-after evidence. Two candidate repairs, neither chosen: name the `---` line, or strip patch-section framing before parsing so a markdown rule is inert -- the second changes what the contract accepts and is the larger decision. Suggested minimum: `tests/scripts/test_check_commit_trailers.py` gains a case pinning the reproduction above | bug | +| [#1575](https://github.com/mudler/vllm.cpp/issues/1575) | `SPEC-DFLASH2` | **`build-newest-gcc` has been RED on `main` since `5702d8f83`, and it is a recurrence of the class `tests/support/process_id.h` was created to close, not a missing include.** `tests/vllm/models/test_qwen3_dflash2_gguf.cpp:547` calls `::getpid()` while including nothing that declares it; the lane builds in a `gcc:16` container where libstdc++ no longer supplies the transitive ``, so it fails to COMPILE (`error: '::getpid' has not been declared; did you mean 'getpt'?`) while every local toolchain at gcc 13 stays green. The seam's own header predicts this: it says the spelling "was fixed once in three files and came back in five more, because each new loader test copies the temp-directory helper from the last one", which is exactly what W5's GGUF test did. The convention is otherwise unanimous — every other file in the tree naming `getpid` either includes `` or uses the helper, and the sibling `tests/vllm/entrypoints/test_dspark_draft_routing.cpp` uses the helper AND carries a comment warning not to reintroduce this class. ATTRIBUTED, not inferred: `build-newest-gcc` is absent from the baseline's failed list at `92406c620` and present at `5702d8f83`, the commit that added the file. MEASURED red-first in the lane's own container at base `947e5f648`, file sha256 `83bba319…`: unmodified `RED_RC=1` with the error byte-identical to CI; with the seam applied `GREEN_RC=0`, `compile_err=0`, `git diff --stat` confirming the edit applied; local gcc 13 `GCC13_RC=0`, so no regression on the shipped toolchain. FIXED IN FLOW with the portable spelling (`#include "support/process_id.h"`, `vllm_test::ProcessId()`) rather than ``, which would work on POSIX but re-copies the idiom the helper centralises and does not compile on MSVC. Found while measuring [#1464](https://github.com/mudler/vllm.cpp/issues/1464) at `origin/main` | bug | | [#1454](https://github.com/mudler/vllm.cpp/issues/1454) | `SPEC-MTP-GGUF` | **`test_qwen3_5_gguf_mtp.cpp` reported `Status: SUCCESS!` with `assertions: 0` on every CI run, and its one arithmetic guarantee was a tautology.** Both cases opened `if (path == nullptr) return;` on `VLLM_MTP_GGUF_MODEL`, and a bare `return` from a doctest case is a PASS: re-derived on a clean Release build at `947e5f648`, unset, the file printed `test cases: 2 \| 2 passed \| 0 failed \| 0 skipped`, `assertions: 0`, `Status: SUCCESS!`, exit 0, and printed nothing else. The variable is set nowhere in `.github/workflows/`, so that was the state of every run. Second defect in the same file: the comment at `:52` stated `num_hidden_layers + depth == block_count` and the line under it asserted `CHECK(c.num_hidden_layers > 0)`, true of every valid model. MEASURED, not argued: mutating `src/vllm/model_executor/models/qwen3_5_gguf_weights.cpp:889` to `c.num_hidden_layers = block_count;` compiled clean and left the file at 2/2 cases, 0 assertions, `SUCCESS!`, exit 0. FIXED IN FLOW. The invariant is now pinned **HERMETICALLY** on KV-only synthetic GGUFs carrying no weight bytes, so CI checks it every run rather than never - 65/1 (the shipped Qwen3.8-27B pair), 25/1 (the Qwen3.5-2B reference this suite was developed against) and 28/3, the third arm separating `- nextn` from `- 1` - plus a head-less arm asserting the key is NOT published, which is the half `NumMtpLayers` cannot express because it answers 1 for an absent key. The two env-gated cases stay, now skipping with a `MESSAGE` naming the variable as `test_gguf_mmproj_reach.cpp` does, and the live one re-derives the invariant from the file's own `block_count` kv. Unset 4 cases / 18 assertions / `SUCCESS!` / rc 0; live on `Qwen3.8-27B-Q4_K_M.gguf` 4 / 38 / `SUCCESS!` / rc 0. Both mutants now red (9/18 and 5/18, exit 1), compiled clean, restored against a pre-taken sha256. **The production line is CORRECT and was not touched**: `block_count - nextn` landed `1a4db5c3c`, the `mtp_num_hidden_layers` republication `493327b4e`. Related but distinct: [#821](https://github.com/mudler/vllm.cpp/issues/821) W2 (`0adeb8b0e`) pins the same arithmetic for the 27B artifact on a committed manifest in `tests/vllm/models/test_qwen38_27b_gguf_manifest.cpp`, and that gate DOES catch both mutants - so the invariant was not globally unpinned, it was unpinned in this row's own file | bug | | [#1434](https://github.com/mudler/vllm.cpp/issues/1434) | `GATE-DOC-CHECKPOINT-STATES` | **`scripts/check-doc-checkpoint.py` could not see `PARTIAL`, so 118 state cells could move with no gate observing them.** `STATES` (`:56-66`) is the whole definition of what a lifecycle state IS for the gate that enforces AGENTS.md's `docs/STATUS.md` / `docs/BENCHMARKS.md` / spec `## Now` triple, and `row_states` drops any row it cannot match. `lifecycle_moves` and `moved_rows` then iterate the AFTER map, so leaving the matched set is silent by construction. Re-derived at `947e5f648` (the report measured `63d87805c`): `PARTIAL` **118** cells and `ANCHOR-BACKFILL` **73**, against `DONE` 77 and `BLOCKED` 9 — `PARTIAL` is the second most used state in the matrices and the gate was blind to it. Over the seven tables `ROW_TABLES` actually reads, the resolved population goes from **153 rows to 226**, a 47.7 % widening. Two of the transitions the report names behave differently from its description, measured with scratch commits at `947e5f648` on an unmodified checker: `READY -> PARTIAL` rc **0** and `PARTIAL -> READY` rc **0** are the real blind spots, while the report's suggested `PARTIAL -> ACTIVE` already reds — by accident, reporting **`added as ACTIVE`** for a row that has existed for months, because it is absent from the BEFORE map. FIXED IN FLOW for `PARTIAL` only. **`ANCHOR-BACKFILL` is deliberately excluded**: `.agents/feature-matrix.md:14-17` defines it as a property of the RECORD (*a legacy implemented row without exact code, test and real-spec anchors*), `docs/STATUS.md` carries no such term and would have nothing true to write on a `DONE <-> ANCHOR-BACKFILL` move, and `REQUIRED["lifecycle"]` cannot demand the spec's `## Now` alone — so admitting it would demand a public-document edit with nothing to say, which is the exact shape `check-doc-checkpoint.py:4-17` records as the reason the file was rewritten (16 of 20 red CI runs, six hardcoded escape hatches). One row's resolved state moves and the move is a REPAIR: `KV-BLOCK-POOL` says `` `PARTIAL` (not `DONE`) `` in its prose and the last-match heuristic believed the parenthesis, resolving `DONE`. No pinned counter moves — `check-gate-commands.py` has its own `GATED_STATES` and `RUNNABLE_BASELINE` is keyed on matrix rows, `UNOWNED_HIGH_WATER` is unmoved because this row names an owner, and no matrix row or public document changes — which was measured, not assumed, because this is the [#1376](https://github.com/mudler/vllm.cpp/issues/1376) ratchet shape. Remainder listed under `## Owed` in [doc-checkpoint-lifecycle-states.md](specs/doc-checkpoint-lifecycle-states.md): `ANCHOR-BACKFILL` moves, `.agents/sglang-matrix.md` never entering `ROW_TABLES`, a row that leaves the matched set entirely, and a new row added directly as `PARTIAL` | bug | diff --git a/.agents/specs/dflash2-spec-decode.md b/.agents/specs/dflash2-spec-decode.md index 7e704ded9..a1f67dc23 100644 --- a/.agents/specs/dflash2-spec-decode.md +++ b/.agents/specs/dflash2-spec-decode.md @@ -3,6 +3,9 @@ **Row:** `SPEC-DFLASH2` (engine-matrix, speculative decoding). **Issue:** [#1314](https://github.com/mudler/vllm.cpp/issues/1314). **Predecessor:** [dflash-spec-decode.md](dflash-spec-decode.md) (`SPEC-DFLASH`, DONE). +**In-flow fix:** [#1575](https://github.com/mudler/vllm.cpp/issues/1575) — W5's `tests/vllm/models/test_qwen3_dflash2_gguf.cpp` called `::getpid()` directly instead of the +portable `tests/support/process_id.h` seam, which held `build-newest-gcc` red on `main` from +`5702d8f83`. Fixed in flow; not owed. **Kind:** structured spec. No production code lands with this spec; the pull request shape for this row is SEPARATE spec and implementation pull requests, recorded at row claim on 2026-08-19. diff --git a/tests/vllm/models/test_qwen3_dflash2_gguf.cpp b/tests/vllm/models/test_qwen3_dflash2_gguf.cpp index de00dc735..511d06801 100644 --- a/tests/vllm/models/test_qwen3_dflash2_gguf.cpp +++ b/tests/vllm/models/test_qwen3_dflash2_gguf.cpp @@ -64,6 +64,7 @@ #include #include "../gguf_builder.h" +#include "support/process_id.h" #include "vllm/model_executor/model_loader/gguf_reader.h" #include "vllm/model_executor/model_loader/safetensors_reader.h" @@ -544,7 +545,7 @@ class ScratchSafetensors { static int counter = 0; dir_ = fs::temp_directory_path() / ("vllmcpp_df2gguf_st_" + std::to_string(counter++) + "_" + - std::to_string(static_cast(::getpid()))); + std::to_string(static_cast(vllm_test::ProcessId()))); fs::create_directories(dir_); json header = json::object(); std::string payload;