-
Notifications
You must be signed in to change notification settings - Fork 2.9k
[Transformation] move the place of ConvertWeightCompressedConv1x1ToMatmul pattern #33103
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
[Transformation] move the place of ConvertWeightCompressedConv1x1ToMatmul pattern #33103
Conversation
|
This is just a code movement work for #32224 |
0712b55 to
841f925
Compare
...ormations/src/transformations/op_conversions/convert_weight_compressed_conv1x1_to_matmul.cpp
Outdated
Show resolved
Hide resolved
mdvoretc-intel
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Confirmed that Phi-Silica use case is not affected, LGTM.
…d its test from intel_gpu plugin to common
5c06c94 to
50c547c
Compare
| #include "openvino/opsets/opset7_decl.hpp" | ||
| #include "transformations/rt_info/decompression.hpp" | ||
|
|
||
| using namespace ov; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test coverage seems limited. Currently there are only two test cases: one positive (1x1 conv transforms to matmul) and one negative (3x3 conv does not transform). Are there plans to add tests for:
asymmetric quantization path (with zero-point subtraction)
bias addition path
Parameter weights instead of Constant
Reshape on output instead of Transpose
Convert on output
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added more test cases.
| public: | ||
| OPENVINO_GRAPH_REWRITE_RTTI("ConvertWeightCompressedConv1x1ToMatmul"); | ||
| ConvertWeightCompressedConv1x1ToMatmul() { | ||
| add_matcher<ov::pass::ConvertWeightCompressedConv1x1ToMatmulMatcher>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the transformation contains only one matcher transformation, do we really need to include in GraphRewrite manually?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed the graph rewriter, only keep one match pass ConvertWeightCompressedConv1x1ToMatmul
Move the ConvertWeightCompressedConv1x1ToMatmul pattern and its test from the intel_gpu plugin to the common transformation folder. The purpose is to reuse it on both the CPU and GPU sides.
Tickets: