Repository navigation
model : DeepSeek-V4.1-Flash-1017 pin on upstream deepseek41 (MXFP8 Engram + extras) - #332
danielhanchen wants to merge 5 commits into
Conversation
…am deepseek41 Upstream deepseek41 (ggml-org ggml-org#31086 and ggml-org#31137) now carries the model, so this keeps only what it does not have: - ggml : MXFP8 (E4M3 + E8M0 scale) block type 43, gather-only (CPU and CUDA GET_ROWS, gguf-py constants and (de)quantize), used by the Engram table of the published UD-Q3_K_XL / UD-Q4_K_XL / UD-Q8_K_XL GGUFs; converter writes the Engram tables bit-exact as MXFP8 (DSV41_ENGRAM_TYPE=bf16 for BF16) - Engram table always in host memory; token and Engram rows gathered in set_input (LLAMA_DSV41_HOST_GATHER=0 for the in-graph CPU GET_ROWS) - output projection through build_lora_mm (output LoRA) - ggml-cpu AMX: 2D weights only, batched src1 folded into M - chat: V4.1 chat template file, namespaced tool names in DSML parsing - fit: draft measured with its backend samplers; llama_sched_reserve - mtmd: deepseek41v placeholder entries skip the bf16 rounding - tests: MXFP8 GET_ROWS, softplus range, V4.1 template cases
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 80f0f5a8e4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| llama_model_params * mparams; | ||
| llama_context_params * cparams; | ||
| bool shares_model; | ||
| bool backend_sampling = true; // the speculative decoder offloads draft sampling to the backend (its default) |
There was a problem hiding this comment.
For DFlash/DSpark runs with params.speculative.draft.backend_sampling set to false, the sole common_fit_extra_model initializer in common/common.cpp still supplies only the original four fields, leaving this field true. Consequently, common_fit_draft_target installs top-k samplers during measurement even though common_speculative_impl_draft_dflash skips them at runtime. Fitting therefore budgets for a different compute graph and can unnecessarily reduce context size or GPU offload under a tight memory budget. Pass the configured draft backend-sampling value into the extra initializer instead of always using this default.
Useful? React with 👍 / 👎.
| } break; | ||
| case GGML_TYPE_MXFP8: | ||
| { | ||
| VALIDATE_ROW_DATA_E_E8M0_IMPL(block_mxfp8, data, nb); |
There was a problem hiding this comment.
When an MXFP8 tensor contains an E4M3 NaN code (0x7F or 0xFF) with a valid scale, this macro checks only the scale and incorrectly accepts the block. I reproduced this with a 33-byte block containing scale 127: ggml_validate_row_data returns true for both NaN codes, while dequantize_row_mxfp8 produces NaNs. Consequently, enabling check_tensors does not reject a corrupted Engram table, and gathering an affected row can propagate NaNs through inference. Validate the payload codes as well as the scale, matching the checks already present in the new conversion path.
Useful? React with 👍 / 👎.
Pin for DeepSeek-V4.1-Flash-1017 now that upstream carries the model: upstream
deepseek41(ggml-org#31086 and the follow-up ggml-org#31137) is kept verbatim, and this pin adds only what upstream does not have. It replaces the full-stack pin #298, which no longer merges once the base contains ggml-org#31137 (conflicts insrc/llama-kv-cache-dsv4.cpp,src/llama-model-saver.cpp,src/models/deepseek41.cpp,src/models/models.h).Base is
dsv41-pin-base61d= the pinned #61 commit (97611a1) with upstreamb11890merged in, the same layout asdsv41-pin-base61b/61c. #61 adds fork-local types 64..66 and this pin adds MXFP8, both on theGGML_TYPE_COUNTline, so this pin carries the combined enum (MXFP8 = 43,IQ1_XS..IQ1_XXXS = 64..66,GGML_TYPE_COUNT = 67) and sits after #61 inpr-set.json, as #298 did.Why MXFP8 is still needed
Upstream has no MXFP8 type (
GGML_TYPE_COUNT = 43), and a stock build rejects these files withtensor 'per_layer_token_embd.weight' has invalid ggml type 43. A header scan of every GGUF inunsloth/DeepSeek-V4.1-Flash-1017-GGUFfinds type 43 in exactly one tensor, the Engram tableper_layer_token_embd.weight, in:engram/DeepSeek-V4.1-Flash-1017-engram-MXFP8.gguf_fileUD-IQ1_S, UD-IQ1_M, UD-IQ2_XXS, UD-IQ3_XXS, UD-IQ3_S, UD-Q2_K_XL, the DSpark drafts, the mmproj files and the IQ4_NL / Q5_K / Q6_K swap-in Engram files have no type 43 and already load on upstream. MXFP8 stays at 43 because that is what the published files store.
What this pin keeps
GET_ROWS, gguf-py constants and (de)quantizeDSV41_ENGRAM_TYPE=bf16for BF16); the V4.1 chat template file is embeddedset_input(LLAMA_DSV41_HOST_GATHER=0restores the in-graph CPUGET_ROWS); output projection throughbuild_lora_mm-ngl/-ot; output LoRAns::f) in V4.1 DSML parsing; the shipped template plus tests next to the upstream onesllama_sched_reserveGET_ROWScases, softplus rangeEverything else from #298 is dropped in favour of the upstream code: Engram hash constants stay in
hparams.ple_*, the deterministic sort of the gathered top-k rows stays, and the per-device compressed-KV mirrors use upstream'scomp_read(theLLAMA_DSV41_KV_MIRRORknob is gone).Evidence
Simulated nightly merge (
pr-set.jsonwith #298 replaced by this head, all other pins unchanged):Pins refused in the same simulation for unrelated upstream changes: ggml-org#24423 and ggml-org#25731 (
src/llama-model-saver.cpp, upstream ggml-org#30880), #241 (src/llama-memory-hybrid-idx.cpp, upstream ggml-org#31141), and from b11930 #251 (common/common.cpp,common/fit.cpp, upstream ggml-org#21231). This pin does not touch them.Merged tree (all pins minus the refused ones above), CUDA sm_100 on B200:
Pin branch alone, CPU:
test-chatall passed,test-llama-archs142/142,test-backend-ops -o GET_ROWSincluding 9 MXFP8 cases.Tiny DeepSeek-V4.1 fixture (real dimensions, few layers, random weights),
llama-perplexity -c 128 --chunks 20scored against the current #298 build:Real models,
llama-perplexityon wikitext-2-c 512 --chunks 20, all GPU layers, scored with--kl-divergenceagainst the current #298 build (A); A again gives the run-to-run floor:invalid ggml type 43)Not covered here: ROCm, Vulkan, macOS, Windows and ARM legs (nightly only).