Skip to content

[CPU] fix CPU FullyConnected Node zeros value output issue #30197

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

liubo-intel
Copy link
Contributor

Details:

  • CPU internal FullyConnected Node will use cached constant weights during inference, while this will make the output of this node always zero values when its weight 'constant_path' conatains 'RandomUniform' Operation. so need to change current 'constant_path' logic to keep use Matmul kernels for these cases.

Tickets:

@liubo-intel liubo-intel requested review from a team as code owners April 18, 2025 00:49
@liubo-intel liubo-intel requested review from itikhono and removed request for a team April 18, 2025 00:49
@github-actions github-actions bot added category: CPU OpenVINO CPU plugin category: transformations OpenVINO Runtime library - Transformations labels Apr 18, 2025
@liubo-intel
Copy link
Contributor Author

Hi, @luweizhou2016 : could you please help review this pr when you are free? thanks.

@liubo-intel liubo-intel force-pushed the liubo/fix_CPU_fc_node_zeros_value_issue branch from 7097b71 to 67ed238 Compare April 18, 2025 02:36
return false;
}
}

if (current_node->get_input_size() == 0 && !ov::is_type<ov::op::v0::Constant>(current_node)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

It should be a bug on is_on_constant_path(). why not try more general fixing? The RandomUniform() node can't be const folded.

Suggested change
if (current_node->get_input_size() == 0 && !ov::is_type<ov::op::v0::Constant>(current_node)) {
if ((current_node->get_input_size() == 0 && !ov::is_type<ov::op::v0::Constant>(current_node)) ||
current_node->can_constant_fold(current_node->input_values())){

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi, @luweizhou2016 : do you mean with this changes || !current_node->can_constant_fold(current_node->input_values()) ?
but it seems with this kind of changes, we will limit the condition to constant inputs only, while this seems will make current 'constant_path' logic miss some cases, e.g. this specific following kind( the following second 'Eltwise' pattern should be suitable pattern for 'constant_path' logic if none the 'RandomUniform'):
image

Copy link
Contributor

Choose a reason for hiding this comment

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

OK. seems can_constant_fold() can't propagate to the graph input.
What about " || ov::is_type(ov::op::v8::RandomUniform)", the key is this should be a bug and should be fixed in the is_on_constant_path() internally not by the API user. Once the graph path has the RandomUniform OP, it should return false.

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: transformations OpenVINO Runtime library - Transformations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants