Skip to content
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

[CPU] [TRANSFORMATIONS] FakeConvert decomposition transformation #28118

Conversation

xuchen-intel
Copy link
Contributor

Details:

  • Implement FakeConvert decomposition transformation.

Tickets:

Prerequisites:

@xuchen-intel xuchen-intel added category: CPU OpenVINO CPU plugin category: transformations OpenVINO Runtime library - Transformations labels Dec 18, 2024
@xuchen-intel xuchen-intel added this to the 2025.0 milestone Dec 18, 2024
@xuchen-intel xuchen-intel requested review from a team as code owners December 18, 2024 03:27
@xuchen-intel xuchen-intel requested review from itikhono and removed request for a team December 18, 2024 03:27
@github-actions github-actions bot added category: IE Tests OpenVINO Test: plugins and common category: ONNX FE OpenVINO ONNX FrontEnd labels Dec 18, 2024
@xuchen-intel xuchen-intel force-pushed the feature/fake_convert_decomposition branch from a5654fb to 7412bc2 Compare December 18, 2024 04:28
const Output<Node> input_scale{fake_convert_node->input_value(1)};
auto input_type = data.get_element_type();

ov::NodeVector decomp_ops;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ov::NodeVector decomp_ops;
ov::pass::NodeRegistry decomp_ops;

use it instead then :

  if (input_type != input_scale.get_element_type()) {
            input_type = input_scale.get_element_type();
            decomp_ops.make<ov::op::v0::Convert>(data, input_type); // it returns created node which can be used later if required
        }
/// at end
  ov::copy_runtime_info(fake_convert_node, decomp_ops.get());

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied. Thanks Pawel!

bool default_shift;
std::tie(data_shape, scale_shape, shift_shape, data_prec, dst_prec, default_shift) = params;

std::shared_ptr<ov::Model> f(nullptr);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
std::shared_ptr<ov::Model> f(nullptr);
std::shared_ptr<ov::Model> model;

Detail: use model instead f

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied. Thanks Pawel!

manager.register_pass<ov::pass::FakeConvertDecomposition>();
manager.run_passes(f);

OV_ASSERT_NO_THROW(check_rt_info(f));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can test be added in test body instead test's setup?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied. Thanks Pawel!

@xuchen-intel xuchen-intel changed the title [Draft] [CPU] [TRANSFORMATIONS] FakeConvert decomposition transformation [CPU] [TRANSFORMATIONS] FakeConvert decomposition transformation Dec 20, 2024
@xuchen-intel xuchen-intel force-pushed the feature/fake_convert_decomposition branch 2 times, most recently from 5690a3e to 13dbba9 Compare December 20, 2024 02:48
@xuchen-intel
Copy link
Contributor Author

@praasz @t-jankowski Hi Pawel and Tomasz, do you have more comments?

@xuchen-intel
Copy link
Contributor Author

@dmitry-gorokhov Hi Dmitry, could you please also take a look?

@xuchen-intel xuchen-intel force-pushed the feature/fake_convert_decomposition branch from 13dbba9 to f5b166a Compare December 26, 2024 02:00
@github-actions github-actions bot removed category: IE Tests OpenVINO Test: plugins and common category: ONNX FE OpenVINO ONNX FrontEnd labels Dec 26, 2024
@dmitry-gorokhov
Copy link
Contributor

@xuchen-intel Could you please also implement shared single layer test for FakeConvert operation and add its instance for CPU plugin?

@xuchen-intel xuchen-intel force-pushed the feature/fake_convert_decomposition branch from f5b166a to cd1b9a9 Compare December 27, 2024 03:14
@github-actions github-actions bot added the category: IE Tests OpenVINO Test: plugins and common label Dec 27, 2024
@xuchen-intel
Copy link
Contributor Author

@xuchen-intel Could you please also implement shared single layer test for FakeConvert operation and add its instance for CPU plugin?

Applied. Thanks Dmitry!

@dmitry-gorokhov dmitry-gorokhov added this pull request to the merge queue Dec 29, 2024
Merged via the queue into openvinotoolkit:master with commit 2ef42d4 Dec 29, 2024
197 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: CPU OpenVINO CPU plugin category: IE Tests OpenVINO Test: plugins and common category: transformations OpenVINO Runtime library - Transformations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants