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

[RUNTIME][CLML] Fix for Softmax op for 4D tensors #16328

Merged
merged 1 commit into from
Jan 18, 2024

Conversation

krishnaraj36
Copy link
Contributor

  • Fixed the softmax layer for 4D tensors to support for NCHW and NHWC layout types.

  • Enabled relevant test cases for softmax layer

Fixed the softmax layer for 4D tensors to support for NCHW and NHWC
layout types.
Enabled relevant test cases for softmax layer
@krishnaraj36
Copy link
Contributor Author

@echuraev Can you please take a look and let me know your feedback.
@srkreddy1238

Copy link
Contributor

@echuraev echuraev left a comment

Choose a reason for hiding this comment

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

Just a small question. In general LGTM. @srkreddy1238 could you please review this PR?

shape)});
} else {
this->layer_.in_placeholder.insert(
{nid, MakeCLMLTensorFromJSONNode(node, layout, dtype, node_data, shape)});
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it necessary to add an ICHECK to check the layout or not?

Copy link
Contributor Author

@krishnaraj36 krishnaraj36 Jan 10, 2024

Choose a reason for hiding this comment

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

This if else statement is to support other layout format CL_TENSOR_LAYOUT_NHWC_QCOM as well for input to the clml subgraph.

Thanks for review, please let me know your opinion! @echuraev

Copy link
Contributor

@srkreddy1238 srkreddy1238 left a comment

Choose a reason for hiding this comment

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

LGTM.

@srkreddy1238 srkreddy1238 merged commit a5e883e into apache:main Jan 18, 2024
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants