[transformation][CPU][GPU] support 3D rope #31949

tiger100256-hu · 2025-09-03T05:56:45Z

Details:

support 3D rope fusing in Qwen2.5-vl-3b and Qwen2-vl-7b

Tickets:

CVS-171588, CVS-171442

Signed-off-by: HU Yuan2 <[email protected]>

Copilot

Pull Request Overview

This PR adds support for 3D RoPE (Rotary Positional Embeddings) operation for both CPU and GPU implementations, specifically targeting Qwen2.5-vl-3b and Qwen2-vl-7b models. This enhancement extends the existing RoPE functionality to handle 3-dimensional tensor operations.

Introduces new RoPEFusionVIT3D transformation pass for recognizing and fusing 3D RoPE patterns
Adds support for 3D tensor handling in both CPU and GPU RoPE implementations
Implements comprehensive test coverage for the new 3D RoPE functionality

Reviewed Changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/tests/functional/plugin/shared/src/subgraph/rotary_pos_emb.cpp	Adds RoPETestQwenVL class and test implementation for 3D RoPE patterns
src/tests/functional/plugin/shared/include/subgraph_tests/rotary_pos_emb.hpp	Declares new test class and types for QwenVL 3D RoPE testing
src/tests/functional/plugin/shared/include/shared_test_classes/subgraph/rotary_pos_emb.hpp	Defines RoPETestQwenVL class structure and parameter types
src/plugins/intel_gpu/tests/functional/shared_tests_instances/subgraph_tests/rotary_pos_emb.cpp	Instantiates GPU-specific test cases for QwenVL 3D RoPE
src/plugins/intel_gpu/src/graph/impls/ocl_v2/rope_opt.cpp	Updates GPU kernel work group sizing for 3D RoPE support
src/plugins/intel_gpu/src/graph/impls/ocl_v2/rope_opt.cl	Adds OpenCL kernel support for 3D cos/sin cache indexing
src/plugins/intel_cpu/tests/functional/shared_tests_instances/subgraph_tests/rotary_pos_emb.cpp	Instantiates CPU-specific test cases for QwenVL 3D RoPE
src/plugins/intel_cpu/src/nodes/rope.cpp	Implements CPU 3D tensor reshaping and handling logic
src/common/transformations/tests/common_optimizations/fuse_rotary_positional_embeddings.cpp	Adds comprehensive tests for VIT 3D RoPE transformation
src/common/transformations/src/transformations/common_optimizations/fuse_rotary_positional_embeddings.cpp	Implements RoPEFusionVIT3D transformation pass
src/common/transformations/include/transformations/common_optimizations/fuse_rotary_positional_embeddings.hpp	Declares RoPEFusionVIT3D class
src/common/transformations/include/ov_ops/rotary_positional_embeddings.hpp	Adds support_3d_rope configuration flag

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-09-08T02:06:40Z

src/tests/functional/plugin/shared/src/subgraph/rotary_pos_emb.cpp

