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

[luci/service] Migrate Quantize shape inference rule to sinf::Algorithm #13812

Merged
merged 3 commits into from
Aug 29, 2024

Conversation

kyeong8139
Copy link
Contributor

When testing dynamic shape inference, the original quantize operation successfully handles it.
I have migrated the inference code and modified shape_get function to circle_shape

Related to: #13697
draft: #13779
first-pr: #13794 (I fail to modify first commit message.. so I create new pull-request)

@kyeong8139 kyeong8139 added PR/ready for review It is ready to review. Please review it. SSAFY labels Aug 28, 2024
ASSERT_EQ(0, shape.dim(2).value());
ASSERT_EQ(4, shape.dim(3).value());

quantize.drop();
Copy link
Contributor

Choose a reason for hiding this comment

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

have you checked with this drop?


ASSERT_FALSE(shape_inf_rule.infer(&quantize, shape));

quantize.drop();
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

@zetwhite
Copy link
Contributor

Also, plz update your commit ( 24ffc63) title.

[luci-quantize]... to [luci/service]...

@@ -24,4 +27,11 @@ luci::CircleNode *CloneNodeLet<CN::OPQR>::visit(const luci::CircleQuantize *)
return _graph->nodes()->create<luci::CircleQuantize>();
}

loco::TensorShape sinf::Algorithm::visit(const luci::CircleQuantize *node)
Copy link
Contributor

Choose a reason for hiding this comment

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

plz revise this like #13810

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kyeong8139 kyeong8139 force-pushed the luci/quantize branch 3 times, most recently from a7387bd to e3d5482 Compare August 29, 2024 01:03
This commit Migrate Quantize shape inference rule to sinf::Algorithm.

ONE-DCO-1.0-Signed-off-by: kyeong8139 <[email protected]>
This commit modify tests without drop.

ONE-DCO-1.0-Signed-off-by: kyeong8139 <[email protected]>
This commit use namespace.

ONE-DCO-1.0-Signed-off-by: kyeong8139 <[email protected]>
Copy link
Contributor

@seanshpark seanshpark left a comment

Choose a reason for hiding this comment

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

LGTM thank you!

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

@jinevening jinevening merged commit 47f76e7 into Samsung:master Aug 29, 2024
10 checks passed
Copy link
Contributor

@shs-park shs-park left a comment

Choose a reason for hiding this comment

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

👍

@kyeong8139 kyeong8139 deleted the luci/quantize branch September 11, 2024 02:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR/ready for review It is ready to review. Please review it. SSAFY
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants