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

kyeong8139 · 2024-08-28T01:50:52Z

When testing dynamic shape inference, the quantize operation successfully handles it.
I have added tests for this case and migrated the inference code.

Related to: #13697
draft: #13779

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

This commit add dynamic shape inference tests for quantize. ONE-DCO-1.0-Signed-off-by: kyeong8139 <[email protected]>

This commit check order of headers. ONE-DCO-1.0-Signed-off-by: kyeong8139 <[email protected]>

seanshpark · 2024-08-28T02:24:07Z

add dynamic shape inference test for Quantize

I was expecting to see test codes... but there exist code migration, but no specific algorithm changes for dynamic shape support.

Can you update the title to express this?

@zetwhite , @shs-park , Q) do we MUST relocate codes ? I don't fully understand/agree on existing design that @llFreetimell made.

shs-park · 2024-08-28T02:43:55Z

Q) do we MUST relocate codes ? I don't fully understand/agree on existing design that @llFreetimell made.

I think this is not a MUST todo,
but we need to have some consistency of the location for the shape inference when we(with SSAFY members)'re working on this time.

The Ops we want to implement this time were discussed in this issue and we have plan to implement them in the location of @llFreetimell's firstly.

seanshpark · 2024-08-28T02:51:52Z

but we need to have some consistency of the...

Q) does Quantize have any problem with dynamic shape inference?

Thoughts,

we can relocate Quantize with additional tests with dynamic shape inference
we can skip this Quantize if there is no problem as of now. eventually it looks like migration is needed. that is if there exist other Ops that need to migrate for dynamic shape infer, lets work on that ops with higher priority.
current title/comments needs update to express for actual code changes.

zetwhite · 2024-08-28T03:07:08Z

I think re-location is helpful to distinguish which op is ready to support dynamic shape and which op is not yet. If not, others might try to re-investigate to support dynamic shapes for the Quantize node.

we can relocate Quantize with additional tests with dynamic shape inference

For me, this option looks good.

current title/comments needs update to express for actual code changes.

For PR title, how about 'Migrate Quantize shape inference rule to sinf::Algorithm'?

(+) /cc @kyeong8139 plz update pr title :)

seanshpark · 2024-08-28T03:09:12Z

... is helpful to distinguish which op is ready to ...

👍🏻

'Migrate Quantize shape inference rule to sinf::Algorithm'?

good to me :)

shs-park · 2024-08-28T03:10:47Z

Q) does Quantize have any problem with dynamic shape inference?

AFAIK roughly, this Op is related to the prompt parsing and token gen parts, from #13697.

@jinevening, could you explain a little bit more..?

kyeong8139 · 2024-08-28T03:15:05Z

For PR title, how about 'Migrate Quantize shape inference rule to sinf::Algorithm'?

I change the title to 'Migrate Quantize shape inference rule to sinf::Algorithm'. I think It is appropriate title about this, too.

seanshpark · 2024-08-28T03:17:55Z

From https://github.com/Samsung/ONE/pull/13794/commits

commit message is

[luci-quantize] add dynamic shape tests

This commit add dynamic shape inference tests for quantize.

can you please fix your commit?
github UI title/commit message won't update the commit itself.

This commit Migrate Quantize shape inference rule to sinf::Algorithm. ONE-DCO-1.0-Signed-off-by: kyeong8139 <[email protected]>

kyeong8139 · 2024-08-28T03:32:26Z

github UI title/commit message won't update the commit itself.

I fixed my commit.
I'd like to ask something. I changed compiler/luci/service/src/Nodes/CircleQuantize.test.cpp to ensure that original quantize algorithm support dynamic shape inference.
it isn't necessary if OP don't have problem for supporting dynamic shape? or It isn't enough to test it?

zetwhite · 2024-08-28T04:08:22Z

I changed compiler/luci/service/src/Nodes/CircleQuantize.test.cpp to ensure that original quantize algorithm support dynamic shape inference.

I agree with this comment. I think adding tc to show it supports dynamic shape well is good idea.

+) Also current test you wrote looks good to me.

seanshpark · 2024-08-28T04:22:48Z

I fixed my commit.

https://github.com/Samsung/ONE/pull/13794/commits will show lots of commits
that will be accumulated (we use squash merge) when landing.

you need to update the first commit.

jinevening · 2024-08-28T04:23:14Z

@jinevening, could you explain a little bit more..?

Quantize Op is included in mixed-precision LLM.

shs-park

LGTM!
=)

jinevening · 2024-08-28T04:26:00Z