+}
+
+void RoPETestQwenVL::SetUp() {
+    const auto& [element_type, _targetDevice, splip_op_type] = this->GetParam();


Variable name contains a typo. 'splip_op_type' should be 'split_op_type'.

Copilot · 2025-09-08T02:06:40Z

src/tests/functional/plugin/shared/src/subgraph/rotary_pos_emb.cpp

+    InputShape cos_shape_value = {cos_shape, {Shape{80, 1, 80}}};
+    InputShape sin_shape_value = {sin_shape, {Shape{80, 1, 80}}};
+    init_input_shapes({input_shape_value, cos_shape_value, sin_shape_value});
+    function = buildROPE_QwenVL(element_type, input_shape, cos_shape, sin_shape, splip_op_type);


Variable name contains a typo. 'splip_op_type' should be 'split_op_type' to match the intended parameter name.

Copilot · 2025-09-08T02:06:41Z

src/plugins/intel_gpu/src/graph/impls/ocl_v2/rope_opt.cl

+    uint sin_idx = cos_sin_idx;
+#else
+    uint cos_idx = INPUT1_GET_INDEX(cos_sin_b, cos_sin_h, 0, 0);
+    uint sin_idx = INPUT2_GET_INDEX(cos_sin_b, cos_sin_h, 0, 0);   


Line has trailing whitespace that should be removed.

Suggested change

uint sin_idx = INPUT2_GET_INDEX(cos_sin_b, cos_sin_h, 0, 0);

uint sin_idx = INPUT2_GET_INDEX(cos_sin_b, cos_sin_h, 0, 0);

zhangYiIntel · 2025-09-09T02:17:48Z

...ansformations/src/transformations/common_optimizations/fuse_rotary_positional_embeddings.cpp

+    auto x1 = NewGenSlice(x, 0, "half_ndims", 1, 2);
+    auto x_rotate_half = pattern::wrap_type<v0::Concat>({x2neg, x1 | varsplit->output(0)}, {{"axis", -1}});
+
+    auto mul_cos = pattern::wrap_type<v1::Multiply>({x_or_cos1, x_or_cos2}, {{"auto_broadcast", "numpy"}});


For mul_cos, if you want to switch lefthand and righthand of the Multiply maybe a or is better

auto mul_cos1 = pattern::wrap_type<v1::Multiply>({x, cos}, {{"auto_broadcast", "numpy"}}); auto mul_cos2 = pattern::wrap_type<v1::Multiply>({cos, x}, {{"auto_broadcast", "numpy"}}); auto mul_cos = mul_cos1 | mul_cos2

fixed, please also help to take a look at the comment #31949 (comment)

zhangYiIntel · 2025-09-09T02:18:56Z

...ansformations/src/transformations/common_optimizations/fuse_rotary_positional_embeddings.cpp

+
+        // check mul(x, cos) exists
+        Output<Node> v_cos;
+        if (pattern_map.at(x_or_cos1) == pattern_map.at(x)) {


If with | of mul_cos suggested above ,this check here could be removed

zhangYiIntel · 2025-09-09T02:22:54Z

...ansformations/src/transformations/common_optimizations/fuse_rotary_positional_embeddings.cpp

    this->register_matcher(m, callback);
 }

+ov::pass::RoPEFusionVIT3D::RoPEFusionVIT3D() {


This pattern matching is almost the same with RoPEFusionGPTNEOX, the only difference is that the rank of input is changed from 4 to 3, do you think we could merge them together ? Just like RoPEFusionQwen(int split_output_id) we could change to RoPEFusionGPTNEOX(int rank)

If not easy to do now, we can leave this as an improvement.

@zhangYiIntel Yes, the transformation code is copy from RoPEFusionGPTNEOX and just modify the rank from 4 to 3, but I don't know the background of RoPEFusionGPTNEOX, so I didn't merge them together, I think we need a review from transformation team.

The RoPEFusionGPTNEOX is copied from CPU previous internal ROPE transformation, it has no special requirement but only describes the vanilla Rope with RotateHalf

ok, I will merge them together

already merged them

Signed-off-by: HU Yuan2 <[email protected]>

maxnick · 2025-09-17T12:59:21Z

@mitruska , could you please review this PR, especially the transformations part?

mitruska · 2025-09-23T09:10:36Z

src/common/transformations/include/ov_ops/rotary_positional_embeddings.hpp

                                        // each head. change input order to [batch, head_cnt, 4608] to support 2d rope
        bool is_qwen = false;           // Qwen is special which overrides other setting
        bool use_rope_cache = false;    // use precomputed RoPE cache for trigonometric values (cosine and sine)
+        bool support_3d_rope = false;


As new attribute "support_3d_rope" has been added, the attribute should be included in visit attributes method as well:

openvino/src/common/transformations/src/ov_ops/rotary_positional_embeddings.cpp

Lines 87 to 105 in a03e4c1

bool RoPE::visit_attributes(ov::AttributeVisitor& visitor) {

INTERNAL_OP_SCOPE(internal_RoPE_visit_attributes);

visitor.start_structure("config");

visitor.on_attribute("slice_start", m_config.slice_start);

visitor.on_attribute("slice_stop", m_config.slice_stop);

visitor.on_attribute("input_trans0213", m_config.input_trans0213);

visitor.on_attribute("output_trans0213", m_config.output_trans0213);

visitor.on_attribute("is_interleaved", m_config.is_interleaved);

visitor.on_attribute("rotary_ndims", m_config.rotary_ndims);

visitor.on_attribute("is_chatglm", m_config.is_chatglm);

visitor.on_attribute("support_2d_rope", m_config.support_2d_rope);

visitor.on_attribute("is_qwen", m_config.is_qwen);

visitor.on_attribute("use_rope_cache", m_config.use_rope_cache);

visitor.on_attribute("head_cnt", m_config.head_cnt);

visitor.on_attribute("head_size", m_config.head_size);

visitor.on_attribute("gather_position_arg_id", m_config.gather_position_arg_id);

visitor.finish_structure();

return true;

}

@zaixing-wang the new config support_3d_rope is used by GPU currently in this PR, could you check if we must add this new attribute, thanks.

mitruska · 2025-09-23T09:19:06Z

...ansformations/src/transformations/common_optimizations/fuse_rotary_positional_embeddings.cpp

+    auto mul_cos1 = pattern::wrap_type<v1::Multiply>({x, cos}, {{"auto_broadcast", "numpy"}});
+    auto mul_cos2 = pattern::wrap_type<v1::Multiply>({cos, x}, {{"auto_broadcast", "numpy"}});
+    auto mul_cos = mul_cos1 | mul_cos2;


There is a mechanism in transformations for commutative operations like Multiply, which match permutation of the pattern inputs, so this explicit "Or" may be redundant here. Please verify.

openvino/src/core/src/op/util/op_types.cpp

Lines 72 to 81 in a03e4c1

bool ov::op::util::is_commutative(const ov::Node* node) {

return ov::as_type<const ov::op::v1::Add>(node) != nullptr ||

ov::as_type<const ov::op::v1::Maximum>(node) != nullptr ||

ov::as_type<const ov::op::v1::Equal>(node) != nullptr ||

ov::as_type<const ov::op::v1::NotEqual>(node) != nullptr ||

ov::as_type<const ov::op::v1::LogicalAnd>(node) != nullptr ||

ov::as_type<const ov::op::v0::Xor>(node) != nullptr ||

ov::as_type<const ov::op::v1::LogicalXor>(node) != nullptr ||

ov::as_type<const ov::op::v1::Minimum>(node) != nullptr ||

ov::as_type<const ov::op::v1::Multiply>(node) != nullptr ||

openvino/src/core/src/pattern/matcher.cpp

Line 159 in a03e4c1

if (ov::op::util::is_commutative(graph_node)) {

Suggested change

auto mul_cos1 = pattern::wrap_type<v1::Multiply>({x, cos}, {{"auto_broadcast", "numpy"}});

auto mul_cos2 = pattern::wrap_type<v1::Multiply>({cos, x}, {{"auto_broadcast", "numpy"}});

auto mul_cos = mul_cos1 | mul_cos2;

auto mul_cos = pattern::wrap_type<v1::Multiply>({x, cos}, {{"auto_broadcast", "numpy"}});

thanks. I modify this part as the comment of #31949 (comment), @mitruska could you also help to take a look at it thanks.

This method is more universal, I think you could adopt this one

revert back to origin code

mitruska · 2025-09-23T09:29:49Z

...ansformations/src/transformations/common_optimizations/fuse_rotary_positional_embeddings.cpp

+    auto x = pattern::any_input(pattern::rank_equals(rank));
+    auto cos = pattern::any_input(pattern::rank_equals(rank));
+    auto t_sin = pattern::any_input(pattern::rank_equals(rank));


Different ranks could be handled within a single pattern match, so the ranks to be fused could be passed as a list of ranks instead of single value. But it's rather a suggestion to be considered not a blocker.

ok, I will take a look to how to add a pattern for different ranks

I only find rank_equals and rank_more_than, I am not sure if need to add a new one, the rank also need be use later, and I don't know how to get it from x.

auto varsplit = pattern::wrap_type<v1::VariadicSplit>({x, rank - 1, {"half_ndims", "?"}});

openvino/src/core/src/pattern/op/pattern.cpp

Line 139 in 09d1712

op::Predicate rank_equals(const Dimension& expected_rank) {

mitruska · 2025-09-23T09:34:41Z

...ansformations/src/transformations/common_optimizations/fuse_rotary_positional_embeddings.cpp

+    symbolic_ctx_manager->register_pass<ov::pass::RoPEFusionGPTNEOX>(4);
    symbolic_ctx_manager->register_pass<ov::pass::RoPEFusionGPTJ>();
+    symbolic_ctx_manager->register_pass<ov::pass::RoPEFusionGPTNEOX>(3);


Is the order of the pass registration here intentional and crucial for correct functionality?
I wonder why RoPEFusionGPTNEOX(3) is not right after RoPEFusionGPTNEOX(4).

because when it was a draft PR, the code is like this, the new registration is at the end.

symbolic_ctx_manager->register_pass<ov::pass::RoPEFusionGPTNEOX>(); symbolic_ctx_manager->register_pass<ov::pass::RoPEFusionGPTJ>(); symbolic_ctx_manager->register_pass<ov::pass::RoPERoPEFusionVIT3D>();

after fix this #31949 (comment), it become

symbolic_ctx_manager->register_pass<ov::pass::RoPEFusionGPTNEOX>(4); symbolic_ctx_manager->register_pass<ov::pass::RoPEFusionGPTJ>(); symbolic_ctx_manager->register_pass<ov::pass::RoPEFusionGPTNEOX>(3);

I will add it right after RoPEFusionGPTNEOX(4)

yeonbok · 2025-09-24T04:03:18Z

src/plugins/intel_gpu/tests/functional/shared_tests_instances/subgraph_tests/rotary_pos_emb.cpp

+INSTANTIATE_TEST_SUITE_P(smoke_RoPEQwenVL,
+                         RoPETestQwenVL,
+                         ::testing::Combine(
+                            ::testing::Values(ov::element::f32),


please add f16 test too.

yeonbok · 2025-09-24T04:04:36Z

LGTM for GPU plugin change

Signed-off-by: HU Yuan2 <[email protected]>

tiger100256-hu and others added 7 commits September 1, 2025 16:39

fusing vit rope

45697fa

Signed-off-by: HU Yuan2 <[email protected]>

fix fusing issue in benchmark_vlm.py

61f22be

Signed-off-by: HU Yuan2 <[email protected]>

also support use benchmark_app

7099a0c

Signed-off-by: HU Yuan2 <[email protected]>

fix CPU plugin issue for 3d rope

0b6a834

Signed-off-by: HU Yuan2 <[email protected]>

draft

0c143b1

fix

e7a555d

fix

369eff5

github-actions bot added category: IE Tests OpenVINO Test: plugins and common category: GPU OpenVINO GPU plugin category: CPU OpenVINO CPU plugin category: transformations OpenVINO Runtime library - Transformations labels Sep 3, 2025

save tmp test

afe7329

Signed-off-by: HU Yuan2 <[email protected]>

tiger100256-hu force-pushed the huyuan/qwen_vit branch from 1557419 to 45eef10 Compare September 4, 2025 04:31

tiger100256-hu and others added 6 commits September 4, 2025 12:34

transformer test

c8ef478

Signed-off-by: HU Yuan2 <[email protected]>

clean

a6ae3e5

clean

365db5a

clean

483ba3f

clean

45eef10

add CPU plugin test case

e9cf458

Signed-off-by: HU Yuan2 <[email protected]>

tiger100256-hu force-pushed the huyuan/qwen_vit branch from 339f91b to 324cb6a Compare September 4, 2025 06:00

tiger100256-hu added 2 commits September 4, 2025 14:13

fix format issue

fe50b84

Signed-off-by: HU Yuan2 <[email protected]>

clean code in transformation

324cb6a

Signed-off-by: HU Yuan2 <[email protected]>

tiger100256-hu force-pushed the huyuan/qwen_vit branch from 22cdecd to 0cf277a Compare September 5, 2025 02:20

add more test for tansformation, GPU and CPU

0cf277a

Signed-off-by: HU Yuan2 <[email protected]>

tiger100256-hu force-pushed the huyuan/qwen_vit branch from b6e5d19 to 1bbd7e8 Compare September 5, 2025 02:54

tiger100256-hu marked this pull request as ready for review September 5, 2025 03:08

tiger100256-hu requested review from a team as code owners September 5, 2025 03:08

tiger100256-hu requested review from mryzhov and zaixing-wang and removed request for a team September 5, 2025 03:08

fix format issue

1bbd7e8

Signed-off-by: HU Yuan2 <[email protected]>

yuxu42 requested review from zhangYiIntel and Copilot September 8, 2025 02:05

Copilot AI reviewed Sep 8, 2025

View reviewed changes

zhangYiIntel requested changes Sep 9, 2025

View reviewed changes

tiger100256-hu added 3 commits September 10, 2025 15:35

fix comments

02ac1f6

Signed-off-by: HU Yuan2 <[email protected]>

fix the testname, it should be skipped on ARM

8180a15

Signed-off-by: HU Yuan2 <[email protected]>

remove RoPEFusionVIT3D, use RoPEFusionGPTNEOX(3)

f2bba9c

Signed-off-by: HU Yuan2 <[email protected]>

maxnick self-assigned this Sep 11, 2025

tiger100256-hu requested a review from zhangYiIntel September 11, 2025 22:17

peterchen-intel requested a review from xipingyan September 14, 2025 12:49

maxnick requested a review from mitruska September 17, 2025 12:59

maxnick assigned mitruska Sep 17, 2025

maxnick added this to the 2025.4 milestone Sep 17, 2025

chenhu-wang mentioned this pull request Sep 19, 2025

Qwen3 MoE 30b a3b model optimization #32151

Open

peterchen-intel requested review from yeonbok and isanghao September 22, 2025 00:15

mitruska reviewed Sep 23, 2025

View reviewed changes

yeonbok reviewed Sep 24, 2025

View reviewed changes

tiger100256-hu added 2 commits September 28, 2025 13:34

add fp16 test for GPU, and move rank 3 after RoPEFusionGPTNEOX>(4);

c1d5e4d

Signed-off-by: HU Yuan2 <[email protected]>

add attribute for config support_3d_rope

14c1db1

Signed-off-by: HU Yuan2 <[email protected]>

tiger100256-hu force-pushed the huyuan/qwen_vit branch from 0a0ebf6 to ea00c5f Compare September 29, 2025 09:03

revert to origin mul_cos code

ea00c5f

Signed-off-by: HU Yuan2 <[email protected]>

zhangYiIntel approved these changes Sep 30, 2025

View reviewed changes

mitruska approved these changes Sep 30, 2025

View reviewed changes

	uint sin_idx = INPUT2_GET_INDEX(cos_sin_b, cos_sin_h, 0, 0);
	uint sin_idx = INPUT2_GET_INDEX(cos_sin_b, cos_sin_h, 0, 0);

	bool RoPE::visit_attributes(ov::AttributeVisitor& visitor) {
	INTERNAL_OP_SCOPE(internal_RoPE_visit_attributes);
	visitor.start_structure("config");
	visitor.on_attribute("slice_start", m_config.slice_start);
	visitor.on_attribute("slice_stop", m_config.slice_stop);
	visitor.on_attribute("input_trans0213", m_config.input_trans0213);
	visitor.on_attribute("output_trans0213", m_config.output_trans0213);
	visitor.on_attribute("is_interleaved", m_config.is_interleaved);
	visitor.on_attribute("rotary_ndims", m_config.rotary_ndims);
	visitor.on_attribute("is_chatglm", m_config.is_chatglm);
	visitor.on_attribute("support_2d_rope", m_config.support_2d_rope);
	visitor.on_attribute("is_qwen", m_config.is_qwen);
	visitor.on_attribute("use_rope_cache", m_config.use_rope_cache);
	visitor.on_attribute("head_cnt", m_config.head_cnt);
	visitor.on_attribute("head_size", m_config.head_size);
	visitor.on_attribute("gather_position_arg_id", m_config.gather_position_arg_id);
	visitor.finish_structure();
	return true;
	}

	bool ov::op::util::is_commutative(const ov::Node* node) {
	return ov::as_type<const ov::op::v1::Add>(node) != nullptr \|\|
	ov::as_type<const ov::op::v1::Maximum>(node) != nullptr \|\|
	ov::as_type<const ov::op::v1::Equal>(node) != nullptr \|\|
	ov::as_type<const ov::op::v1::NotEqual>(node) != nullptr \|\|
	ov::as_type<const ov::op::v1::LogicalAnd>(node) != nullptr \|\|
	ov::as_type<const ov::op::v0::Xor>(node) != nullptr \|\|
	ov::as_type<const ov::op::v1::LogicalXor>(node) != nullptr \|\|
	ov::as_type<const ov::op::v1::Minimum>(node) != nullptr \|\|
	ov::as_type<const ov::op::v1::Multiply>(node) != nullptr \|\|

[transformation][CPU][GPU] support 3D rope #31949

Are you sure you want to change the base?

[transformation][CPU][GPU] support 3D rope #31949

Conversation

tiger100256-hu commented Sep 3, 2025 • edited by peterchen-intel Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Details:

Tickets:

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Sep 8, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 8, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 8, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maxnick commented Sep 17, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

tiger100256-hu commented Sep 3, 2025 •

edited by peterchen-intel

Loading