Repository navigation
mtmd: cast the attention mask to f16 once, not once per layer - #30241
Closed
Peterc3-dev wants to merge 2 commits into
Closed
Peterc3-dev wants to merge 2 commits into
Peterc3-dev wants to merge 2 commits into
Conversation
A same-type ggml_cast is still a full copy. Callers that cast the window mask to f16 once (qwen2vl) still paid one 2x(n_pos^2) f16 copy per windowed layer. On Qwen2.5-VL-3B at 1024x1024 (5476 patch tokens, 28 windowed layers) that is 28 CPY ops, about 34 ms of a 2.6 s encode on a Radeon 890M (Vulkan). Output is unchanged. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017V9bu3AeCUmiCUhbB8ab2u
With flash attention, build_attn cast the f32 mask to f16 on every layer. Models that pass the same mask to many layers through build_vit_opts (attn_mask or attn_mask_layers) paid one n_pos^2 copy per layer. Cast each distinct mask once before the layer loop; the per-layer cast in build_attn is now skipped because the mask is f16. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017V9bu3AeCUmiCUhbB8ab2u
|
Hi @Peterc3-dev, thanks for your contribution! Per our contribution guidelines, the automated PR checker found the following issue(s) that need your attention:
Please note that maintainers reserve the right to make final decisions on PRs. If you believe there is a mistake, please comment below. |
Collaborator
|
this is quite hacky, proper impl is to set f16 from input, same pattern as |
Collaborator
|
closing since this PR is not respecting AGENTS.md |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
With flash attention,
clip_graph::build_attncast the mask to f16 on every call. A same-typeggml_castis still a fullGGML_OP_CPY, so:qwen2vl.cppalready casts its window mask to f16 once, but each of the 28 windowed layers still ran a redundant f16→f16 copy of the wholen_pos²mask.build_vit_opts(attn_mask/attn_mask_layers) ran one f32→f16 cast per layer.Two small changes:
build_attn: only cast whenkq_mask->type != GGML_TYPE_F16.build_vit: cast each distinct mask once before the layer loop (deduplicated by tensor pointer), gated onflash_attn_type == CLIP_FLASH_ATTN_TYPE_ENABLED. The non-FA path is unchanged.Measured (Vulkan, AMD Radeon 890M / RDNA 3.5, RADV, Mesa 26.2.4)
Qwen2.5-VL-3B (Q4_K_M text, f16 mmproj), 1024×1024 image = 5476 patch tokens,
-ngl 99,--temp 0:Generated text is byte-identical. SmolVLM2-2.2B (goes through
build_vit, no mask) also byte-identical, same encode time.Not exercised on hardware here: a model that passes a mask via
build_vit_opts(deepseekocr2, mimo-audio, muse-glimmer). The change there is the same cast, moved earlier.🤖 Generated with Claude Code
https://claude.ai/code/session_017V9bu3AeCUmiCUhbB8ab2u