[EXPERIMENT][WIP][OpenVINO] Fix DeepSeek-V3 YaRN attention-scale and RoPE-interleave bugs - #1877
Draft
mlukasze wants to merge 2 commits into
Draft
[EXPERIMENT][WIP][OpenVINO] Fix DeepSeek-V3 YaRN attention-scale and RoPE-interleave bugs#1877mlukasze wants to merge 2 commits into
mlukasze wants to merge 2 commits into
Conversation
…erleaved RoPE
Two correctness bugs in DeepseekPatcher.deepseek_v3_attn_forward caused
severely degraded generation quality for deepseek_v3 checkpoints using
YaRN rope_scaling with mscale_all_dim set (e.g. ByteDance-Seed/academic-ds-9B),
independent of weight precision (reproduced on FP16, INT8, and INT4 alike):
1. torch.nn.functional.scaled_dot_product_attention was called without
an explicit scale argument, silently falling back to the default
1/sqrt(head_dim) scale. DeepSeek-V3's MLA attention requires
self.softmax_scale, which additionally folds in a mscale-squared
correction factor whenever YaRN rope_scaling is used. Omitting it
under-scales attention logits (about 1.87x for this checkpoint's
YaRN config: factor=40, mscale_all_dim=1.0).
2. The local apply_rotary_pos_emb helper omitted the interleaved-pair
rearrangement step (view into (d/2, 2) pairs, transpose, reshape)
that DeepSeek-V3's own modeling_deepseek.py applies before rotate_half.
Without it, q_pe/k_pe rotary components use the wrong element pairing,
corrupting the positional part of MLA attention.
Verified: with both fixes applied, patched eager-PyTorch attention output
matches the original unpatched model's logits to about 1e-5 (float
precision noise) on a first-token test ("The capital of France is"
correctly predicts " Paris" again). Re-exporting FP16/INT8/INT4 OpenVINO
IR with the fix restores coherent, on-topic generation matching the
original HF checkpoint, resolving what had been misdiagnosed as a
quantization-induced accuracy regression.
deepseek_v2_attn_forward already had both of these correct; this brings
deepseek_v3_attn_forward in line with it.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- utils_tests.py: note that the existing 'deepseek' tiny fixture already exercises the YaRN mscale_all_dim code path fixed in the previous commit, and document why its qk_rope_head_dim=2 makes the interleaved-RoPE part of the fix a no-op at this fixture size (a standalone isolated-math reproduction script is provided separately for reviewers). - docs/source/openvino/models.mdx: note the YaRN attention-scale/RoPE fix next to the DeepSeek-V3 entry. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.
This PR was created by an AI agent as part of automated model enablement.
A human maintainer must review and approve it before it can be considered for merge.
Do NOT merge without human review and sign-off.
What does this PR do?
Fixes two independent correctness bugs in
DeepseekPatcher.deepseek_v3_attn_forward(
optimum/exporters/openvino/model_patcher.py) that silently corrupt MLA attention for anydeepseek_v3-architecture checkpoint that uses YaRNrope_scalingwithmscale_all_dimset(e.g. real DeepSeek-V3, and derivatives such as
ByteDance-Seed/academic-ds-9B).The two bugs
softmax_scale.torch.nn.functional.scaled_dot_product_attentionwascalled without an explicit
scale=argument, silently falling back to SDPA's default1/sqrt(head_dim)instead of the model's ownself.softmax_scale, which folds inDeepSeek-V3's YaRN
mscale-squared correction. ForByteDance-Seed/academic-ds-9B's YaRNconfig (
factor=40,mscale_all_dim=1.0) this under-scales attention logits by ~1.87x.apply_rotary_pos_embhelperomitted the
viewinto(d/2, 2)pairs + transpose + reshape that DeepSeek-V3's ownmodeling_deepseek.pyapplies beforerotate_half. Without it,q_pe/k_perotarycomponents use the wrong element pairing, corrupting MLA's positional attention component.
deepseek_v2_attn_forward(the sibling function for DeepSeek-V2-architecture models) alreadyhad both of these correct — bug (1) is also fixed there for consistency/correctness, since it is
present verbatim in that function too. This brings
deepseek_v3_attn_forwardin line with itssibling, which is strong internal-consistency evidence this was an oversight rather than an
intentional difference.
Verification
deepseek_v3_attn_forwardonto the live HF
AutoModelForCausalLMforByteDance-Seed/academic-ds-9B(a real, publicdeepseek_v3-architecture checkpoint). Before the fix: patched-attention logits differed fromthe original unpatched model by up to ~11.8 max absolute difference on a single forward pass.
After the fix: matches to ~1e-5 (float precision noise).
FP16 IR did not predict " Paris" anywhere in its top-10 tokens for "The capital of France is"
(HF correctly predicts it top-1). After the fix, the OV IR matches HF exactly (top-1 " Paris",
logit 34.15).
story completion, business/ERP passage, AI/technology passage) — all three precisions
(FP16/INT8/INT4) produce coherent, on-topic, semantically-correct text matching the raw HF
PyTorch reference after the fix; before the fix, all three produced garbled word-salad or
empty output on most prompts.
reproducing both bugs in isolation with realistic head dims (
qk_rope_head_dim=64,qk_nope_head_dim=128, YaRNfactor=40/mscale_all_dim=1.0matching this checkpoint's actualconfig) confirms a ~1.87x softmax-scale ratio and a large (>8.0 max-abs) RoPE divergence. This
script is available on request as CI-independent evidence for reviewers (kept out of the repo
diff per this pipeline's tiny-model/test conventions — it needs no fixture or network access).
Tests
tests/openvino/test_decoder.py::OVModelForCausalLMIntegrationTest::test_compare_to_transformers_5_deepseekalready runs the "deepseek" (
deepseek_v3) architecture through this exact code path — itsfixture (
optimum-intel-internal-testing/tiny-random-deepseek-v3) already setsrope_scaling={"type": "yarn", "mscale_all_dim": 0.707, "factor": 40, ...}and assertstorch.allclose(ov_outputs.logits, transformers_outputs.logits, atol=1e-4). With this fixapplied, that assertion passes (confirmed locally). However, we found and want to be fully
transparent that this existing test does not reliably regression-guard bug (2): the fixture's
qk_rope_head_dim=2makes the interleaved-pair rearrangement a mathematical no-op (a 1x2transpose+reshape is the identity permutation), and reverting the fix locally
(
git revert --no-commit) and re-running the same test still passes at this fixture size. Wetherefore did not add a new pytest unit test tightly coupled to specific tensor shapes/seeds (per
this pipeline's testing conventions, that would be a brittle, narrow addition); instead:
tests/openvino/utils_tests.pynext to the"deepseek"fixture entry, with a recommendation to bumpqk_rope_head_dim/qk_nope_head_dim(e.g. to 4-8) so this fixture would catch a future regression of bug (2) automatically. We do
not have upload access to
optimum-intel-internal-testingto make that fixture changeourselves.
check for both bugs, at realistic (non-degenerate) head dims.
test_compare_to_transformersforminicpm3(a sibling architecture sharing this samemodel_patcher.pyfile, though a different, untouched attention-forward function) as aregression check per this pipeline's "shared file" convention — it passes, confirming no
collateral regression from this change.
docs/source/openvino/models.mdx.WWB accuracy (context, not blocking this specific fix)
This fix improved CPU accuracy on both precisions but did not, by itself, bring
ByteDance-Seed/academic-ds-9Babove the pipeline's 0.90 similarity gate on either device — aseparate GPU-plugin-numerics issue and a possible INT8/INT4 quantization-calibration/MoE-routing
gap remain and are being tracked/handled independently in the enablement pipeline. That said,
this fix is independently correct and valuable regardless of those remaining issues: it corrects
attention/RoPE math that was objectively wrong for any YaRN-scaled
deepseek_v3(and, for bug 1,deepseek_v2) checkpoint, irrespective of quantization or device.Known warnings (acknowledged per this pipeline's quality checklist)
mutation:check_pr_quality.pyreports 27 pre-existing in-placeconfigmutations elsewherein
model_patcher.py(lines 1034-1064, 2480-2581) — none in the code touched by this PR.base_class: a recent upstream commit (e6f612a7, "Remove onnx dependency") touchedmodel_configs.py; not applicable here since this PR does not add or change anyOpenVINOConfig/base-class code.Installation instructions
Exporting cmd-line
optimum-cli export openvino -m ByteDance-Seed/academic-ds-9B ov_model --weight-format fp16Inference script
Before submitting