Skip to content

model : disallow integer dflash sliding_window_pattern - #26900

Merged
ggerganov merged 2 commits into
masterfrom
cisc/dflash-sliding-window-pattern-fix
Aug 12, 2026
Merged

model : disallow integer dflash sliding_window_pattern#26900
ggerganov merged 2 commits into
masterfrom
cisc/dflash-sliding-window-pattern-fix

Conversation

@CISC

@CISC CISC commented Aug 11, 2026

Copy link
Copy Markdown
Member

Overview

Nixes #26894

Additional information

DFlash model code could potentially incorrectly read the metadata. Disallow integer pattern.

Requirements

@github-actions github-actions Bot added the model Model specific label Aug 11, 2026
@CISC CISC changed the title fix sliding_window_pattern model : fix dflash sliding_window_pattern Aug 11, 2026
@200lz

200lz commented Aug 11, 2026

Copy link
Copy Markdown

For regression coverage, would it be useful to include both scalar and array encodings of attention.sliding_window_pattern and assert that they resolve to the same effective per-layer SWA configuration before DFlash binding?

That would cover the full semantic path:

serialization representation → normalized model configuration → target loading → drafter inheritance/binding

It would also avoid making either GGUF serialization form accidentally canonical when both are intended to represent the same effective pattern.

ggerganov
ggerganov previously approved these changes Aug 11, 2026

@pcuenca pcuenca left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @CISC, sorry for the oversight!

@CISC

CISC commented Aug 12, 2026

Copy link
Copy Markdown
Member Author

Thank you @CISC, sorry for the oversight!

Well, this changed mid-flight, easy to miss. :)

I need someone to confirm it actually works now though, not had time to test.

@pcuenca

pcuenca commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

hmm, on second look, in the case of glimmer we have this pattern: https://huggingface.co/meta-models/Muse-Glimmer-30B-assistant/blob/main/config.json#L14, which is replicated here: https://huggingface.co/meta-models/Muse-Glimmer-30B-GGUF/tree/main?show_file_info=dflash-kquant.gguf. So all layers are SWA in this drafter after all?

cc @zucchini-nlp

@CISC

CISC commented Aug 12, 2026

Copy link
Copy Markdown
Member Author

Reading over the issue again I think it may not actually be related (probably a case of a confused LLM), it states using the same DFlash GGUF for both (ie, the one where sliding_window_pattern was an array), it doesn't look like there ever publicly existed one with an integer.

@ggerganov
ggerganov dismissed their stale review August 12, 2026 08:59

I did approve the PR without looking much into the topic. Dismissing for now.

@CISC

CISC commented Aug 12, 2026

Copy link
Copy Markdown
Member Author

So all layers are SWA in this drafter after all?

With target_layers being [2, 14, 26, 38, 50] this should be correct.

@CISC

CISC commented Aug 12, 2026

Copy link
Copy Markdown
Member Author

it doesn't look like there ever publicly existed one with an integer.

It may make more sense to disallow this as integer for DFlash as a stepping pattern is unlikely to be useful.

The current implementation in master is wrong either way though as it would just fill the array with the integer.

@pcuenca

pcuenca commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

The current implementation in master is wrong either way though as it would just fill the array with the integer

Yes, I agree. I think this PR (or an equivalent solution) makes sense for future proofing, but I can't reproduce the original problem that was reported in #26894.

@ngxson

ngxson commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

yeah, I agree that this PR is still valid for future-proofing, it should be harmless for existing GGUFs

@CISC

CISC commented Aug 12, 2026

Copy link
Copy Markdown
Member Author

So it seems Muse Glimmer DFlash is ok after all, this PR should prevent any ambiguity though.

@CISC
CISC requested a review from ggerganov August 12, 2026 10:49
@CISC CISC changed the title model : fix dflash sliding_window_pattern model : disallow integer dflash sliding_window_pattern Aug 12, 2026
@ggerganov
ggerganov merged commit ece98b8 into master Aug 12, 2026
24 of 27 checks passed
@ggerganov
ggerganov deleted the cisc/dflash-sliding-window-pattern-fix branch August 12, 2026 11:24
gabe-l-hart added a commit to gabe-l-hart/llama.cpp that referenced this pull request Aug 12, 2026
Signed-off-by: Gabe Goodhart <ghart@us.ibm.com>

* origin/master: (383 commits)
  cmake :  introduce semantic versioning  (ggml-org#26839)
  gguf : harden loader against malformed tensor dims and metadata types (ggml-org#25596)
  kleidiai: Add runtime feature detection mechanism for aarch64/kleidiai (ggml-org#26076)
  model : disallow integer dflash sliding_window_pattern (ggml-org#26900)
  sync : ggml
  cmake : add config version support (ggml/1582)
  server : support slot save/restore with media inputs (ggml-org#26640)
  ui: add read_media tool (ggml-org#25877)
  opencl: default FA c8 cluster width to 16 on X1E (ggml-org#26433)
  tests : update speculative params (ggml-org#26925)
  vulkan: add TQ2_0 (ternary) support (ggml-org#25850)
  wavtokenizer-dec : bound posnet/convnext block_count against n_layer_all (ggml-org#26892)
  convert : handle per_layer_config in Gemma4 (transformers 5.15) (ggml-org#26882)
  opencl: use flat mv q5_k when weight exceeds image1d_buffer_t limit (ggml-org#26880)
  chat : fix muse-glimmer detection of tool calls after EOM (ggml-org#26879)
  ci : add missing release check (ggml-org#26923)
  CUDA: only disable CUDA graphs when mul_mat_id actually needs a stream sync (ggml-org#26802)
  cuda : add warp-per-row wkv7 kernel for single-token decode (ggml-org#26111)
  spec : update speculative-simple (ggml-org#26904)
  chat : tighten bare function parsing for Qwen models (ggml-org#26793)
  ...
huaxel pushed a commit to huaxel/CachyLLama that referenced this pull request Aug 12, 2026
* fix sliding_window_pattern

* disallow integer pattern
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

model Model specific

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants