Repository navigation
[ROCm] Disable AITER FP8/FP4 BMM for MLA by default - #53123
Conversation
|
Documentation preview: https://vllm--53123.org.readthedocs.build/en/53123/ |
Signed-off-by: xuebwang-amd <xuebwang@amd.com>
246fc92 to
5683657
Compare
fxmarty-amd
left a comment
There was a problem hiding this comment.
I think these self.is_aiter_triton_fp8_bmm_enabled / self.is_aiter_triton_fp4_bmm_enabled flags were precisely meant to bypass any exclusion rule, e.g. with https://huggingface.co/amd/DeepSeek-R1-0528-MXFP4 - see my offline message
See #52167 => these flags are enabled by default, silently turning on online quantization while users are unaware. This should be moved to use --quantization-config.targets.
Enabling silent online quantization that may have an impact on accuracy is not great IMO
…k that the online quantization path is silent Signed-off-by: xuebwang-amd <xuebwang@amd.com>
|
|
|
|
Correct, it's not in the diff. The guard only checks |
|
I fail to understand vllm/vllm/model_executor/layers/quantization/quark/quark.py Lines 87 to 112 in a60c66e vllm/vllm/model_executor/layers/attention/mla_attention.py Lines 1112 to 1124 in a60c66e Completely unrelated to: I also fail to see the connection with FP8:
|
It is not complementary, it should be the proper long-term solution, removing all this aiter-specific logic. The current vllm behavior of silently quantizing layers by default without users opting in is the cause of issues as #48051. |
|
@xuebwang-amd I would prefer for now, we just change the env variables of |
|
Thanks @tjtanaa |
|
@xuebwang-amd TJ's point is to switch: Lines 147 to 148 in 7a99938 to |
|
Let's do this first, accuracy over speed for now.
|
Signed-off-by: xuebwang-amd <xuebwang@amd.com>
|
OK, re-purpose of this PR to be turning off VLLM_ROCM_USE_AITER_*BMM by default for MLA accuracy first. PR title and description are updated as well. |
|
This pull request has merge conflicts that must be resolved before it can be |
|
/ci run |
|
✅ @xuebwang, CI is now available for this PR.
|
|
✅ Triggered Buildkite CI #94742 for commit |
|
Hey TJ, feel free to review #55741 as well that moves these |
|
Glad to see this PR landing. Thanks @tjtanaa ! and @fxmarty-amd as well. |
|
thanks @xuebwang :D |
Signed-off-by: xuebwang-amd <xuebwang@amd.com> Signed-off-by: Andreas Karatzas <AndreasKaratzas@users.noreply.github.com>
Signed-off-by: xuebwang-amd <xuebwang@amd.com>
Purpose
Fixes background issue #52833
VLLM_ROCM_USE_AITER_FP8BMMandVLLM_ROCM_USE_AITER_FP4BMMboth default toTrue. OnceVLLM_ROCM_USE_AITER=1is set, MLAkv_b_projweights are re-quantized online regardless of the precision the checkpoint asked for, and there is no per-model opt-out.kv_b_projholds[W_UK; W_UV], which decompress the latent KV cache on every read, so the perturbation applies to the whole KV history rather than to a single GEMM. That is why the damage shows up first in long context — exactly theregime where these defaults have not been validated.
Two data points:
amd/GLM-5.2-MXFP4, MTP acceptance rate goes 0% -> 58.5% when the re-quantization is skipped ([Bug]: GLM-5.2 MTP accepts 0% of drafts on MI355X (gfx950); disabling expert parallelism hits hipErrorIllegalAddress #52833).This PR flips both defaults to
False. Accuracy over speed until the kernels are validated in long context. Anyone who wants them still gets them withVLLM_ROCM_USE_AITER_FP8BMM=1/VLLM_ROCM_USE_AITER_FP4BMM=1.Changes
vllm/envs.py: both dataclass annotations and bothos.getenvdefaults flipTrue->False; the two "By default is enabled" comments now say what the flags do and why they are off.tests/kernels/quantization/test_rocm_fp8.py:ENV_DEFAULTSpins the newVLLM_ROCM_USE_AITER_FP8BMMdefault.tests/kernels/quantization/test_rocm_mxfp4.py:test_fp4_env_defaultslikewise.Test Plan
Test Result
Passed.
--end--
=======================================
History for ones who may be interested
Purpose
Fixes issue #52833
This PR replaces #48051’s model-name guard with the generic rule:
When that condition is true, both AITER FP4BMM and FP8BMM re-quantization are disabled for the projection and MLA uses the existing BF16 BMM path.
The behavior remains unchanged for:
attention path selects QuarkLinearMethod.
The current dispatch treats a BF16 weight as sufficient evidence that this online re-quantization is safe. That is not true for quantized checkpoints which explicitly exclude kv_b_proj to preserve it in high precision.
Test Plan
focused unit tests
gfx950 model validation
Test Result
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.