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

[onert/core] Fix ShapeValidator of RmsNorm #14217

Merged

Conversation

seockho-kim
Copy link
Contributor

This commit fixes ShapeValidator of RmsNorm to allow rank 3 input and single gamma.

ONE-DCO-1.0-Signed-off-by: Seockho Kim [email protected]

issue: #14089
draft: #14088

This commit fixes ShapeValidator of RmsNorm to allow rank 3 input and single gamma.

ONE-DCO-1.0-Signed-off-by: Seockho Kim [email protected]
@seockho-kim seockho-kim requested a review from a team October 14, 2024 06:45
Comment on lines 1136 to 1138
auto ifm_shape = operands.at(ifm_index).shape();
auto ofm_shape = operands.at(ofm_index).shape();
auto gamma_shape = operands.at(gamma_index).shape();
Copy link
Contributor

Choose a reason for hiding this comment

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

(optional) How about adding const reference?

Suggested change
auto ifm_shape = operands.at(ifm_index).shape();
auto ofm_shape = operands.at(ofm_index).shape();
auto gamma_shape = operands.at(gamma_index).shape();
const auto& ifm_shape = operands.at(ifm_index).shape();
const auto& ofm_shape = operands.at(ofm_index).shape();
const auto& gamma_shape = operands.at(gamma_index).shape();

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(optional) How about adding const reference?

Okay, I'll update it :)

auto ofm_shape = operands.at(ofm_index).shape();
auto gamma_shape = operands.at(gamma_index).shape();

OP_REQUIRES(ifm_shape.rank() == 3 || ifm_shape.rank() == 4);
Copy link
Contributor

Choose a reason for hiding this comment

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

IDK the background, Is there a reason to support input with rank==3?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

IDK the background, Is there a reason to support input with rank==3?

Current testing model has "rank = 3" inputs, like A x B x C.

zetwhite
zetwhite previously approved these changes Oct 14, 2024
Copy link
Contributor

@zetwhite zetwhite left a comment

Choose a reason for hiding this comment

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

LGTM

@zetwhite zetwhite requested a review from a team October 14, 2024 09:03
Fixes variables to be const in RmsNorm ShapeValidator.

ONE-DCO-1.0-Signed-off-by: Seockho Kim [email protected]
Copy link
Contributor

@glistening glistening left a comment

Choose a reason for hiding this comment

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

LGTM

@glistening glistening merged commit 4337d83 into Samsung:master Oct 15, 2024
9 checks passed
@seockho-kim seockho-kim deleted the onert_core_shapevalidator_rmsnorm branch October 15, 2024 07:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants