[XPU] Enable sequence parallel support for XPU - #38608
Conversation
There was a problem hiding this comment.
Code Review
This pull request enables sequence parallelism tests on XPU platforms by updating pytest markers and generalizing device selection using current_platform.device_type and torch.accelerator. It also moves the SequenceParallelismPass import out of the CUDA-specific guard in the pass manager. Feedback indicates that moving this pass alone is insufficient, as RMSNormQuantFusionPass remains guarded but is required by the newly enabled XPU tests, which will likely result in a NameError.
| @@ -23,13 +23,14 @@ | |||
| RocmAiterTritonAddRMSNormPadFusionPass, | |||
| ) | |||
|
|
|||
| from .fusion.sequence_parallelism import SequenceParallelismPass | |||
There was a problem hiding this comment.
Moving SequenceParallelismPass out of the is_cuda_alike() guard is necessary to support it on XPU. However, RMSNormQuantFusionPass (currently at line 32) remains inside the guard. Since the XPU tests added in this PR (tests/compile/passes/distributed/test_sequence_parallelism.py) explicitly enable fuse_norm_quant, the PostGradPassManager.configure() method will raise a NameError on XPU platforms when it attempts to instantiate RMSNormQuantFusionPass.
Additionally, AsyncTPPass (line 39) is guarded by is_cuda(), which will cause a similar NameError if fuse_gemm_comms is enabled on XPU. You should move RMSNormQuantFusionPass out of the guard as well, and ensure AsyncTPPass is handled safely for XPU.
8e50c2c to
0bcadcd
Compare
0bcadcd to
9b5336e
Compare
9b5336e to
aae4a27
Compare
|
This pull request has merge conflicts that must be resolved before it can be |
|
@chaojun-zhang Any latency difference w/ and w/o this feature enabled? and also the case with asyncTP enabled. |
751f05a to
e31dea0
Compare
|
| @@ -247,6 +247,10 @@ def is_data_center_gpu(cls) -> bool: | |||
| device_name = cls.get_device_name().lower() | |||
| return device_name.count("data center gpu") > 0 | |||
|
|
|||
| @classmethod | |||
| def use_custom_op_collectives(cls) -> bool: | |||
There was a problem hiding this comment.
I remember that all reduce op provided by xpu communicator is in place instead of out place, this may have accuracy issue or some other issue.
6209eda to
4e23d12
Compare
| @@ -111,7 +129,9 @@ def __init__( | |||
| self.tp_size = get_tensor_model_parallel_world_size() | |||
|
|
|||
| def _all_reduce(self, x: torch.Tensor) -> torch.Tensor: | |||
| return tensor_model_parallel_all_reduce(x) | |||
| return torch.ops.vllm.all_reduce.default( | |||
There was a problem hiding this comment.
yes. safe to use custom op for torch.compile mode , otherwise it will throw below error when xpu set use_custom_op_collectives to False.
NotImplementedError: c10d::allreduce_: attempted to run this operator with Meta tensors, but there was no fake impl or Meta kernel registered.
There was a problem hiding this comment.
my question may be :
- will it break other platform/OOT device if it doesn't registere this but enabled this pass.
- will it impact cuda graph capture behavior.
There was a problem hiding this comment.
- It won't break other platform. when enable SP pass, custom-op collectives must be use. To use the custom-op collective, either:
- directly in the SP pass, or
- indirectly by enabling use_custom_op_collectives(). (CUDA & ROCM follow this path)
- I have reverted code and enabling use_custom_op_collectives For XPU.
|
This pull request has merge conflicts that must be resolved before it can be |
4e23d12 to
b3ee390
Compare
b3ee390 to
cfa425e
Compare
|
Documentation preview: https://vllm--38608.org.readthedocs.build/en/38608/ |
|
This pull request has merge conflicts that must be resolved before it can be |
|
Hi @chaojun-zhang, the pre-commit checks have failed. Please run: uv pip install pre-commit>=4.5.1
pre-commit install
pre-commit run --all-filesThen, commit the changes and push to your branch. For future commits, Tip Is
|
1 similar comment
|
Hi @chaojun-zhang, the pre-commit checks have failed. Please run: uv pip install pre-commit>=4.5.1
pre-commit install
pre-commit run --all-filesThen, commit the changes and push to your branch. For future commits, Tip Is
|
Signed-off-by: chaojun-zhang <chaojun.zhang@intel.com> Signed-off-by: Chaojun Zhang <chaojun.zhang@intel.com>
Signed-off-by: Chaojun,Zhang <chaojun.zhang@intel.com> Signed-off-by: Chaojun Zhang <chaojun.zhang@intel.com>
- Fix mock_cuda_platform: is_xpu.return_value was set to 'not is_cuda', so mock_cuda_platform(is_cuda=False) accidentally triggered the XPU branch in get_sequence_parallelism_threshold. Set is_xpu=False always. - Add mock_xpu_platform fixture for XPU platform mocking. - Add TestGetSequenceParallelismThresholdXPU test class covering: - XPU small hidden_size returns None - XPU large model returns calculated threshold - XPU threshold calculation with various parameters - XPU hidden_size boundary behavior Signed-off-by: Chaojun Zhang <chaojun.zhang@intel.com>
Test Plan
Test Result
UT :
pytest -s -v tests/compile/correctness_e2e/test_sequence_parallel.py
pytest -s -v pytest -s -v tests/compile/correctness_e2e/test_sequence_parallel.py
Accuracy (GSM8K)
meta-llama/Llama-2-13b-chat-hf
Enable SPDisable SPEagerQwen/Qwen3-32B
Enable SPDisable SPEagerBenchmark
meta-llama/Llama-2-13b-chat-hf
EagerDisable SPEnable SPQwen/Qwen3-32B
EagerDisable SPEnable SPSummary
meta-llama/Llama-2-13b-chat-hf,Enable SPgives the best TTFT, whileEagerkeeps the best throughput.Qwen/Qwen3-32B,Enable SPis the best case on TTFT, TPOT, and throughput.Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.