compiler/luci/service/src/Nodes/CircleQuantize.cpp

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

+loco::TensorShape sinf::Algorithm::visit(const luci::CircleQuantize *node)
+{
+  const auto input_shape = luci::shape_get(node->input()).as<loco::TensorShape>();


Could you use circle_shape function? shape_get is a temporary function, which will be removed in the future.

CircleShapeInferenceHelper.h

// NOTE Functions in this namespace will be removed after new inference // algorithms are fully implemented. // This function is temporary function for deprecating loco::shape_get loco::NodeShape shape_get(const loco::Node *node);

This commit add dynamic shape inference tests for quantize. ONE-DCO-1.0-Signed-off-by: kyeong8139 <[email protected]>

This commit Migrate Quantize shape inference rule to sinf::Algorithm. ONE-DCO-1.0-Signed-off-by: kyeong8139 <[email protected]>

This commit check order of headers. ONE-DCO-1.0-Signed-off-by: kyeong8139 <[email protected]>

This commit Migrate Quantize shape inference rule to sinf::Algorithm. ONE-DCO-1.0-Signed-off-by: kyeong8139 <[email protected]>

This commit use circle_shape function instead of shape_get. ONE-DCO-1.0-Signed-off-by: kyeong8139 <[email protected]>

jinevening · 2024-08-28T04:42:51Z

compiler/luci/service/src/Nodes/CircleQuantize.cpp

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

+loco::TensorShape sinf::Algorithm::visit(const luci::CircleQuantize *node)
+{
+  const auto input_shape = circle_shape(node);


It has to be input's shape.

This commit migrate quantize shape inference rule to sinf::Algorithm. ONE-DCO-1.0-Signed-off-by: kyeong8139 <[email protected]> [luci-quantize] add dynamic shape tests This commit add dynamic shape inference tests for quantize. ONE-DCO-1.0-Signed-off-by: kyeong8139 <[email protected]> [luci-quantize] check code format This commit add dynamic shape inference tests for quantize. ONE-DCO-1.0-Signed-off-by: kyeong8139 <[email protected]> [luci-quantize] add dynamic shape tests This commit add dynamic shape inference tests for quantize. ONE-DCO-1.0-Signed-off-by: kyeong8139 <[email protected]> [luci-quantize] modify order of headers This commit check order of headers. ONE-DCO-1.0-Signed-off-by: kyeong8139 <[email protected]> [luci-quantize] Migrate Quantize shape inference rule to sinf::Algorithm This commit Migrate Quantize shape inference rule to sinf::Algorithm. ONE-DCO-1.0-Signed-off-by: kyeong8139 <[email protected]> [luci-quantize] add dynamic shape tests This commit add dynamic shape inference tests for quantize. ONE-DCO-1.0-Signed-off-by: kyeong8139 <[email protected]> [luci-quantize] Migrate Quantize shape inference rule to sinf::Algorithm This commit Migrate Quantize shape inference rule to sinf::Algorithm. ONE-DCO-1.0-Signed-off-by: kyeong8139 <[email protected]> [luci-quantize] modify order of headers This commit check order of headers. ONE-DCO-1.0-Signed-off-by: kyeong8139 <[email protected]> [luci-quantize] Migrate Quantize shape inference rule to sinf::Algorithm This commit Migrate Quantize shape inference rule to sinf::Algorithm. ONE-DCO-1.0-Signed-off-by: kyeong8139 <[email protected]>

kyeong8139 · 2024-08-28T07:30:53Z

there are some troblems while using git rebase.
after close this pr, I write new pull request.

kyeong8139 added 4 commits August 27, 2024 14:33

[luci-quantize] add dynamic shape tests

f559b83

This commit add dynamic shape inference tests for quantize. ONE-DCO-1.0-Signed-off-by: kyeong8139 <[email protected]>

[luci-quantize] check code format

c6b4473

This commit add dynamic shape inference tests for quantize. ONE-DCO-1.0-Signed-off-by: kyeong8139 <[email protected]>

[luci-quantize] add dynamic shape tests

e927bc5

This commit add dynamic shape inference tests for quantize. ONE-DCO-1.0-Signed-off-by: kyeong8139 <[email protected]>

[luci-quantize] modify order of headers

930bb06

This commit check order of headers. ONE-DCO-1.0-Signed-off-by: kyeong8139 <[email protected]>

kyeong8139 added PR/ready for review It is ready to review. Please review it. SSAFY labels Aug 28, 2024

kyeong8139 requested review from seanshpark, zetwhite and shs-park August 28, 2024 01:50

kyeong8139 changed the title ~~[luci/quantize] add dynamic shape inference test for Quantize~~ [luci/service] add dynamic shape inference test for Quantize Aug 28, 2024

kyeong8139 changed the title ~~[luci/service] add dynamic shape inference test for Quantize~~ [luci/service] Migrate Quantize shape inference rule to sinf::Algorithm Aug 28, 2024

kyeong8139 force-pushed the quantize branch from e584dfe to c5b2418 Compare August 28, 2024 03:21

[luci-quantize] Migrate Quantize shape inference rule to sinf::Algorithm

4cd9fa5

This commit Migrate Quantize shape inference rule to sinf::Algorithm. ONE-DCO-1.0-Signed-off-by: kyeong8139 <[email protected]>

kyeong8139 force-pushed the quantize branch from c5b2418 to 4cd9fa5 Compare August 28, 2024 03:22

shs-park previously approved these changes Aug 28, 2024

View reviewed changes

jinevening reviewed Aug 28, 2024

View reviewed changes

kyeong8139 added 2 commits August 28, 2024 13:29

[luci-quantize] add dynamic shape tests

9bd9409

This commit add dynamic shape inference tests for quantize. ONE-DCO-1.0-Signed-off-by: kyeong8139 <[email protected]>

[luci-quantize] Migrate Quantize shape inference rule to sinf::Algorithm

cb5175b

This commit Migrate Quantize shape inference rule to sinf::Algorithm. ONE-DCO-1.0-Signed-off-by: kyeong8139 <[email protected]>

kyeong8139 added 3 commits August 28, 2024 13:31

[luci-quantize] modify order of headers

241e321

This commit check order of headers. ONE-DCO-1.0-Signed-off-by: kyeong8139 <[email protected]>

[luci-quantize] Migrate Quantize shape inference rule to sinf::Algorithm

899c987

This commit Migrate Quantize shape inference rule to sinf::Algorithm. ONE-DCO-1.0-Signed-off-by: kyeong8139 <[email protected]>

[luci-quantize] use circle_shape function instead of shape_get

2f33f89

This commit use circle_shape function instead of shape_get. ONE-DCO-1.0-Signed-off-by: kyeong8139 <[email protected]>

kyeong8139 dismissed shs-park’s stale review via 2f33f89 August 28, 2024 04:40

jinevening reviewed Aug 28, 2024

View reviewed changes

kyeong8139 closed this Aug 28, 2024

kyeong8139 mentioned this pull request Aug 28, 2024

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

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

kyeong8139 commented Aug 28, 2024

seanshpark commented Aug 28, 2024

shs-park commented Aug 28, 2024

seanshpark commented Aug 28, 2024

zetwhite commented Aug 28, 2024 •

edited

Loading

seanshpark commented Aug 28, 2024

shs-park commented Aug 28, 2024

kyeong8139 commented Aug 28, 2024 •

edited

Loading

seanshpark commented Aug 28, 2024

kyeong8139 commented Aug 28, 2024 •

edited

Loading

zetwhite commented Aug 28, 2024 •

edited

Loading

seanshpark commented Aug 28, 2024

jinevening commented Aug 28, 2024 •

edited

Loading

shs-park left a comment

jinevening Aug 28, 2024

jinevening Aug 28, 2024

kyeong8139 commented Aug 28, 2024

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

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

Conversation

kyeong8139 commented Aug 28, 2024

seanshpark commented Aug 28, 2024

shs-park commented Aug 28, 2024

seanshpark commented Aug 28, 2024

zetwhite commented Aug 28, 2024 • edited Loading

seanshpark commented Aug 28, 2024

shs-park commented Aug 28, 2024

kyeong8139 commented Aug 28, 2024 • edited Loading

seanshpark commented Aug 28, 2024

kyeong8139 commented Aug 28, 2024 • edited Loading

zetwhite commented Aug 28, 2024 • edited Loading

seanshpark commented Aug 28, 2024

jinevening commented Aug 28, 2024 • edited Loading

shs-park left a comment

Choose a reason for hiding this comment

jinevening Aug 28, 2024

Choose a reason for hiding this comment

jinevening Aug 28, 2024

Choose a reason for hiding this comment

kyeong8139 commented Aug 28, 2024

zetwhite commented Aug 28, 2024 •

edited

Loading

kyeong8139 commented Aug 28, 2024 •

edited

Loading

kyeong8139 commented Aug 28, 2024 •

edited

Loading

zetwhite commented Aug 28, 2024 •

edited

Loading

jinevening commented Aug 28, 2024 •

edited

Loading